From e4d4fa3295bf5bfd8a798557d5cf0158ca453f84 Mon Sep 17 00:00:00 2001 From: James Miller Date: Mon, 4 Apr 2016 19:21:27 +1200 Subject: [PATCH 1/2] Handle operand temps for function calls This allows temporary destinations for function calls to have their allocas omitted. --- src/librustc/middle/region.rs | 5 +- src/librustc/mir/visit.rs | 7 +- src/librustc_trans/mir/analyze.rs | 1 + src/librustc_trans/mir/block.rs | 192 +++++++++++++++++++++++------- src/librustc_trans/mir/lvalue.rs | 31 +++++ 5 files changed, 191 insertions(+), 45 deletions(-) diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index 2cde6ce93208f..5fe70608ec6d3 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -45,8 +45,9 @@ impl fmt::Debug for CodeExtent { ty::tls::with_opt(|opt_tcx| { if let Some(tcx) = opt_tcx { - let data = tcx.region_maps.code_extents.borrow()[self.0 as usize]; - write!(f, "/{:?}", data)?; + if let Some(data) = tcx.region_maps.code_extents.borrow().get(self.0 as usize) { + write!(f, "/{:?}", data)?; + } } Ok(()) })?; diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 403c749fe4bcb..66c7c4e41c132 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -407,7 +407,7 @@ macro_rules! make_mir_visitor { self.visit_operand(arg); } if let Some((ref $($mutability)* destination, target)) = *destination { - self.visit_lvalue(destination, LvalueContext::Store); + self.visit_lvalue(destination, LvalueContext::Call); self.visit_branch(block, target); } cleanup.map(|t| self.visit_branch(block, t)); @@ -692,9 +692,12 @@ make_mir_visitor!(MutVisitor,mut); #[derive(Copy, Clone, Debug)] pub enum LvalueContext { - // Appears as LHS of an assignment or as dest of a call + // Appears as LHS of an assignment Store, + // Dest of a call + Call, + // Being dropped Drop, diff --git a/src/librustc_trans/mir/analyze.rs b/src/librustc_trans/mir/analyze.rs index 9aa3d6c7dd08e..9b7b55842ccd1 100644 --- a/src/librustc_trans/mir/analyze.rs +++ b/src/librustc_trans/mir/analyze.rs @@ -105,6 +105,7 @@ impl<'tcx> Visitor<'tcx> for TempAnalyzer { match *lvalue { mir::Lvalue::Temp(index) => { match context { + LvalueContext::Call | LvalueContext::Consume => { } LvalueContext::Store | diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index 3fabdd8fd4226..b8417f985650c 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -11,7 +11,7 @@ use llvm::{self, BasicBlockRef, ValueRef, OperandBundleDef}; use rustc::ty; use rustc::mir::repr as mir; -use abi::{Abi, FnType}; +use abi::{Abi, FnType, ArgType}; use adt; use base; use build; @@ -25,7 +25,7 @@ use type_of; use glue; use type_::Type; -use super::{MirContext, drop}; +use super::{MirContext, TempRef, drop}; use super::lvalue::{LvalueRef, load_fat_ptr}; use super::operand::OperandRef; use super::operand::OperandValue::{self, FatPtr, Immediate, Ref}; @@ -191,25 +191,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { if intrinsic == Some("transmute") { let &(ref dest, target) = destination.as_ref().unwrap(); - let dst = self.trans_lvalue(&bcx, dest); - let mut val = self.trans_operand(&bcx, &args[0]); - if let ty::TyFnDef(def_id, substs, _) = val.ty.sty { - let llouttype = type_of::type_of(bcx.ccx(), dst.ty.to_ty(bcx.tcx())); - let out_type_size = llbitsize_of_real(bcx.ccx(), llouttype); - if out_type_size != 0 { - // FIXME #19925 Remove this hack after a release cycle. - let f = Callee::def(bcx.ccx(), def_id, substs); - let datum = f.reify(bcx.ccx()); - val = OperandRef { - val: OperandValue::Immediate(datum.val), - ty: datum.ty - }; - } - } + self.with_lvalue_ref(&bcx, dest, |this, dest| { + this.trans_transmute(&bcx, &args[0], dest); + }); - let llty = type_of::type_of(bcx.ccx(), val.ty); - let cast_ptr = bcx.pointercast(dst.llval, llty.ptr_to()); - self.store_operand(&bcx, cast_ptr, val); self.set_operand_dropped(&bcx, &args[0]); funclet_br(bcx, self.llblock(target)); return; @@ -227,17 +212,71 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { // Prepare the return value destination let ret_dest = if let Some((ref d, _)) = *destination { - let dest = self.trans_lvalue(&bcx, d); - if fn_ty.ret.is_indirect() { - llargs.push(dest.llval); - None - } else if fn_ty.ret.is_ignore() { - None - } else { - Some(dest) + match *d { + // Handle temporary lvalues, specifically Operand ones, as + // they don't have allocas + mir::Lvalue::Temp(idx) => { + let lvalue_ty = self.mir.lvalue_ty(bcx.tcx(), d); + let ret_ty = lvalue_ty.to_ty(bcx.tcx()); + match self.temps[idx as usize] { + TempRef::Lvalue(dest) => { + if fn_ty.ret.is_indirect() { + llargs.push(dest.llval); + ReturnDest::Nothing + } else if fn_ty.ret.is_ignore() { + ReturnDest::Nothing + } else { + ReturnDest::Store(dest.llval) + } + } + TempRef::Operand(None) => { + let is_intrinsic = if let Intrinsic = callee.data { + true + } else { + false + }; + + if fn_ty.ret.is_indirect() { + // Odd, but possible, case, we have an operand temporary, + // but the calling convention has an indirect return. + let tmp = bcx.with_block(|bcx| { + base::alloc_ty(bcx, ret_ty, "tmp_ret") + }); + llargs.push(tmp); + ReturnDest::IndirectOperand(tmp, idx) + } else if is_intrinsic { + // Currently, intrinsics always need a location to store + // the result. so we create a temporary alloca for the + // result + let tmp = bcx.with_block(|bcx| { + base::alloc_ty(bcx, ret_ty, "tmp_ret") + }); + ReturnDest::IndirectOperand(tmp, idx) + } else if fn_ty.ret.is_ignore() { + ReturnDest::Nothing + } else { + ReturnDest::DirectOperand(idx) + } + } + TempRef::Operand(Some(_)) => { + bug!("lvalue temp already assigned to"); + } + } + } + _ => { + let dest = self.trans_lvalue(&bcx, d); + if fn_ty.ret.is_indirect() { + llargs.push(dest.llval); + ReturnDest::Nothing + } else if fn_ty.ret.is_ignore() { + ReturnDest::Nothing + } else { + ReturnDest::Store(dest.llval) + } + } } } else { - None + ReturnDest::Nothing }; // Split the rust-call tupled arguments off. @@ -269,12 +308,15 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { use expr::{Ignore, SaveIn}; use intrinsic::trans_intrinsic_call; - let (dest, llargs) = if fn_ty.ret.is_indirect() { - (SaveIn(llargs[0]), &llargs[1..]) - } else if let Some(dest) = ret_dest { - (SaveIn(dest.llval), &llargs[..]) - } else { - (Ignore, &llargs[..]) + let (dest, llargs) = match ret_dest { + _ if fn_ty.ret.is_indirect() => { + (SaveIn(llargs[0]), &llargs[1..]) + } + ReturnDest::Nothing => (Ignore, &llargs[..]), + ReturnDest::IndirectOperand(dst, _) | + ReturnDest::Store(dst) => (SaveIn(dst), &llargs[..]), + ReturnDest::DirectOperand(_) => + bug!("Cannot use direct operand with an intrinsic call") }; bcx.with_block(|bcx| { @@ -292,6 +334,16 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { // bcx.unreachable(); } }); + + if let ReturnDest::IndirectOperand(dst, _) = ret_dest { + // Make a fake operand for store_return + let op = OperandRef { + val: OperandValue::Ref(dst), + ty: sig.0.output.unwrap() + }; + self.store_return(&bcx, ret_dest, fn_ty.ret, op); + } + return; } Fn(f) => f, @@ -321,9 +373,11 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { if destination.is_some() { let ret_bcx = ret_bcx.build(); ret_bcx.at_start(|ret_bcx| { - if let Some(ret_dest) = ret_dest { - fn_ty.ret.store(&ret_bcx, invokeret, ret_dest.llval); - } + let op = OperandRef { + val: OperandValue::Immediate(invokeret), + ty: sig.0.output.unwrap() + }; + self.store_return(&ret_bcx, ret_dest, fn_ty.ret, op); for op in args { self.set_operand_dropped(&ret_bcx, op); } @@ -333,9 +387,11 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { let llret = bcx.call(fn_ptr, &llargs, cleanup_bundle.as_ref()); fn_ty.apply_attrs_callsite(llret); if let Some((_, target)) = *destination { - if let Some(ret_dest) = ret_dest { - fn_ty.ret.store(&bcx, llret, ret_dest.llval); - } + let op = OperandRef { + val: OperandValue::Immediate(llret), + ty: sig.0.output.unwrap() + }; + self.store_return(&bcx, ret_dest, fn_ty.ret, op); for op in args { self.set_operand_dropped(&bcx, op); } @@ -544,4 +600,58 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { pub fn llblock(&self, bb: mir::BasicBlock) -> BasicBlockRef { self.blocks[bb.index()].llbb } + + fn trans_transmute(&mut self, bcx: &BlockAndBuilder<'bcx, 'tcx>, + src: &mir::Operand<'tcx>, dst: LvalueRef<'tcx>) { + let mut val = self.trans_operand(bcx, src); + if let ty::TyFnDef(def_id, substs, _) = val.ty.sty { + let llouttype = type_of::type_of(bcx.ccx(), dst.ty.to_ty(bcx.tcx())); + let out_type_size = llbitsize_of_real(bcx.ccx(), llouttype); + if out_type_size != 0 { + // FIXME #19925 Remove this hack after a release cycle. + let f = Callee::def(bcx.ccx(), def_id, substs); + let datum = f.reify(bcx.ccx()); + val = OperandRef { + val: OperandValue::Immediate(datum.val), + ty: datum.ty + }; + } + } + + let llty = type_of::type_of(bcx.ccx(), val.ty); + let cast_ptr = bcx.pointercast(dst.llval, llty.ptr_to()); + self.store_operand(bcx, cast_ptr, val); + } + + // Stores the return value of a function call into it's final location. + fn store_return(&mut self, + bcx: &BlockAndBuilder<'bcx, 'tcx>, + dest: ReturnDest, + ret_ty: ArgType, + op: OperandRef<'tcx>) { + use self::ReturnDest::*; + + match dest { + Nothing => (), + Store(dst) => ret_ty.store(bcx, op.immediate(), dst), + IndirectOperand(tmp, idx) => { + let op = self.trans_load(bcx, tmp, op.ty); + self.temps[idx as usize] = TempRef::Operand(Some(op)); + } + DirectOperand(idx) => { + self.temps[idx as usize] = TempRef::Operand(Some(op)); + } + } + } +} + +enum ReturnDest { + // Do nothing, the return value is indirect or ignored + Nothing, + // Store the return value to the pointer + Store(ValueRef), + // Stores an indirect return value to an operand temporary lvalue + IndirectOperand(ValueRef, u32), + // Stores a direct return value to an operand temporary lvalue + DirectOperand(u32) } diff --git a/src/librustc_trans/mir/lvalue.rs b/src/librustc_trans/mir/lvalue.rs index c9087181f9ddc..5a0992a6b6b41 100644 --- a/src/librustc_trans/mir/lvalue.rs +++ b/src/librustc_trans/mir/lvalue.rs @@ -207,6 +207,37 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } } + // Perform an action using the given Lvalue. + // If the Lvalue is an empty TempRef::Operand, then a temporary stack slot + // is created first, then used as an operand to update the Lvalue. + pub fn with_lvalue_ref(&mut self, bcx: &BlockAndBuilder<'bcx, 'tcx>, + lvalue: &mir::Lvalue<'tcx>, f: F) -> U + where F: FnOnce(&mut Self, LvalueRef<'tcx>) -> U + { + match *lvalue { + mir::Lvalue::Temp(idx) => { + match self.temps[idx as usize] { + TempRef::Lvalue(lvalue) => f(self, lvalue), + TempRef::Operand(None) => { + let lvalue_ty = self.mir.lvalue_ty(bcx.tcx(), lvalue); + let lvalue = LvalueRef::alloca(bcx, lvalue_ty.to_ty(bcx.tcx()), "lvalue_temp"); + let ret = f(self, lvalue); + let op = self.trans_load(bcx, lvalue.llval, lvalue_ty.to_ty(bcx.tcx())); + self.temps[idx as usize] = TempRef::Operand(Some(op)); + ret + } + TempRef::Operand(Some(_)) => { + bug!("Lvalue temp already set"); + } + } + } + _ => { + let lvalue = self.trans_lvalue(bcx, lvalue); + f(self, lvalue) + } + } + } + /// Adjust the bitwidth of an index since LLVM is less forgiving /// than we are. /// From 73790f02e3a9c021cc5dd65ac7c017a9f6ae889f Mon Sep 17 00:00:00 2001 From: James Miller Date: Wed, 6 Apr 2016 17:57:42 +1200 Subject: [PATCH 2/2] Move ReturnDest creation into a method It's quite a large amount of code, and moving it into a method allowed for some refactoring to make the logic a little easier to understand --- src/librustc_trans/mir/block.rs | 122 +++++++++++++++---------------- src/librustc_trans/mir/lvalue.rs | 4 +- 2 files changed, 61 insertions(+), 65 deletions(-) diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index b8417f985650c..7234bff439724 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -211,70 +211,13 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { let mut llargs = Vec::with_capacity(arg_count); // Prepare the return value destination - let ret_dest = if let Some((ref d, _)) = *destination { - match *d { - // Handle temporary lvalues, specifically Operand ones, as - // they don't have allocas - mir::Lvalue::Temp(idx) => { - let lvalue_ty = self.mir.lvalue_ty(bcx.tcx(), d); - let ret_ty = lvalue_ty.to_ty(bcx.tcx()); - match self.temps[idx as usize] { - TempRef::Lvalue(dest) => { - if fn_ty.ret.is_indirect() { - llargs.push(dest.llval); - ReturnDest::Nothing - } else if fn_ty.ret.is_ignore() { - ReturnDest::Nothing - } else { - ReturnDest::Store(dest.llval) - } - } - TempRef::Operand(None) => { - let is_intrinsic = if let Intrinsic = callee.data { - true - } else { - false - }; - - if fn_ty.ret.is_indirect() { - // Odd, but possible, case, we have an operand temporary, - // but the calling convention has an indirect return. - let tmp = bcx.with_block(|bcx| { - base::alloc_ty(bcx, ret_ty, "tmp_ret") - }); - llargs.push(tmp); - ReturnDest::IndirectOperand(tmp, idx) - } else if is_intrinsic { - // Currently, intrinsics always need a location to store - // the result. so we create a temporary alloca for the - // result - let tmp = bcx.with_block(|bcx| { - base::alloc_ty(bcx, ret_ty, "tmp_ret") - }); - ReturnDest::IndirectOperand(tmp, idx) - } else if fn_ty.ret.is_ignore() { - ReturnDest::Nothing - } else { - ReturnDest::DirectOperand(idx) - } - } - TempRef::Operand(Some(_)) => { - bug!("lvalue temp already assigned to"); - } - } - } - _ => { - let dest = self.trans_lvalue(&bcx, d); - if fn_ty.ret.is_indirect() { - llargs.push(dest.llval); - ReturnDest::Nothing - } else if fn_ty.ret.is_ignore() { - ReturnDest::Nothing - } else { - ReturnDest::Store(dest.llval) - } - } - } + let ret_dest = if let Some((ref dest, _)) = *destination { + let is_intrinsic = if let Intrinsic = callee.data { + true + } else { + false + }; + self.make_return_dest(&bcx, dest, &fn_ty.ret, &mut llargs, is_intrinsic) } else { ReturnDest::Nothing }; @@ -601,6 +544,57 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { self.blocks[bb.index()].llbb } + fn make_return_dest(&mut self, bcx: &BlockAndBuilder<'bcx, 'tcx>, + dest: &mir::Lvalue<'tcx>, fn_ret_ty: &ArgType, + llargs: &mut Vec, is_intrinsic: bool) -> ReturnDest { + // If the return is ignored, we can just return a do-nothing ReturnDest + if fn_ret_ty.is_ignore() { + return ReturnDest::Nothing; + } + let dest = match *dest { + mir::Lvalue::Temp(idx) => { + let lvalue_ty = self.mir.lvalue_ty(bcx.tcx(), dest); + let ret_ty = lvalue_ty.to_ty(bcx.tcx()); + match self.temps[idx as usize] { + TempRef::Lvalue(dest) => dest, + TempRef::Operand(None) => { + // Handle temporary lvalues, specifically Operand ones, as + // they don't have allocas + return if fn_ret_ty.is_indirect() { + // Odd, but possible, case, we have an operand temporary, + // but the calling convention has an indirect return. + let tmp = bcx.with_block(|bcx| { + base::alloc_ty(bcx, ret_ty, "tmp_ret") + }); + llargs.push(tmp); + ReturnDest::IndirectOperand(tmp, idx) + } else if is_intrinsic { + // Currently, intrinsics always need a location to store + // the result. so we create a temporary alloca for the + // result + let tmp = bcx.with_block(|bcx| { + base::alloc_ty(bcx, ret_ty, "tmp_ret") + }); + ReturnDest::IndirectOperand(tmp, idx) + } else { + ReturnDest::DirectOperand(idx) + }; + } + TempRef::Operand(Some(_)) => { + bug!("lvalue temp already assigned to"); + } + } + } + _ => self.trans_lvalue(bcx, dest) + }; + if fn_ret_ty.is_indirect() { + llargs.push(dest.llval); + ReturnDest::Nothing + } else { + ReturnDest::Store(dest.llval) + } + } + fn trans_transmute(&mut self, bcx: &BlockAndBuilder<'bcx, 'tcx>, src: &mir::Operand<'tcx>, dst: LvalueRef<'tcx>) { let mut val = self.trans_operand(bcx, src); diff --git a/src/librustc_trans/mir/lvalue.rs b/src/librustc_trans/mir/lvalue.rs index 5a0992a6b6b41..13e1894df16af 100644 --- a/src/librustc_trans/mir/lvalue.rs +++ b/src/librustc_trans/mir/lvalue.rs @@ -220,7 +220,9 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { TempRef::Lvalue(lvalue) => f(self, lvalue), TempRef::Operand(None) => { let lvalue_ty = self.mir.lvalue_ty(bcx.tcx(), lvalue); - let lvalue = LvalueRef::alloca(bcx, lvalue_ty.to_ty(bcx.tcx()), "lvalue_temp"); + let lvalue = LvalueRef::alloca(bcx, + lvalue_ty.to_ty(bcx.tcx()), + "lvalue_temp"); let ret = f(self, lvalue); let op = self.trans_load(bcx, lvalue.llval, lvalue_ty.to_ty(bcx.tcx())); self.temps[idx as usize] = TempRef::Operand(Some(op));