From 4b74b8dca4076797bb928707fb19e98cfbca09d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Sun, 28 Jul 2013 16:40:35 +0200 Subject: [PATCH] Elide unnecessary ret slot allocas When there is only a single store to the ret slot that dominates the load that gets the value for the "ret" instruction, we can elide the ret slot and directly return the operand of the dominating store instruction. This is the same thing that clang does, except for a special case that doesn't seem to affect us. Fixes #8238 --- src/librustc/lib/llvm.rs | 2 + src/librustc/middle/trans/base.rs | 28 +++- src/librustc/middle/trans/basic_block.rs | 42 ++++++ src/librustc/middle/trans/mod.rs | 2 + src/librustc/middle/trans/value.rs | 157 +++++++++++++++++++++++ src/test/codegen/single-return-value.cc | 14 ++ src/test/codegen/single-return-value.rs | 14 ++ 7 files changed, 255 insertions(+), 4 deletions(-) create mode 100644 src/librustc/middle/trans/basic_block.rs create mode 100644 src/librustc/middle/trans/value.rs create mode 100644 src/test/codegen/single-return-value.cc create mode 100644 src/test/codegen/single-return-value.rs diff --git a/src/librustc/lib/llvm.rs b/src/librustc/lib/llvm.rs index 90db3f8edb0a6..11a2429cb8372 100644 --- a/src/librustc/lib/llvm.rs +++ b/src/librustc/lib/llvm.rs @@ -1553,6 +1553,8 @@ pub mod llvm { /* Selected entries from the downcasts. */ #[fast_ffi] pub fn LLVMIsATerminatorInst(Inst: ValueRef) -> ValueRef; + #[fast_ffi] + pub fn LLVMIsAStoreInst(Inst: ValueRef) -> ValueRef; /** Writes a module to the specified path. Returns 0 on success. */ #[fast_ffi] diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index db8a86fe948df..170c6dc335202 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -59,6 +59,7 @@ use middle::trans::monomorphize; use middle::trans::tvec; use middle::trans::type_of; use middle::trans::type_of::*; +use middle::trans::value::Value; use middle::ty; use util::common::indenter; use util::ppaux::{Repr, ty_to_str}; @@ -1791,11 +1792,30 @@ pub fn finish_fn(fcx: @mut FunctionContext, last_bcx: @mut Block) { // Builds the return block for a function. pub fn build_return_block(fcx: &FunctionContext, ret_cx: @mut Block) { // Return the value if this function immediate; otherwise, return void. - if fcx.llretptr.is_some() && fcx.has_immediate_return_value { - Ret(ret_cx, Load(ret_cx, fcx.llretptr.unwrap())) - } else { - RetVoid(ret_cx) + if fcx.llretptr.is_none() || !fcx.has_immediate_return_value { + return RetVoid(ret_cx); } + + let retptr = Value(fcx.llretptr.unwrap()); + let retval = match retptr.get_dominating_store(ret_cx) { + // If there's only a single store to the ret slot, we can directly return + // the value that was stored and omit the store and the alloca + Some(s) => { + let retval = *s.get_operand(0).unwrap(); + s.erase_from_parent(); + + if retptr.has_no_uses() { + retptr.erase_from_parent(); + } + + retval + } + // Otherwise, load the return value from the ret slot + None => Load(ret_cx, fcx.llretptr.unwrap()) + }; + + + Ret(ret_cx, retval); } pub enum self_arg { impl_self(ty::t, ty::SelfMode), no_self, } diff --git a/src/librustc/middle/trans/basic_block.rs b/src/librustc/middle/trans/basic_block.rs new file mode 100644 index 0000000000000..a1f7b5ad5c834 --- /dev/null +++ b/src/librustc/middle/trans/basic_block.rs @@ -0,0 +1,42 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use lib::llvm::{llvm, BasicBlockRef}; +use middle::trans::value::{UserIterator, Value}; +use std::iterator::{Filter, Map}; + +pub struct BasicBlock(BasicBlockRef); + +pub type PredIterator<'self> = Map<'self, Value, BasicBlock, Filter<'self, Value, UserIterator>>; + +/** + * Wrapper for LLVM BasicBlockRef + */ +impl BasicBlock { + pub fn as_value(self) -> Value { + unsafe { + Value(llvm::LLVMBasicBlockAsValue(*self)) + } + } + + pub fn pred_iter(self) -> PredIterator { + self.as_value().user_iter() + .filter(|user| user.is_a_terminator_inst()) + .transform(|user| user.get_parent().unwrap()) + } + + pub fn get_single_predecessor(self) -> Option { + let mut iter = self.pred_iter(); + match (iter.next(), iter.next()) { + (Some(first), None) => Some(first), + _ => None + } + } +} diff --git a/src/librustc/middle/trans/mod.rs b/src/librustc/middle/trans/mod.rs index d47d9a4ff1628..387a8ecc5eec6 100644 --- a/src/librustc/middle/trans/mod.rs +++ b/src/librustc/middle/trans/mod.rs @@ -42,3 +42,5 @@ pub mod machine; pub mod adt; pub mod asm; pub mod type_; +pub mod value; +pub mod basic_block; diff --git a/src/librustc/middle/trans/value.rs b/src/librustc/middle/trans/value.rs new file mode 100644 index 0000000000000..08b2db6eff9bc --- /dev/null +++ b/src/librustc/middle/trans/value.rs @@ -0,0 +1,157 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use lib::llvm::{llvm, UseRef, ValueRef}; +use middle::trans::basic_block::BasicBlock; +use middle::trans::common::Block; +use std::libc::c_uint; + +pub struct Value(ValueRef); + +macro_rules! opt_val ( ($e:expr) => ( + unsafe { + match $e { + p if p.is_not_null() => Some(Value(p)), + _ => None + } + } +)) + +/** + * Wrapper for LLVM ValueRef + */ +impl Value { + /// Returns the BasicBlock that contains this value + pub fn get_parent(self) -> Option { + unsafe { + match llvm::LLVMGetInstructionParent(*self) { + p if p.is_not_null() => Some(BasicBlock(p)), + _ => None + } + } + } + + /// Removes this value from its containing BasicBlock + pub fn erase_from_parent(self) { + unsafe { + llvm::LLVMInstructionEraseFromParent(*self); + } + } + + /// Returns the single dominating store to this value, if any + /// This only performs a search for a trivially dominating store. The store + /// must be the only user of this value, and there must not be any conditional + /// branches between the store and the given block. + pub fn get_dominating_store(self, bcx: @mut Block) -> Option { + match self.get_single_user().chain(|user| user.as_store_inst()) { + Some(store) => { + do store.get_parent().chain |store_bb| { + let mut bb = BasicBlock(bcx.llbb); + let mut ret = Some(store); + while *bb != *store_bb { + match bb.get_single_predecessor() { + Some(pred) => bb = pred, + None => { ret = None; break } + } + } + ret + } + } + _ => None + } + } + + /// Returns the first use of this value, if any + pub fn get_first_use(self) -> Option { + unsafe { + match llvm::LLVMGetFirstUse(*self) { + u if u.is_not_null() => Some(Use(u)), + _ => None + } + } + } + + /// Tests if there are no uses of this value + pub fn has_no_uses(self) -> bool { + self.get_first_use().is_none() + } + + /// Returns the single user of this value + /// If there are no users or multiple users, this returns None + pub fn get_single_user(self) -> Option { + let mut iter = self.user_iter(); + match (iter.next(), iter.next()) { + (Some(first), None) => Some(first), + _ => None + } + } + + /// Returns an iterator for the users of this value + pub fn user_iter(self) -> UserIterator { + UserIterator { + next: self.get_first_use() + } + } + + /// Returns the requested operand of this instruction + /// Returns None, if there's no operand at the given index + pub fn get_operand(self, i: uint) -> Option { + opt_val!(llvm::LLVMGetOperand(*self, i as c_uint)) + } + + /// Returns the Store represent by this value, if any + pub fn as_store_inst(self) -> Option { + opt_val!(llvm::LLVMIsAStoreInst(*self)) + } + + /// Tests if this value is a terminator instruction + pub fn is_a_terminator_inst(self) -> bool { + unsafe { + llvm::LLVMIsATerminatorInst(*self).is_not_null() + } + } +} + +pub struct Use(UseRef); + +/** + * Wrapper for LLVM UseRef + */ +impl Use { + pub fn get_user(self) -> Value { + unsafe { + Value(llvm::LLVMGetUser(*self)) + } + } + + pub fn get_next_use(self) -> Option { + unsafe { + match llvm::LLVMGetNextUse(*self) { + u if u.is_not_null() => Some(Use(u)), + _ => None + } + } + } +} + +/// Iterator for the users of a value +pub struct UserIterator { + priv next: Option +} + +impl Iterator for UserIterator { + fn next(&mut self) -> Option { + let current = self.next; + + self.next = do current.chain |u| { u.get_next_use() }; + + do current.map |u| { u.get_user() } + } +} diff --git a/src/test/codegen/single-return-value.cc b/src/test/codegen/single-return-value.cc new file mode 100644 index 0000000000000..97d80d3950fbb --- /dev/null +++ b/src/test/codegen/single-return-value.cc @@ -0,0 +1,14 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +extern "C" +int test() { + return 5; +} diff --git a/src/test/codegen/single-return-value.rs b/src/test/codegen/single-return-value.rs new file mode 100644 index 0000000000000..e6eb9a2be72cb --- /dev/null +++ b/src/test/codegen/single-return-value.rs @@ -0,0 +1,14 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[no_mangle] +fn test() -> int { + 5 +}