From 1e576a35eb7e1eeb2c4a2d0c03955490337891c1 Mon Sep 17 00:00:00 2001 From: Steven Stewart-Gallus Date: Thu, 8 Aug 2013 19:33:34 -0700 Subject: [PATCH 1/2] Isolate common wait_end logic --- src/libextra/sync.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libextra/sync.rs b/src/libextra/sync.rs index 4172c715adb96..3bdb16a13d21a 100644 --- a/src/libextra/sync.rs +++ b/src/libextra/sync.rs @@ -71,6 +71,12 @@ impl WaitQueue { } count } + + fn wait_end(&self) -> WaitEnd { + let (wait_end, signal_end) = comm::oneshot(); + self.tail.send_deferred(signal_end); + wait_end + } } // The building-block used to make semaphores, mutexes, and rwlocks. @@ -99,12 +105,9 @@ impl Sem { do (**self).with |state| { state.count -= 1; if state.count < 0 { - // Create waiter nobe. - let (WaitEnd, SignalEnd) = comm::oneshot(); - // Tell outer scope we need to block. - waiter_nobe = Some(WaitEnd); - // Enqueue ourself. - state.waiters.tail.send_deferred(SignalEnd); + // Create waiter nobe, enqueue ourself, and tell + // outer scope we need to block. + waiter_nobe = Some(state.waiters.wait_end()); } } // Uncomment if you wish to test for sem races. Not valgrind-friendly. @@ -200,10 +203,7 @@ impl<'self> Condvar<'self> { * wait() is equivalent to wait_on(0). */ pub fn wait_on(&self, condvar_id: uint) { - // Create waiter nobe. - let (WaitEnd, SignalEnd) = comm::oneshot(); - let mut WaitEnd = Some(WaitEnd); - let mut SignalEnd = Some(SignalEnd); + let mut WaitEnd = None; let mut out_of_bounds = None; do task::unkillable { // Release lock, 'atomically' enqueuing ourselves in so doing. @@ -215,9 +215,9 @@ impl<'self> Condvar<'self> { if state.count <= 0 { state.waiters.signal(); } - // Enqueue ourself to be woken up by a signaller. - let SignalEnd = SignalEnd.take_unwrap(); - state.blocked[condvar_id].tail.send_deferred(SignalEnd); + // Create waiter nobe, and enqueue ourself to + // be woken up by a signaller. + WaitEnd = Some(state.blocked[condvar_id].wait_end()); } else { out_of_bounds = Some(state.blocked.len()); } 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 2/2] 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 +}