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

@@ -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]

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, }

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<BasicBlock> {
+        let mut iter = self.pred_iter();
+        match (iter.next(), iter.next()) {
+            (Some(first), None) => Some(first),
+            _ => None
+        }
+    }
+}

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;

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<BasicBlock> {
+        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<Value> {
+        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<Use> {
+        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<Value> {
+        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<Value> {
+        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<Value> {
+        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<Use> {
+        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<Use>
+}
+
+impl Iterator<Value> for UserIterator {
+    fn next(&mut self) -> Option<Value> {
+        let current = self.next;
+
+        self.next = do current.chain |u| { u.get_next_use() };
+
+        do current.map |u| { u.get_user() }
+    }
+}

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+extern "C"
+int test() {
+  return 5;
+}

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[no_mangle]
+fn test() -> int {
+    5
+}