From 4a485f8cec524c8f3f57e4fd3248d5093ed3dc5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Mon, 8 Jul 2013 07:42:56 +0200 Subject: [PATCH 1/2] Avoid unused allocas for immediate return values There's no need to allocate a return slot for anykind of immediate return value, not just not for nils. Also, when the return value is ignored, we only have to copy it to a temporary alloca if it's actually required to call drop_ty on it. --- src/librustc/middle/trans/callee.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 05fe0bed3b69..2a5e8f2ddc06 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -672,15 +672,8 @@ pub fn trans_call_inner(in_cx: block, expr::Ignore => { // drop the value if it is not being saved. unsafe { - if llvm::LLVMIsUndef(llretslot) != lib::llvm::True { - if ty::type_is_nil(ret_ty) { - // When implementing the for-loop sugar syntax, the - // type of the for-loop is nil, but the function - // it's invoking returns a bool. This is a special - // case to ignore instead of invoking the Store - // below into a scratch pointer of a mismatched - // type. - } else if ty::type_is_immediate(bcx.tcx(), ret_ty) { + if ty::type_needs_drop(bcx.tcx(), ret_ty) { + if ty::type_is_immediate(bcx.tcx(), ret_ty) { let llscratchptr = alloc_ty(bcx, ret_ty); Store(bcx, llresult, llscratchptr); bcx = glue::drop_ty(bcx, llscratchptr, ret_ty); @@ -734,7 +727,7 @@ pub fn trans_ret_slot(bcx: block, fn_ty: ty::t, dest: expr::Dest) match dest { expr::SaveIn(dst) => dst, expr::Ignore => { - if ty::type_is_nil(retty) { + if ty::type_is_immediate(bcx.tcx(), retty) { unsafe { llvm::LLVMGetUndef(Type::nil().ptr_to().to_ref()) } From 00ba8b3ac0692293511858cae6fe1a3e1dcc316b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Mon, 8 Jul 2013 08:12:01 +0200 Subject: [PATCH 2/2] Improve handling of immediate return values We currently still handle immediate return values a lot like non-immediate ones. We provide a slot for them and store them into memory, often just to immediately load them again. To improve this situation, trans_call_inner has to return a Result which contains the immediate return value. Also, it also needs to accept "No destination" in addition to just SaveIn and Ignore. Since "No destination" isn't something that fits well into the Dest type, I've chosen to simply use Option instead, paired with an assertion that checks that "None" is only allowed for immediate return values. --- src/librustc/middle/trans/_match.rs | 10 ++----- src/librustc/middle/trans/base.rs | 21 +++++++-------- src/librustc/middle/trans/callee.rs | 33 ++++++++++++------------ src/librustc/middle/trans/closure.rs | 8 +++--- src/librustc/middle/trans/controlflow.rs | 4 +-- src/librustc/middle/trans/expr.rs | 4 +-- src/librustc/middle/trans/foreign.rs | 2 +- src/librustc/middle/trans/glue.rs | 4 +-- src/librustc/middle/trans/reflect.rs | 9 +++---- src/librustc/middle/trans/tvec.rs | 2 +- src/librustc/middle/trans/write_guard.rs | 16 ++++++------ 11 files changed, 51 insertions(+), 62 deletions(-) diff --git a/src/librustc/middle/trans/_match.rs b/src/librustc/middle/trans/_match.rs index 80654342d0de..b7168cbfdec5 100644 --- a/src/librustc/middle/trans/_match.rs +++ b/src/librustc/middle/trans/_match.rs @@ -1095,26 +1095,20 @@ pub fn compare_values(cx: block, match ty::get(rhs_t).sty { ty::ty_estr(ty::vstore_uniq) => { - let scratch_result = scratch_datum(cx, ty::mk_bool(), false); let scratch_lhs = alloca(cx, val_ty(lhs)); Store(cx, lhs, scratch_lhs); let scratch_rhs = alloca(cx, val_ty(rhs)); Store(cx, rhs, scratch_rhs); let did = cx.tcx().lang_items.uniq_str_eq_fn(); - let bcx = callee::trans_lang_call(cx, did, [scratch_lhs, scratch_rhs], - expr::SaveIn(scratch_result.val)); - let result = scratch_result.to_result(bcx); + let result = callee::trans_lang_call(cx, did, [scratch_lhs, scratch_rhs], None); Result { bcx: result.bcx, val: bool_to_i1(result.bcx, result.val) } } ty::ty_estr(_) => { - let scratch_result = scratch_datum(cx, ty::mk_bool(), false); let did = cx.tcx().lang_items.str_eq_fn(); - let bcx = callee::trans_lang_call(cx, did, [lhs, rhs], - expr::SaveIn(scratch_result.val)); - let result = scratch_result.to_result(bcx); + let result = callee::trans_lang_call(cx, did, [lhs, rhs], None); Result { bcx: result.bcx, val: bool_to_i1(result.bcx, result.val) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index e2795f77d7d8..577f1c689606 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -294,13 +294,12 @@ pub fn malloc_raw_dyn(bcx: block, let llalign = llalign_of_min(ccx, llty_value); // Allocate space: - let rval = alloca(bcx, Type::i8p()); - let bcx = callee::trans_lang_call( + let r = callee::trans_lang_call( bcx, bcx.tcx().lang_items.exchange_malloc_fn(), [C_i32(llalign as i32), size], - expr::SaveIn(rval)); - rslt(bcx, PointerCast(bcx, Load(bcx, rval), llty_value.ptr_to())) + None); + rslt(r.bcx, PointerCast(r.bcx, r.val, llty_value.ptr_to())) } else if heap == heap_exchange_vector { // Grab the TypeRef type of box_ptr_ty. let element_type = match ty::get(t).sty { @@ -314,13 +313,12 @@ pub fn malloc_raw_dyn(bcx: block, let llalign = llalign_of_min(ccx, llty_value); // Allocate space: - let rval = alloca(bcx, Type::i8p()); - let bcx = callee::trans_lang_call( + let r = callee::trans_lang_call( bcx, bcx.tcx().lang_items.vector_exchange_malloc_fn(), [C_i32(llalign as i32), size], - expr::SaveIn(rval)); - rslt(bcx, PointerCast(bcx, Load(bcx, rval), llty)) + None); + rslt(r.bcx, PointerCast(r.bcx, r.val, llty)) } else { // we treat ~fn, @fn and @[] as @ here, which isn't ideal let (mk_fn, langcall) = match heap { @@ -343,13 +341,12 @@ pub fn malloc_raw_dyn(bcx: block, // Allocate space: let tydesc = PointerCast(bcx, static_ti.tydesc, Type::i8p()); - let rval = alloca(bcx, Type::i8p()); - let bcx = callee::trans_lang_call( + let r = callee::trans_lang_call( bcx, langcall, [tydesc, size], - expr::SaveIn(rval)); - let r = rslt(bcx, PointerCast(bcx, Load(bcx, rval), llty)); + None); + let r = rslt(r.bcx, PointerCast(r.bcx, r.val, llty)); maybe_set_managed_unique_rc(r.bcx, r.val, heap); r } diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 2a5e8f2ddc06..473afda48e65 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -446,8 +446,8 @@ pub fn trans_call(in_cx: block, node_id_type(in_cx, id), |cx| trans(cx, f), args, - dest, - DontAutorefArg) + Some(dest), + DontAutorefArg).bcx } pub fn trans_method_call(in_cx: block, @@ -484,15 +484,15 @@ pub fn trans_method_call(in_cx: block, } }, args, - dest, - DontAutorefArg) + Some(dest), + DontAutorefArg).bcx } pub fn trans_lang_call(bcx: block, did: ast::def_id, args: &[ValueRef], - dest: expr::Dest) - -> block { + dest: Option) + -> Result { let fty = if did.crate == ast::local_crate { ty::node_id_to_type(bcx.ccx().tcx, did.node) } else { @@ -552,7 +552,7 @@ pub fn trans_lang_call_with_type_params(bcx: block, } Callee { bcx: callee.bcx, data: Fn(FnData { llfn: new_llval }) } }, - ArgVals(args), dest, DontAutorefArg); + ArgVals(args), Some(dest), DontAutorefArg).bcx; } pub fn body_contains_ret(body: &ast::blk) -> bool { @@ -579,10 +579,10 @@ pub fn trans_call_inner(in_cx: block, ret_ty: ty::t, get_callee: &fn(block) -> Callee, args: CallArgs, - dest: expr::Dest, + dest: Option, autoref_arg: AutorefArg) - -> block { - do base::with_scope(in_cx, call_info, "call") |cx| { + -> Result { + do base::with_scope_result(in_cx, call_info, "call") |cx| { let ret_in_loop = match args { ArgExprs(args) => { args.len() > 0u && match args.last().node { @@ -669,7 +669,8 @@ pub fn trans_call_inner(in_cx: block, bcx = new_bcx; match dest { - expr::Ignore => { + None => { assert!(ty::type_is_immediate(bcx.tcx(), ret_ty)) } + Some(expr::Ignore) => { // drop the value if it is not being saved. unsafe { if ty::type_needs_drop(bcx.tcx(), ret_ty) { @@ -683,7 +684,7 @@ pub fn trans_call_inner(in_cx: block, } } } - expr::SaveIn(lldest) => { + Some(expr::SaveIn(lldest)) => { // If this is an immediate, store into the result location. // (If this was not an immediate, the result will already be // directly written into the output slot.) @@ -710,7 +711,7 @@ pub fn trans_call_inner(in_cx: block, bcx } } - bcx + rslt(bcx, llresult) } } @@ -720,13 +721,13 @@ pub enum CallArgs<'self> { ArgVals(&'self [ValueRef]) } -pub fn trans_ret_slot(bcx: block, fn_ty: ty::t, dest: expr::Dest) +pub fn trans_ret_slot(bcx: block, fn_ty: ty::t, dest: Option) -> ValueRef { let retty = ty::ty_fn_ret(fn_ty); match dest { - expr::SaveIn(dst) => dst, - expr::Ignore => { + Some(expr::SaveIn(dst)) => dst, + _ => { if ty::type_is_immediate(bcx.tcx(), retty) { unsafe { llvm::LLVMGetUndef(Type::nil().ptr_to().to_ref()) diff --git a/src/librustc/middle/trans/closure.rs b/src/librustc/middle/trans/closure.rs index 3478925753e2..4c63b8dc8445 100644 --- a/src/librustc/middle/trans/closure.rs +++ b/src/librustc/middle/trans/closure.rs @@ -531,13 +531,13 @@ pub fn make_opaque_cbox_take_glue( // Allocate memory, update original ptr, and copy existing data let opaque_tydesc = PointerCast(bcx, tydesc, Type::i8p()); - let rval = alloca(bcx, Type::i8p()); - let bcx = callee::trans_lang_call( + let mut bcx = bcx; + let llresult = unpack_result!(bcx, callee::trans_lang_call( bcx, bcx.tcx().lang_items.closure_exchange_malloc_fn(), [opaque_tydesc, sz], - expr::SaveIn(rval)); - let cbox_out = PointerCast(bcx, Load(bcx, rval), llopaquecboxty); + None)); + let cbox_out = PointerCast(bcx, llresult, llopaquecboxty); call_memcpy(bcx, cbox_out, cbox_in, sz, 1); Store(bcx, cbox_out, cboxptr); diff --git a/src/librustc/middle/trans/controlflow.rs b/src/librustc/middle/trans/controlflow.rs index db6a954ee911..8ca4253ead8b 100644 --- a/src/librustc/middle/trans/controlflow.rs +++ b/src/librustc/middle/trans/controlflow.rs @@ -395,7 +395,7 @@ fn trans_fail_value(bcx: block, let V_filename = PointerCast(bcx, V_filename, Type::i8p()); let args = ~[V_str, V_filename, C_int(ccx, V_line)]; let bcx = callee::trans_lang_call( - bcx, bcx.tcx().lang_items.fail_fn(), args, expr::Ignore); + bcx, bcx.tcx().lang_items.fail_fn(), args, Some(expr::Ignore)).bcx; Unreachable(bcx); return bcx; } @@ -406,7 +406,7 @@ pub fn trans_fail_bounds_check(bcx: block, sp: span, let (filename, line) = filename_and_line_num_from_span(bcx, sp); let args = ~[filename, line, index, len]; let bcx = callee::trans_lang_call( - bcx, bcx.tcx().lang_items.fail_bounds_check_fn(), args, expr::Ignore); + bcx, bcx.tcx().lang_items.fail_bounds_check_fn(), args, Some(expr::Ignore)).bcx; Unreachable(bcx); return bcx; } diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs index 4cf3873ee706..0180eeb3d220 100644 --- a/src/librustc/middle/trans/expr.rs +++ b/src/librustc/middle/trans/expr.rs @@ -1551,8 +1551,8 @@ fn trans_overloaded_op(bcx: block, origin) }, callee::ArgExprs(args), - dest, - DoAutorefArg) + Some(dest), + DoAutorefArg).bcx } fn int_cast(bcx: block, lldsttype: Type, llsrctype: Type, diff --git a/src/librustc/middle/trans/foreign.rs b/src/librustc/middle/trans/foreign.rs index 365681924544..2c505853d5eb 100644 --- a/src/librustc/middle/trans/foreign.rs +++ b/src/librustc/middle/trans/foreign.rs @@ -789,7 +789,7 @@ pub fn trans_intrinsic(ccx: @mut CrateContext, bcx = trans_call_inner( bcx, None, fty, ty::mk_nil(), |bcx| Callee {bcx: bcx, data: Closure(datum)}, - ArgVals(arg_vals), Ignore, DontAutorefArg); + ArgVals(arg_vals), Some(Ignore), DontAutorefArg).bcx; } "morestack_addr" => { // XXX This is a hack to grab the address of this particular diff --git a/src/librustc/middle/trans/glue.rs b/src/librustc/middle/trans/glue.rs index 682a4f133c08..bc493bfa23ec 100644 --- a/src/librustc/middle/trans/glue.rs +++ b/src/librustc/middle/trans/glue.rs @@ -47,7 +47,7 @@ pub fn trans_free(cx: block, v: ValueRef) -> block { callee::trans_lang_call(cx, cx.tcx().lang_items.free_fn(), [PointerCast(cx, v, Type::i8p())], - expr::Ignore) + Some(expr::Ignore)).bcx } pub fn trans_exchange_free(cx: block, v: ValueRef) -> block { @@ -55,7 +55,7 @@ pub fn trans_exchange_free(cx: block, v: ValueRef) -> block { callee::trans_lang_call(cx, cx.tcx().lang_items.exchange_free_fn(), [PointerCast(cx, v, Type::i8p())], - expr::Ignore) + Some(expr::Ignore)).bcx } pub fn take_ty(cx: block, v: ValueRef, t: ty::t) -> block { diff --git a/src/librustc/middle/trans/reflect.rs b/src/librustc/middle/trans/reflect.rs index de91993f345a..cc4111aa1947 100644 --- a/src/librustc/middle/trans/reflect.rs +++ b/src/librustc/middle/trans/reflect.rs @@ -17,7 +17,6 @@ use middle::trans::callee::{ArgVals, DontAutorefArg}; use middle::trans::callee; use middle::trans::common::*; use middle::trans::datum::*; -use middle::trans::expr::SaveIn; use middle::trans::glue; use middle::trans::machine; use middle::trans::meth; @@ -96,14 +95,13 @@ impl Reflector { ty::mk_bare_fn(tcx, copy self.visitor_methods[mth_idx].fty); let v = self.visitor_val; debug!("passing %u args:", args.len()); - let bcx = self.bcx; + let mut bcx = self.bcx; for args.iter().enumerate().advance |(i, a)| { debug!("arg %u: %s", i, bcx.val_to_str(*a)); } let bool_ty = ty::mk_bool(); - let scratch = scratch_datum(bcx, bool_ty, false); // XXX: Should not be BoxTraitStore! - let bcx = callee::trans_call_inner( + let result = unpack_result!(bcx, callee::trans_call_inner( self.bcx, None, mth_ty, bool_ty, |bcx| meth::trans_trait_callee_from_llval(bcx, mth_ty, @@ -113,8 +111,7 @@ impl Reflector { ast::sty_region( None, ast::m_imm)), - ArgVals(args), SaveIn(scratch.val), DontAutorefArg); - let result = scratch.to_value_llval(bcx); + ArgVals(args), None, DontAutorefArg)); let result = bool_to_i1(bcx, result); let next_bcx = sub_block(bcx, "next"); CondBr(bcx, result, next_bcx.llbb, self.final_bcx.llbb); diff --git a/src/librustc/middle/trans/tvec.rs b/src/librustc/middle/trans/tvec.rs index 809838ded0ab..41dbe320d2d1 100644 --- a/src/librustc/middle/trans/tvec.rs +++ b/src/librustc/middle/trans/tvec.rs @@ -337,7 +337,7 @@ pub fn trans_uniq_or_managed_vstore(bcx: block, heap: heap, vstore_expr: @ast::e bcx, bcx.tcx().lang_items.strdup_uniq_fn(), [ llptrval, llsizeval ], - expr::SaveIn(lldestval.to_ref_llval(bcx))); + Some(expr::SaveIn(lldestval.to_ref_llval(bcx)))).bcx; return DatumBlock { bcx: bcx, datum: lldestval diff --git a/src/librustc/middle/trans/write_guard.rs b/src/librustc/middle/trans/write_guard.rs index 0db770b6c8bc..bd22e41aff8f 100644 --- a/src/librustc/middle/trans/write_guard.rs +++ b/src/librustc/middle/trans/write_guard.rs @@ -81,7 +81,7 @@ pub fn return_to_mut(mut bcx: block, filename_val, line_val ], - expr::Ignore); + Some(expr::Ignore)).bcx; } callee::trans_lang_call( @@ -93,8 +93,8 @@ pub fn return_to_mut(mut bcx: block, filename_val, line_val ], - expr::Ignore - ) + Some(expr::Ignore) + ).bcx } fn root(datum: &Datum, @@ -144,7 +144,7 @@ fn root(datum: &Datum, let box_ptr = Load(bcx, PointerCast(bcx, scratch.val, Type::i8p().ptr_to())); - bcx = callee::trans_lang_call( + let llresult = unpack_result!(bcx, callee::trans_lang_call( bcx, freeze_did, [ @@ -152,7 +152,7 @@ fn root(datum: &Datum, filename, line ], - expr::SaveIn(scratch_bits.val)); + Some(expr::SaveIn(scratch_bits.val)))); if bcx.tcx().sess.debug_borrows() { bcx = callee::trans_lang_call( @@ -160,11 +160,11 @@ fn root(datum: &Datum, bcx.tcx().lang_items.record_borrow_fn(), [ box_ptr, - Load(bcx, scratch_bits.val), + llresult, filename, line ], - expr::Ignore); + Some(expr::Ignore)).bcx; } add_clean_return_to_mut( @@ -188,5 +188,5 @@ fn perform_write_guard(datum: &Datum, bcx, bcx.tcx().lang_items.check_not_borrowed_fn(), [PointerCast(bcx, llval, Type::i8p()), filename, line], - expr::Ignore) + Some(expr::Ignore)).bcx }