From 68cbd6c9294cadd7ff868be0bd756fcde84d757d Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Tue, 29 Jul 2014 12:25:06 -0700 Subject: [PATCH 1/6] librustc: Use separate stack slot for each return. --- src/librustc/middle/trans/base.rs | 65 ++++++++++++++++-------- src/librustc/middle/trans/callee.rs | 15 ++++-- src/librustc/middle/trans/closure.rs | 8 +-- src/librustc/middle/trans/common.rs | 14 ++--- src/librustc/middle/trans/controlflow.rs | 16 ++++-- src/librustc/middle/trans/reflect.rs | 4 +- 6 files changed, 81 insertions(+), 41 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 242a820c997ff..9c304f839dbee 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1210,16 +1210,23 @@ pub fn arrayalloca(cx: &Block, ty: Type, v: ValueRef) -> ValueRef { p } -// Creates and returns space for, or returns the argument representing, the -// slot where the return value of the function must go. -pub fn make_return_pointer(fcx: &FunctionContext, output_type: ty::t) - -> ValueRef { +// Creates the alloca slot which holds the pointer to the slot for the final return value +pub fn make_return_slot_pointer(fcx: &FunctionContext, output_type: ty::t) -> ValueRef { + let lloutputtype = type_of::type_of(fcx.ccx, output_type); + + // Let's create the stack slot + let slot = AllocaFcx(fcx, lloutputtype.ptr_to(), "llretslotptr"); + + // and if we're using an out pointer, then store that in our newly made slot if type_of::return_uses_outptr(fcx.ccx, output_type) { - get_param(fcx.llfn, 0) - } else { - let lloutputtype = type_of::type_of(fcx.ccx, output_type); - AllocaFcx(fcx, lloutputtype, "__make_return_pointer") + let outptr = get_param(fcx.llfn, 0); + + let b = fcx.ccx.builder(); + b.position_before(fcx.alloca_insert_pt.get().unwrap()); + b.store(outptr, slot); } + + slot } // NB: must keep 4 fns in sync: @@ -1258,7 +1265,7 @@ pub fn new_fn_ctxt<'a>(ccx: &'a CrateContext, let mut fcx = FunctionContext { llfn: llfndecl, llenv: None, - llretptr: Cell::new(None), + llretslotptr: Cell::new(None), alloca_insert_pt: Cell::new(None), llreturn: Cell::new(None), personality: Cell::new(None), @@ -1303,12 +1310,12 @@ pub fn init_function<'a>(fcx: &'a FunctionContext<'a>, if !return_type_is_void(fcx.ccx, substd_output_type) { // If the function returns nil/bot, there is no real return - // value, so do not set `llretptr`. + // value, so do not set `llretslotptr`. if !skip_retptr || fcx.caller_expects_out_pointer { - // Otherwise, we normally allocate the llretptr, unless we + // Otherwise, we normally allocate the llretslotptr, unless we // have been instructed to skip it for immediate return // values. - fcx.llretptr.set(Some(make_return_pointer(fcx, substd_output_type))); + fcx.llretslotptr.set(Some(make_return_slot_pointer(fcx, substd_output_type))); } } @@ -1533,12 +1540,12 @@ pub fn finish_fn<'a>(fcx: &'a FunctionContext<'a>, // Builds the return block for a function. pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block, retty: ty::t) { - // Return the value if this function immediate; otherwise, return void. - if fcx.llretptr.get().is_none() || fcx.caller_expects_out_pointer { + if fcx.llretslotptr.get().is_none() { return RetVoid(ret_cx); } - let retptr = Value(fcx.llretptr.get().unwrap()); + let retslot = Load(ret_cx, fcx.llretslotptr.get().unwrap()); + let retptr = Value(retslot); 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 @@ -1557,10 +1564,15 @@ pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block, retty: ty::t) { } } // Otherwise, load the return value from the ret slot - None => load_ty(ret_cx, fcx.llretptr.get().unwrap(), retty) + None => load_ty(ret_cx, retslot, retty) }; - Ret(ret_cx, retval); + if fcx.caller_expects_out_pointer { + store_ty(ret_cx, retval, get_param(fcx.llfn, 0), retty); + RetVoid(ret_cx); + } else { + Ret(ret_cx, retval); + } } #[deriving(Clone, Eq, PartialEq)] @@ -1658,10 +1670,10 @@ pub fn trans_closure(ccx: &CrateContext, // emitting should be enabled. debuginfo::start_emitting_source_locations(&fcx); - let dest = match fcx.llretptr.get() { - Some(e) => {expr::SaveIn(e)} + let dest = match fcx.llretslotptr.get() { + Some(_) => expr::SaveIn(alloca(bcx, type_of::type_of(bcx.ccx(), block_ty), "iret_slot")), None => { - assert!(type_is_zero_size(bcx.ccx(), block_ty)) + assert!(type_is_zero_size(bcx.ccx(), block_ty)); expr::Ignore } }; @@ -1672,6 +1684,13 @@ pub fn trans_closure(ccx: &CrateContext, // (trans_block, trans_expr, et cetera). bcx = controlflow::trans_block(bcx, body, dest); + match dest { + expr::SaveIn(slot) => { + Store(bcx, slot, fcx.llretslotptr.get().unwrap()); + } + _ => {} + } + match fcx.llreturn.get() { Some(_) => { Br(bcx, fcx.return_exit_block()); @@ -1841,16 +1860,18 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext, let arg_datums = create_datums_for_fn_args(&fcx, arg_tys.as_slice()); if !type_is_zero_size(fcx.ccx, result_ty) { + let dest = alloca(bcx, type_of::type_of(bcx.ccx(), result_ty), "eret_slot"); let repr = adt::represent_type(ccx, result_ty); for (i, arg_datum) in arg_datums.move_iter().enumerate() { let lldestptr = adt::trans_field_ptr(bcx, &*repr, - fcx.llretptr.get().unwrap(), + dest, disr, i); arg_datum.store_to(bcx, lldestptr); } - adt::trans_set_discr(bcx, &*repr, fcx.llretptr.get().unwrap(), disr); + adt::trans_set_discr(bcx, &*repr, dest, disr); + Store(bcx, dest, fcx.llretslotptr.get().unwrap()); } finish_fn(&fcx, bcx, result_ty); diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index f186af48321c3..3bea9d6446253 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -389,6 +389,10 @@ pub fn trans_unboxing_shim(bcx: &Block, for i in range(1, arg_types.len()) { llshimmedargs.push(get_param(fcx.llfn, fcx.arg_pos(i) as u32)); } + let dest = match fcx.llretslotptr.get() { + Some(_) => Some(expr::SaveIn(alloca(bcx, type_of::type_of(ccx, return_type), "ret_slot"))), + None => None + }; bcx = trans_call_inner(bcx, None, function_type, @@ -399,10 +403,13 @@ pub fn trans_unboxing_shim(bcx: &Block, } }, ArgVals(llshimmedargs.as_slice()), - match fcx.llretptr.get() { - None => None, - Some(llretptr) => Some(expr::SaveIn(llretptr)), - }).bcx; + dest).bcx; + match dest { + Some(expr::SaveIn(slot)) => { + Store(bcx, slot, fcx.llretslotptr.get().unwrap()); + } + _ => {} + } bcx = fcx.pop_and_trans_custom_cleanup_scope(bcx, arg_scope); finish_fn(&fcx, bcx, return_type); diff --git a/src/librustc/middle/trans/closure.rs b/src/librustc/middle/trans/closure.rs index 98b2ebb70f3cb..dc77edf270638 100644 --- a/src/librustc/middle/trans/closure.rs +++ b/src/librustc/middle/trans/closure.rs @@ -582,16 +582,16 @@ pub fn get_wrapper_for_bare_fn(ccx: &CrateContext, ty::ty_fn_args(closure_ty) .as_slice()); let mut llargs = Vec::new(); - match fcx.llretptr.get() { - Some(llretptr) => { - llargs.push(llretptr); + match fcx.llretslotptr.get() { + Some(llretslotptr) => { + llargs.push(Load(bcx, llretslotptr)); } None => {} } llargs.extend(args.iter().map(|arg| arg.val)); let retval = Call(bcx, fn_ptr, llargs.as_slice(), None); - if type_is_zero_size(ccx, f.sig.output) || fcx.llretptr.get().is_some() { + if type_is_zero_size(ccx, f.sig.output) || fcx.llretslotptr.get().is_some() { RetVoid(bcx); } else { Ret(bcx, retval); diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index 84f380e862a4d..6aca8fe8e4111 100644 --- a/src/librustc/middle/trans/common.rs +++ b/src/librustc/middle/trans/common.rs @@ -240,11 +240,11 @@ pub struct FunctionContext<'a> { // The environment argument in a closure. pub llenv: Option, - // The place to store the return value. If the return type is immediate, - // this is an alloca in the function. Otherwise, it's the hidden first - // parameter to the function. After function construction, this should - // always be Some. - pub llretptr: Cell>, + // A pointer to where to store the return value. If the return type is + // immediate, this points to an alloca in the function. Otherwise, it's a + // pointer to the hidden first parameter of the function. After function + // construction, this should always be Some. + pub llretslotptr: Cell>, // These pub elements: "hoisted basic blocks" containing // administrative activities that have to happen in only one place in @@ -259,8 +259,8 @@ pub struct FunctionContext<'a> { pub personality: Cell>, // True if the caller expects this fn to use the out pointer to - // return. Either way, your code should write into llretptr, but if - // this value is false, llretptr will be a local alloca. + // return. Either way, your code should write into the slot llretslotptr + // points to, but if this value is false, that slot will be a local alloca. pub caller_expects_out_pointer: bool, // Maps arguments to allocas created for them in llallocas. diff --git a/src/librustc/middle/trans/controlflow.rs b/src/librustc/middle/trans/controlflow.rs index 2fd2e69cfc3bc..b9644b1145327 100644 --- a/src/librustc/middle/trans/controlflow.rs +++ b/src/librustc/middle/trans/controlflow.rs @@ -26,6 +26,7 @@ use middle::trans::debuginfo; use middle::trans::expr; use middle::trans::meth; use middle::trans::type_::Type; +use middle::trans::type_of; use middle::ty; use middle::typeck::MethodCall; use util::ppaux::Repr; @@ -462,13 +463,22 @@ pub fn trans_ret<'a>(bcx: &'a Block<'a>, let _icx = push_ctxt("trans_ret"); let fcx = bcx.fcx; let mut bcx = bcx; - let dest = match bcx.fcx.llretptr.get() { - None => expr::Ignore, - Some(retptr) => expr::SaveIn(retptr), + let dest = match (fcx.llretslotptr.get(), e) { + (Some(_), Some(e)) => { + let ret_ty = expr_ty(bcx, &*e); + expr::SaveIn(alloca(bcx, type_of::type_of(bcx.ccx(), ret_ty), "ret_slot")) + } + _ => expr::Ignore, }; match e { Some(x) => { bcx = expr::trans_into(bcx, &*x, dest); + match dest { + expr::SaveIn(slot) => { + Store(bcx, slot, fcx.llretslotptr.get().unwrap()); + } + _ => {} + } } _ => {} } diff --git a/src/librustc/middle/trans/reflect.rs b/src/librustc/middle/trans/reflect.rs index eb0d77da5519f..35ac0b2707195 100644 --- a/src/librustc/middle/trans/reflect.rs +++ b/src/librustc/middle/trans/reflect.rs @@ -321,7 +321,9 @@ impl<'a, 'b> Reflector<'a, 'b> { let arg = get_param(llfdecl, fcx.arg_pos(0u) as c_uint); let arg = BitCast(bcx, arg, llptrty); let ret = adt::trans_get_discr(bcx, &*repr, arg, Some(Type::i64(ccx))); - Store(bcx, ret, fcx.llretptr.get().unwrap()); + let ret_alloca = alloca(bcx, Type::i64(ccx), "ret_slot"); + Store(bcx, ret, ret_alloca); + Store(bcx, ret_alloca, fcx.llretslotptr.get().unwrap()); match fcx.llreturn.get() { Some(llreturn) => Br(bcx, llreturn), None => {} From d3028138889d8eb2aaa3dce899d556f8bbf04d13 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Tue, 29 Jul 2014 12:35:38 -0700 Subject: [PATCH 2/6] Reenable ignored test and add run-pass test. --- src/libserialize/json.rs | 1 - src/test/run-pass/issue-15763.rs | 96 ++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 src/test/run-pass/issue-15763.rs diff --git a/src/libserialize/json.rs b/src/libserialize/json.rs index 58d69e38cc6e0..51b8985e655a5 100644 --- a/src/libserialize/json.rs +++ b/src/libserialize/json.rs @@ -2800,7 +2800,6 @@ mod tests { } } #[test] - #[ignore] // FIXME(#15763) fn test_decode_errors_struct() { check_err::("[]", ExpectedError("Object".to_string(), "[]".to_string())); check_err::("{\"x\": true, \"y\": true, \"z\": \"\", \"w\": []}", diff --git a/src/test/run-pass/issue-15763.rs b/src/test/run-pass/issue-15763.rs new file mode 100644 index 0000000000000..6e3599bda149a --- /dev/null +++ b/src/test/run-pass/issue-15763.rs @@ -0,0 +1,96 @@ +// Copyright 2014 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. + + +#[deriving(PartialEq, Show)] +struct Bar { + x: int +} +impl Drop for Bar { + fn drop(&mut self) { + assert_eq!(self.x, 22); + } +} + +#[deriving(PartialEq, Show)] +struct Foo { + x: Bar, + a: int +} + +fn foo() -> Result { + return Ok(Foo { + x: Bar { x: 22 }, + a: return Err(32) + }); +} + +fn baz() -> Result { + Ok(Foo { + x: Bar { x: 22 }, + a: return Err(32) + }) +} + +// explicit immediate return +fn aa() -> int { + return 3; +} + +// implicit immediate return +fn bb() -> int { + 3 +} + +// implicit outptr return +fn cc() -> Result { + Ok(3) +} + +// explicit outptr return +fn dd() -> Result { + return Ok(3); +} + +trait A { + fn aaa(self) -> int { + 3 + } + fn bbb(self) -> int { + return 3; + } + fn ccc(self) -> Result { + Ok(3) + } + fn ddd(self) -> Result { + return Ok(3); + } +} + +impl A for int {} + +fn main() { + assert_eq!(foo(), Err(32)); + assert_eq!(baz(), Err(32)); + + assert_eq!(aa(), 3); + assert_eq!(bb(), 3); + assert_eq!(cc().unwrap(), 3); + assert_eq!(dd().unwrap(), 3); + + let i = box 32i as Box; + assert_eq!(i.aaa(), 3); + let i = box 32i as Box; + assert_eq!(i.bbb(), 3); + let i = box 32i as Box; + assert_eq!(i.ccc().unwrap(), 3); + let i = box 32i as Box; + assert_eq!(i.ddd().unwrap(), 3); +} From 9dac85f92daa094f94f124fd06cfeb5339d48772 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 4 Aug 2014 11:48:26 -0700 Subject: [PATCH 3/6] librustc: Add an intrinsic to retrieve the return pointer of a function. This is needed for some GC stuff in Servo. --- src/librustc/middle/trans/intrinsic.rs | 6 +++ src/librustc/middle/typeck/check/mod.rs | 2 + src/test/run-pass/intrinsic-return-address.rs | 43 +++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 src/test/run-pass/intrinsic-return-address.rs diff --git a/src/librustc/middle/trans/intrinsic.rs b/src/librustc/middle/trans/intrinsic.rs index fb420a4035d94..154f2b122ab56 100644 --- a/src/librustc/middle/trans/intrinsic.rs +++ b/src/librustc/middle/trans/intrinsic.rs @@ -425,6 +425,12 @@ pub fn trans_intrinsic_call<'a>(mut bcx: &'a Block<'a>, node: ast::NodeId, with_overflow_intrinsic(bcx, "llvm.umul.with.overflow.i64", ret_ty, *llargs.get(0), *llargs.get(1)), + (_, "return_address") => { + PointerCast(bcx, + bcx.fcx.llretptr.get().unwrap(), + Type::i8p(bcx.ccx())) + } + // This requires that atomic intrinsics follow a specific naming pattern: // "atomic_[_]", and no ordering means SeqCst (_, name) if name.starts_with("atomic_") => { diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 787a4bebc559b..aa38ff68f24e6 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -4986,6 +4986,8 @@ pub fn check_intrinsic_type(ccx: &CrateCtxt, it: &ast::ForeignItem) { (0, vec!(ty::mk_u64(), ty::mk_u64()), ty::mk_tup(tcx, vec!(ty::mk_u64(), ty::mk_bool()))), + "return_address" => (0, vec![], ty::mk_imm_ptr(tcx, ty::mk_u8())), + ref other => { span_err!(tcx.sess, it.span, E0093, "unrecognized intrinsic function: `{}`", *other); diff --git a/src/test/run-pass/intrinsic-return-address.rs b/src/test/run-pass/intrinsic-return-address.rs new file mode 100644 index 0000000000000..a28a79d306bbf --- /dev/null +++ b/src/test/run-pass/intrinsic-return-address.rs @@ -0,0 +1,43 @@ +// Copyright 2012-2014 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. + +#![feature(intrinsics)]; + +use std::ptr; + +struct Point { + x: f32, + y: f32, + z: f32, +} + +extern "rust-intrinsic" { + fn return_address() -> *const u8; +} + +fn f(result: &mut uint) -> Point { + unsafe { + *result = return_address() as uint; + Point { + x: 1.0, + y: 2.0, + z: 3.0, + } + } + +} + +fn main() { + let mut intrinsic_reported_address = 0; + let pt = f(&mut intrinsic_reported_address); + let actual_address = &pt as *const Point as uint; + assert_eq!(intrinsic_reported_address, actual_address); +} + From 5aedcb1e916711bfeb65c26b5c6210a3fb5d72bf Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Tue, 5 Aug 2014 14:38:14 -0700 Subject: [PATCH 4/6] librustc: Don't allow return_address intrinsic in functions that don't use an out pointer. --- src/libcore/intrinsics.rs | 7 +++++ src/librustc/middle/trans/callee.rs | 4 ++- src/librustc/middle/trans/intrinsic.rs | 13 +++++--- .../compile-fail/intrinsic-return-address.rs | 31 +++++++++++++++++++ src/test/run-pass/intrinsic-return-address.rs | 2 +- 5 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 src/test/compile-fail/intrinsic-return-address.rs diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 002babf7df976..1dd3d6ce9db9d 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -310,6 +310,13 @@ extern "rust-intrinsic" { /// ``` pub fn transmute(e: T) -> U; + /// Gives the address for the return value of the enclosing function. + /// + /// Using this instrinsic in a function that does not use an out pointer + /// will trigger a compiler error. + #[cfg(not(stage0))] + pub fn return_address() -> *const u8; + /// Returns `true` if a type requires drop glue. pub fn needs_drop() -> bool; diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 3bea9d6446253..dd37f3adab9d5 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -765,9 +765,11 @@ pub fn trans_call_inner<'a>( assert!(abi == synabi::RustIntrinsic); assert!(dest.is_some()); + let call_info = call_info.expect("no call info for intrinsic call?"); return intrinsic::trans_intrinsic_call(bcx, node, callee_ty, arg_cleanup_scope, args, - dest.unwrap(), substs); + dest.unwrap(), substs, + call_info); } NamedTupleConstructor(substs, disr) => { assert!(dest.is_some()); diff --git a/src/librustc/middle/trans/intrinsic.rs b/src/librustc/middle/trans/intrinsic.rs index 154f2b122ab56..359c8d24f7270 100644 --- a/src/librustc/middle/trans/intrinsic.rs +++ b/src/librustc/middle/trans/intrinsic.rs @@ -126,7 +126,7 @@ pub fn check_intrinsics(ccx: &CrateContext) { pub fn trans_intrinsic_call<'a>(mut bcx: &'a Block<'a>, node: ast::NodeId, callee_ty: ty::t, cleanup_scope: cleanup::CustomScopeIndex, args: callee::CallArgs, dest: expr::Dest, - substs: subst::Substs) -> Result<'a> { + substs: subst::Substs, call_info: NodeInfo) -> Result<'a> { let fcx = bcx.fcx; let ccx = fcx.ccx; @@ -426,9 +426,14 @@ pub fn trans_intrinsic_call<'a>(mut bcx: &'a Block<'a>, node: ast::NodeId, *llargs.get(0), *llargs.get(1)), (_, "return_address") => { - PointerCast(bcx, - bcx.fcx.llretptr.get().unwrap(), - Type::i8p(bcx.ccx())) + if !fcx.caller_expects_out_pointer { + tcx.sess.span_err(call_info.span, + "invalid use of `return_address` intrinsic: function \ + does not use out pointer"); + C_null(Type::i8p(ccx)) + } else { + PointerCast(bcx, llvm::get_param(fcx.llfn, 0), Type::i8p(ccx)) + } } // This requires that atomic intrinsics follow a specific naming pattern: diff --git a/src/test/compile-fail/intrinsic-return-address.rs b/src/test/compile-fail/intrinsic-return-address.rs new file mode 100644 index 0000000000000..9c1db4057c6cd --- /dev/null +++ b/src/test/compile-fail/intrinsic-return-address.rs @@ -0,0 +1,31 @@ +// Copyright 2014 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. + +#![allow(warnings)] +#![feature(intrinsics)] + +extern "rust-intrinsic" { + fn return_address() -> *const u8; +} + +unsafe fn f() { + let _ = return_address(); + //~^ ERROR invalid use of `return_address` intrinsic: function does not use out pointer +} + +unsafe fn g() -> int { + let _ = return_address(); + //~^ ERROR invalid use of `return_address` intrinsic: function does not use out pointer + 0 +} + +fn main() {} + + diff --git a/src/test/run-pass/intrinsic-return-address.rs b/src/test/run-pass/intrinsic-return-address.rs index a28a79d306bbf..91af669340d61 100644 --- a/src/test/run-pass/intrinsic-return-address.rs +++ b/src/test/run-pass/intrinsic-return-address.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(intrinsics)]; +#![feature(intrinsics)] use std::ptr; From 0ad97c042a1ae41652a0157d0ac148e6e90db6c2 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Mon, 11 Aug 2014 15:58:46 -0700 Subject: [PATCH 5/6] librustc: Don't use Load/Store for structural values. --- src/librustc/middle/trans/base.rs | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 9c304f839dbee..22f8107a706bf 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1122,7 +1122,7 @@ pub fn memcpy_ty(bcx: &Block, dst: ValueRef, src: ValueRef, t: ty::t) { let llalign = llalign_of_min(ccx, llty); call_memcpy(bcx, dst, src, llsz, llalign as u32); } else { - Store(bcx, Load(bcx, src), dst); + store_ty(bcx, Load(bcx, src), dst, t); } } @@ -1546,7 +1546,7 @@ pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block, retty: ty::t) { let retslot = Load(ret_cx, fcx.llretslotptr.get().unwrap()); let retptr = Value(retslot); - let retval = match retptr.get_dominating_store(ret_cx) { + 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) => { @@ -1557,21 +1557,28 @@ pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block, retty: ty::t) { retptr.erase_from_parent(); } - if ty::type_is_bool(retty) { + let retval = if ty::type_is_bool(retty) { Trunc(ret_cx, retval, Type::i1(fcx.ccx)) } else { retval + }; + + if fcx.caller_expects_out_pointer { + store_ty(ret_cx, retval, get_param(fcx.llfn, 0), retty); + return RetVoid(ret_cx); + } else { + return Ret(ret_cx, retval); + } + } + // Otherwise, copy the return value to the ret slot + None => { + if fcx.caller_expects_out_pointer { + memcpy_ty(ret_cx, get_param(fcx.llfn, 0), retslot, retty); + return RetVoid(ret_cx); + } else { + return Ret(ret_cx, load_ty(ret_cx, retslot, retty)); } } - // Otherwise, load the return value from the ret slot - None => load_ty(ret_cx, retslot, retty) - }; - - if fcx.caller_expects_out_pointer { - store_ty(ret_cx, retval, get_param(fcx.llfn, 0), retty); - RetVoid(ret_cx); - } else { - Ret(ret_cx, retval); } } From 71e19d528633a470d2d93580651a922e21d7a512 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Mon, 11 Aug 2014 19:16:00 -0700 Subject: [PATCH 6/6] librustc: Don't use an alloca per return if the function doesn't have nested returns. --- src/librustc/middle/trans/base.rs | 136 ++++++++++++++++++++--- src/librustc/middle/trans/callee.rs | 11 +- src/librustc/middle/trans/closure.rs | 7 +- src/librustc/middle/trans/common.rs | 15 +++ src/librustc/middle/trans/controlflow.rs | 5 +- src/librustc/middle/trans/glue.rs | 2 +- src/librustc/middle/trans/reflect.rs | 8 +- 7 files changed, 150 insertions(+), 34 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 22f8107a706bf..0860f84dbef47 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1214,19 +1214,117 @@ pub fn arrayalloca(cx: &Block, ty: Type, v: ValueRef) -> ValueRef { pub fn make_return_slot_pointer(fcx: &FunctionContext, output_type: ty::t) -> ValueRef { let lloutputtype = type_of::type_of(fcx.ccx, output_type); - // Let's create the stack slot - let slot = AllocaFcx(fcx, lloutputtype.ptr_to(), "llretslotptr"); + // We create an alloca to hold a pointer of type `output_type` + // which will hold the pointer to the right alloca which has the + // final ret value + if fcx.needs_ret_allocas { + // Let's create the stack slot + let slot = AllocaFcx(fcx, lloutputtype.ptr_to(), "llretslotptr"); - // and if we're using an out pointer, then store that in our newly made slot - if type_of::return_uses_outptr(fcx.ccx, output_type) { - let outptr = get_param(fcx.llfn, 0); + // and if we're using an out pointer, then store that in our newly made slot + if type_of::return_uses_outptr(fcx.ccx, output_type) { + let outptr = get_param(fcx.llfn, 0); - let b = fcx.ccx.builder(); - b.position_before(fcx.alloca_insert_pt.get().unwrap()); - b.store(outptr, slot); + let b = fcx.ccx.builder(); + b.position_before(fcx.alloca_insert_pt.get().unwrap()); + b.store(outptr, slot); + } + + slot + + // But if there are no nested returns, we skip the indirection and have a single + // retslot + } else { + if type_of::return_uses_outptr(fcx.ccx, output_type) { + get_param(fcx.llfn, 0) + } else { + AllocaFcx(fcx, lloutputtype, "sret_slot") + } } +} - slot +struct CheckForNestedReturnsVisitor { + found: bool +} + +impl Visitor for CheckForNestedReturnsVisitor { + fn visit_expr(&mut self, e: &ast::Expr, in_return: bool) { + match e.node { + ast::ExprRet(..) if in_return => { + self.found = true; + return; + } + ast::ExprRet(..) => visit::walk_expr(self, e, true), + _ => visit::walk_expr(self, e, in_return) + } + } +} + +fn has_nested_returns(tcx: &ty::ctxt, id: ast::NodeId) -> bool { + match tcx.map.find(id) { + Some(ast_map::NodeItem(i)) => { + match i.node { + ast::ItemFn(_, _, _, _, blk) => { + let mut explicit = CheckForNestedReturnsVisitor { found: false }; + let mut implicit = CheckForNestedReturnsVisitor { found: false }; + visit::walk_item(&mut explicit, &*i, false); + visit::walk_expr_opt(&mut implicit, blk.expr, true); + explicit.found || implicit.found + } + _ => tcx.sess.bug("unexpected item variant in has_nested_returns") + } + } + Some(ast_map::NodeTraitMethod(trait_method)) => { + match *trait_method { + ast::Provided(m) => { + match m.node { + ast::MethDecl(_, _, _, _, _, _, blk, _) => { + let mut explicit = CheckForNestedReturnsVisitor { found: false }; + let mut implicit = CheckForNestedReturnsVisitor { found: false }; + visit::walk_method_helper(&mut explicit, &*m, false); + visit::walk_expr_opt(&mut implicit, blk.expr, true); + explicit.found || implicit.found + } + ast::MethMac(_) => tcx.sess.bug("unexpanded macro") + } + } + ast::Required(_) => tcx.sess.bug("unexpected variant: required trait method in \ + has_nested_returns") + } + } + Some(ast_map::NodeMethod(m)) => { + match m.node { + ast::MethDecl(_, _, _, _, _, _, blk, _) => { + let mut explicit = CheckForNestedReturnsVisitor { found: false }; + let mut implicit = CheckForNestedReturnsVisitor { found: false }; + visit::walk_method_helper(&mut explicit, &*m, false); + visit::walk_expr_opt(&mut implicit, blk.expr, true); + explicit.found || implicit.found + } + ast::MethMac(_) => tcx.sess.bug("unexpanded macro") + } + } + Some(ast_map::NodeExpr(e)) => { + match e.node { + ast::ExprFnBlock(_, blk) | ast::ExprProc(_, blk) | ast::ExprUnboxedFn(_, blk) => { + let mut explicit = CheckForNestedReturnsVisitor { found: false }; + let mut implicit = CheckForNestedReturnsVisitor { found: false }; + visit::walk_expr(&mut explicit, &*e, false); + visit::walk_expr_opt(&mut implicit, blk.expr, true); + explicit.found || implicit.found + } + _ => tcx.sess.bug("unexpected expr variant in has_nested_returns") + } + } + + Some(ast_map::NodeVariant(..)) | Some(ast_map::NodeStructCtor(..)) => false, + + // glue, shims, etc + None if id == ast::DUMMY_NODE_ID => false, + + _ => tcx.sess.bug(format!("unexpected variant in has_nested_returns: {}", + tcx.map.path_to_string(id)).as_slice()) + } } // NB: must keep 4 fns in sync: @@ -1261,6 +1359,7 @@ pub fn new_fn_ctxt<'a>(ccx: &'a CrateContext, let substd_output_type = output_type.substp(ccx.tcx(), param_substs); let uses_outptr = type_of::return_uses_outptr(ccx, substd_output_type); let debug_context = debuginfo::create_function_debug_context(ccx, id, param_substs, llfndecl); + let nested_returns = has_nested_returns(ccx.tcx(), id); let mut fcx = FunctionContext { llfn: llfndecl, @@ -1268,6 +1367,7 @@ pub fn new_fn_ctxt<'a>(ccx: &'a CrateContext, llretslotptr: Cell::new(None), alloca_insert_pt: Cell::new(None), llreturn: Cell::new(None), + needs_ret_allocas: nested_returns, personality: Cell::new(None), caller_expects_out_pointer: uses_outptr, llargs: RefCell::new(NodeMap::new()), @@ -1540,11 +1640,16 @@ pub fn finish_fn<'a>(fcx: &'a FunctionContext<'a>, // Builds the return block for a function. pub fn build_return_block(fcx: &FunctionContext, ret_cx: &Block, retty: ty::t) { - if fcx.llretslotptr.get().is_none() { + if fcx.llretslotptr.get().is_none() || + (!fcx.needs_ret_allocas && fcx.caller_expects_out_pointer) { return RetVoid(ret_cx); } - let retslot = Load(ret_cx, fcx.llretslotptr.get().unwrap()); + let retslot = if fcx.needs_ret_allocas { + Load(ret_cx, fcx.llretslotptr.get().unwrap()) + } else { + fcx.llretslotptr.get().unwrap() + }; let retptr = Value(retslot); match retptr.get_dominating_store(ret_cx) { // If there's only a single store to the ret slot, we can directly return @@ -1678,7 +1783,7 @@ pub fn trans_closure(ccx: &CrateContext, debuginfo::start_emitting_source_locations(&fcx); let dest = match fcx.llretslotptr.get() { - Some(_) => expr::SaveIn(alloca(bcx, type_of::type_of(bcx.ccx(), block_ty), "iret_slot")), + Some(_) => expr::SaveIn(fcx.get_ret_slot(bcx, block_ty, "iret_slot")), None => { assert!(type_is_zero_size(bcx.ccx(), block_ty)); expr::Ignore @@ -1692,7 +1797,7 @@ pub fn trans_closure(ccx: &CrateContext, bcx = controlflow::trans_block(bcx, body, dest); match dest { - expr::SaveIn(slot) => { + expr::SaveIn(slot) if fcx.needs_ret_allocas => { Store(bcx, slot, fcx.llretslotptr.get().unwrap()); } _ => {} @@ -1862,12 +1967,14 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext, param_substs, None, &arena, TranslateItems); let bcx = init_function(&fcx, false, result_ty); + assert!(!fcx.needs_ret_allocas); + let arg_tys = ty::ty_fn_args(ctor_ty); let arg_datums = create_datums_for_fn_args(&fcx, arg_tys.as_slice()); if !type_is_zero_size(fcx.ccx, result_ty) { - let dest = alloca(bcx, type_of::type_of(bcx.ccx(), result_ty), "eret_slot"); + let dest = fcx.get_ret_slot(bcx, result_ty, "eret_slot"); let repr = adt::represent_type(ccx, result_ty); for (i, arg_datum) in arg_datums.move_iter().enumerate() { let lldestptr = adt::trans_field_ptr(bcx, @@ -1878,7 +1985,6 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext, arg_datum.store_to(bcx, lldestptr); } adt::trans_set_discr(bcx, &*repr, dest, disr); - Store(bcx, dest, fcx.llretslotptr.get().unwrap()); } finish_fn(&fcx, bcx, result_ty); diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index dd37f3adab9d5..8e6ff0cdb3e11 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -334,7 +334,7 @@ pub fn trans_unboxing_shim(bcx: &Block, let return_type = ty::ty_fn_ret(boxed_function_type); let fcx = new_fn_ctxt(ccx, llfn, - -1, + ast::DUMMY_NODE_ID, false, return_type, &empty_param_substs, @@ -389,8 +389,9 @@ pub fn trans_unboxing_shim(bcx: &Block, for i in range(1, arg_types.len()) { llshimmedargs.push(get_param(fcx.llfn, fcx.arg_pos(i) as u32)); } + assert!(!fcx.needs_ret_allocas); let dest = match fcx.llretslotptr.get() { - Some(_) => Some(expr::SaveIn(alloca(bcx, type_of::type_of(ccx, return_type), "ret_slot"))), + Some(_) => Some(expr::SaveIn(fcx.get_ret_slot(bcx, return_type, "ret_slot"))), None => None }; bcx = trans_call_inner(bcx, @@ -404,12 +405,6 @@ pub fn trans_unboxing_shim(bcx: &Block, }, ArgVals(llshimmedargs.as_slice()), dest).bcx; - match dest { - Some(expr::SaveIn(slot)) => { - Store(bcx, slot, fcx.llretslotptr.get().unwrap()); - } - _ => {} - } bcx = fcx.pop_and_trans_custom_cleanup_scope(bcx, arg_scope); finish_fn(&fcx, bcx, return_type); diff --git a/src/librustc/middle/trans/closure.rs b/src/librustc/middle/trans/closure.rs index dc77edf270638..eeff0609a56dd 100644 --- a/src/librustc/middle/trans/closure.rs +++ b/src/librustc/middle/trans/closure.rs @@ -574,7 +574,7 @@ pub fn get_wrapper_for_bare_fn(ccx: &CrateContext, let arena = TypedArena::new(); let empty_param_substs = param_substs::empty(); - let fcx = new_fn_ctxt(ccx, llfn, -1, true, f.sig.output, + let fcx = new_fn_ctxt(ccx, llfn, ast::DUMMY_NODE_ID, true, f.sig.output, &empty_param_substs, None, &arena, TranslateItems); let bcx = init_function(&fcx, true, f.sig.output); @@ -583,8 +583,9 @@ pub fn get_wrapper_for_bare_fn(ccx: &CrateContext, .as_slice()); let mut llargs = Vec::new(); match fcx.llretslotptr.get() { - Some(llretslotptr) => { - llargs.push(Load(bcx, llretslotptr)); + Some(llretptr) => { + assert!(!fcx.needs_ret_allocas); + llargs.push(llretptr); } None => {} } diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index 6aca8fe8e4111..5e46550eccba5 100644 --- a/src/librustc/middle/trans/common.rs +++ b/src/librustc/middle/trans/common.rs @@ -21,11 +21,13 @@ use middle::def; use middle::lang_items::LangItem; use middle::subst; use middle::subst::Subst; +use middle::trans::base; use middle::trans::build; use middle::trans::cleanup; use middle::trans::datum; use middle::trans::debuginfo; use middle::trans::type_::Type; +use middle::trans::type_of; use middle::ty; use middle::typeck; use util::ppaux::Repr; @@ -254,6 +256,11 @@ pub struct FunctionContext<'a> { pub alloca_insert_pt: Cell>, pub llreturn: Cell>, + // If the function has any nested return's, including something like: + // fn foo() -> Option { Some(Foo { x: return None }) }, then + // we use a separate alloca for each return + pub needs_ret_allocas: bool, + // The a value alloca'd for calls to upcalls.rust_personality. Used when // outputting the resume instruction. pub personality: Cell>, @@ -345,6 +352,14 @@ impl<'a> FunctionContext<'a> { self.llreturn.get().unwrap() } + pub fn get_ret_slot(&self, bcx: &Block, ty: ty::t, name: &str) -> ValueRef { + if self.needs_ret_allocas { + base::alloca_no_lifetime(bcx, type_of::type_of(bcx.ccx(), ty), name) + } else { + self.llretslotptr.get().unwrap() + } + } + pub fn new_block(&'a self, is_lpad: bool, name: &str, diff --git a/src/librustc/middle/trans/controlflow.rs b/src/librustc/middle/trans/controlflow.rs index b9644b1145327..02a5f715ecb23 100644 --- a/src/librustc/middle/trans/controlflow.rs +++ b/src/librustc/middle/trans/controlflow.rs @@ -26,7 +26,6 @@ use middle::trans::debuginfo; use middle::trans::expr; use middle::trans::meth; use middle::trans::type_::Type; -use middle::trans::type_of; use middle::ty; use middle::typeck::MethodCall; use util::ppaux::Repr; @@ -466,7 +465,7 @@ pub fn trans_ret<'a>(bcx: &'a Block<'a>, let dest = match (fcx.llretslotptr.get(), e) { (Some(_), Some(e)) => { let ret_ty = expr_ty(bcx, &*e); - expr::SaveIn(alloca(bcx, type_of::type_of(bcx.ccx(), ret_ty), "ret_slot")) + expr::SaveIn(fcx.get_ret_slot(bcx, ret_ty, "ret_slot")) } _ => expr::Ignore, }; @@ -474,7 +473,7 @@ pub fn trans_ret<'a>(bcx: &'a Block<'a>, Some(x) => { bcx = expr::trans_into(bcx, &*x, dest); match dest { - expr::SaveIn(slot) => { + expr::SaveIn(slot) if fcx.needs_ret_allocas => { Store(bcx, slot, fcx.llretslotptr.get().unwrap()); } _ => {} diff --git a/src/librustc/middle/trans/glue.rs b/src/librustc/middle/trans/glue.rs index 40288a33930e5..1d016f6db2659 100644 --- a/src/librustc/middle/trans/glue.rs +++ b/src/librustc/middle/trans/glue.rs @@ -467,7 +467,7 @@ fn make_generic_glue(ccx: &CrateContext, let arena = TypedArena::new(); let empty_param_substs = param_substs::empty(); - let fcx = new_fn_ctxt(ccx, llfn, -1, false, ty::mk_nil(), + let fcx = new_fn_ctxt(ccx, llfn, ast::DUMMY_NODE_ID, false, ty::mk_nil(), &empty_param_substs, None, &arena, TranslateItems); let bcx = init_function(&fcx, false, ty::mk_nil()); diff --git a/src/librustc/middle/trans/reflect.rs b/src/librustc/middle/trans/reflect.rs index 35ac0b2707195..e49d483e8d93d 100644 --- a/src/librustc/middle/trans/reflect.rs +++ b/src/librustc/middle/trans/reflect.rs @@ -310,7 +310,7 @@ impl<'a, 'b> Reflector<'a, 'b> { sym.as_slice()); let arena = TypedArena::new(); let empty_param_substs = param_substs::empty(); - let fcx = new_fn_ctxt(ccx, llfdecl, -1, false, + let fcx = new_fn_ctxt(ccx, llfdecl, ast::DUMMY_NODE_ID, false, ty::mk_u64(), &empty_param_substs, None, &arena, TranslateItems); let bcx = init_function(&fcx, false, ty::mk_u64()); @@ -321,9 +321,9 @@ impl<'a, 'b> Reflector<'a, 'b> { let arg = get_param(llfdecl, fcx.arg_pos(0u) as c_uint); let arg = BitCast(bcx, arg, llptrty); let ret = adt::trans_get_discr(bcx, &*repr, arg, Some(Type::i64(ccx))); - let ret_alloca = alloca(bcx, Type::i64(ccx), "ret_slot"); - Store(bcx, ret, ret_alloca); - Store(bcx, ret_alloca, fcx.llretslotptr.get().unwrap()); + assert!(!fcx.needs_ret_allocas); + let ret_slot = fcx.get_ret_slot(bcx, ty::mk_u64(), "ret_slot"); + Store(bcx, ret, ret_slot); match fcx.llreturn.get() { Some(llreturn) => Br(bcx, llreturn), None => {}