From 1983e3d8d7faed3e9e47d5589d3826398577b152 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 20 Sep 2012 12:29:15 -0700 Subject: [PATCH] Make + mode by-value if the type is immediate, by-ref otherwise Fixes #3523 --- src/libcore/to_bytes.rs | 9 +++ src/rustc/middle/trans/base.rs | 104 ++++++++++++++----------- src/rustc/middle/trans/callee.rs | 13 +++- src/rustc/middle/trans/common.rs | 21 +++-- src/rustc/middle/trans/datum.rs | 53 ++++++++++--- src/rustc/middle/trans/foreign.rs | 16 +++- src/rustc/middle/trans/monomorphize.rs | 45 +++++++---- src/rustc/middle/trans/type_of.rs | 10 ++- src/rustc/middle/trans/type_use.rs | 20 +++-- 9 files changed, 199 insertions(+), 92 deletions(-) diff --git a/src/libcore/to_bytes.rs b/src/libcore/to_bytes.rs index 0519347e43843..db59077437073 100644 --- a/src/libcore/to_bytes.rs +++ b/src/libcore/to_bytes.rs @@ -37,6 +37,15 @@ trait IterBytes { pure fn iter_bytes(lsb0: bool, f: Cb); } +impl bool: IterBytes { + #[inline(always)] + pure fn iter_bytes(_lsb0: bool, f: Cb) { + f([ + self as u8 + ]); + } +} + impl u8: IterBytes { #[inline(always)] pure fn iter_bytes(_lsb0: bool, f: Cb) { diff --git a/src/rustc/middle/trans/base.rs b/src/rustc/middle/trans/base.rs index 8cf9be711a3b2..1601d583f00d9 100644 --- a/src/rustc/middle/trans/base.rs +++ b/src/rustc/middle/trans/base.rs @@ -1437,10 +1437,9 @@ fn new_fn_ctxt(ccx: @crate_ctxt, path: path, llfndecl: ValueRef, // field of the fn_ctxt with fn create_llargs_for_fn_args(cx: fn_ctxt, ty_self: self_arg, - args: ~[ast::arg]) { + args: ~[ast::arg]) -> ~[ValueRef] { let _icx = cx.insn_ctxt("create_llargs_for_fn_args"); - // Skip the implicit arguments 0, and 1. - let mut arg_n = first_real_arg; + match ty_self { impl_self(tt) => { cx.llself = Some(ValSelfData { @@ -1459,28 +1458,22 @@ fn create_llargs_for_fn_args(cx: fn_ctxt, no_self => () } - // Populate the llargs field of the function context with the ValueRefs - // that we get from llvm::LLVMGetParam for each argument. - for vec::each(args) |arg| { - let llarg = llvm::LLVMGetParam(cx.llfn, arg_n as c_uint); - assert (llarg as int != 0); - // Note that this uses local_mem even for things passed by value. - // copy_args_to_allocas will overwrite the table entry with local_imm - // before it's actually used. - cx.llargs.insert(arg.id, local_mem(llarg)); - arg_n += 1u; - } + // Return an array containing the ValueRefs that we get from + // llvm::LLVMGetParam for each argument. + vec::from_fn(args.len(), |i| { + let arg_n = first_real_arg + i; + llvm::LLVMGetParam(cx.llfn, arg_n as c_uint) + }) } -fn copy_args_to_allocas(fcx: fn_ctxt, bcx: block, args: ~[ast::arg], - arg_tys: ~[ty::arg]) -> block { +fn copy_args_to_allocas(fcx: fn_ctxt, + bcx: block, + args: &[ast::arg], + raw_llargs: &[ValueRef], + arg_tys: &[ty::arg]) -> block { let _icx = fcx.insn_ctxt("copy_args_to_allocas"); let tcx = bcx.tcx(); - let mut arg_n: uint = 0u, bcx = bcx; - let epic_fail = fn@() -> ! { - tcx.sess.bug(~"someone forgot\ - to document an invariant in copy_args_to_allocas!"); - }; + let mut bcx = bcx; match fcx.llself { Some(copy slf) => { @@ -1496,31 +1489,50 @@ fn copy_args_to_allocas(fcx: fn_ctxt, bcx: block, args: ~[ast::arg], _ => {} } - for vec::each(arg_tys) |arg| { - let id = args[arg_n].id; - let argval = match fcx.llargs.get(id) { - local_mem(v) => v, - _ => epic_fail() - }; - match ty::resolved_mode(tcx, arg.mode) { - ast::by_mutbl_ref => (), - ast::by_move | ast::by_copy => add_clean(bcx, argval, arg.ty), - ast::by_val => { - if !ty::type_is_immediate(arg.ty) { - let alloc = alloc_ty(bcx, arg.ty); - Store(bcx, argval, alloc); - fcx.llargs.insert(id, local_mem(alloc)); - } else { - fcx.llargs.insert(id, local_imm(argval)); + for uint::range(0, arg_tys.len()) |arg_n| { + let arg_ty = &arg_tys[arg_n]; + let raw_llarg = raw_llargs[arg_n]; + let arg_id = args[arg_n].id; + + // For certain mode/type combinations, the raw llarg values are passed + // by value. However, within the fn body itself, we want to always + // have all locals and argumenst be by-ref so that we can cancel the + // cleanup and for better interaction with LLVM's debug info. So, if + // the argument would be passed by value, we store it into an alloca. + // This alloca should be optimized away by LLVM's mem-to-reg pass in + // the event it's not truly needed. + let llarg; + match ty::resolved_mode(tcx, arg_ty.mode) { + ast::by_ref | ast::by_mutbl_ref => { + llarg = raw_llarg; + } + ast::by_move | ast::by_copy => { + // only by value if immediate: + if datum::appropriate_mode(arg_ty.ty).is_by_value() { + let alloc = alloc_ty(bcx, arg_ty.ty); + Store(bcx, raw_llarg, alloc); + llarg = alloc; + } else { + llarg = raw_llarg; + } + + add_clean(bcx, llarg, arg_ty.ty); + } + ast::by_val => { + // always by value, also not owned, so don't add a cleanup: + let alloc = alloc_ty(bcx, arg_ty.ty); + Store(bcx, raw_llarg, alloc); + llarg = alloc; } - } - ast::by_ref => () } + + fcx.llargs.insert(arg_id, local_mem(llarg)); + if fcx.ccx.sess.opts.extra_debuginfo { debuginfo::create_arg(bcx, args[arg_n], args[arg_n].ty.span); } - arg_n += 1u; } + return bcx; } @@ -1558,7 +1570,7 @@ fn trans_closure(ccx: @crate_ctxt, path: path, decl: ast::fn_decl, // Set up arguments to the function. let fcx = new_fn_ctxt_w_id(ccx, path, llfndecl, id, param_substs, Some(body.span)); - create_llargs_for_fn_args(fcx, ty_self, decl.inputs); + let raw_llargs = create_llargs_for_fn_args(fcx, ty_self, decl.inputs); // Set GC for function. if ccx.sess.opts.gc { @@ -1576,7 +1588,7 @@ fn trans_closure(ccx: @crate_ctxt, path: path, decl: ast::fn_decl, let block_ty = node_id_type(bcx, body.node.id); let arg_tys = ty::ty_fn_args(node_id_type(bcx, id)); - bcx = copy_args_to_allocas(fcx, bcx, decl.inputs, arg_tys); + bcx = copy_args_to_allocas(fcx, bcx, decl.inputs, raw_llargs, arg_tys); maybe_load_env(fcx); @@ -1648,14 +1660,14 @@ fn trans_enum_variant(ccx: @crate_ctxt, id: varg.id}); let fcx = new_fn_ctxt_w_id(ccx, ~[], llfndecl, variant.node.id, param_substs, None); - create_llargs_for_fn_args(fcx, no_self, fn_args); + let raw_llargs = create_llargs_for_fn_args(fcx, no_self, fn_args); let ty_param_substs = match param_substs { Some(substs) => substs.tys, None => ~[] }; let bcx = top_scope_block(fcx, None), lltop = bcx.llbb; let arg_tys = ty::ty_fn_args(node_id_type(bcx, variant.node.id)); - let bcx = copy_args_to_allocas(fcx, bcx, fn_args, arg_tys); + let bcx = copy_args_to_allocas(fcx, bcx, fn_args, raw_llargs, arg_tys); // Cast the enum to a type we can GEP into. let llblobptr = if is_degen { @@ -1705,11 +1717,11 @@ fn trans_class_ctor(ccx: @crate_ctxt, path: path, decl: ast::fn_decl, // Make the fn context let fcx = new_fn_ctxt_w_id(ccx, path, llctor_decl, ctor_id, Some(psubsts), Some(sp)); - create_llargs_for_fn_args(fcx, no_self, decl.inputs); + let raw_llargs = create_llargs_for_fn_args(fcx, no_self, decl.inputs); let mut bcx_top = top_scope_block(fcx, body.info()); let lltop = bcx_top.llbb; let arg_tys = ty::ty_fn_args(node_id_type(bcx_top, ctor_id)); - bcx_top = copy_args_to_allocas(fcx, bcx_top, decl.inputs, arg_tys); + bcx_top = copy_args_to_allocas(fcx, bcx_top, decl.inputs, raw_llargs, arg_tys); // Create a temporary for `self` that we will return at the end let selfdatum = datum::scratch_datum(bcx_top, rslt_ty, true); diff --git a/src/rustc/middle/trans/callee.rs b/src/rustc/middle/trans/callee.rs index b4865044961d7..946a55318ebb3 100644 --- a/src/rustc/middle/trans/callee.rs +++ b/src/rustc/middle/trans/callee.rs @@ -548,6 +548,7 @@ fn trans_arg_expr(bcx: block, let llformal_ty = type_of::type_of(ccx, formal_ty.ty); val = llvm::LLVMGetUndef(llformal_ty); } else { + // FIXME(#3548) use the adjustments table match autoref_arg { DoAutorefArg => { val = arg_datum.to_ref_llval(bcx); } DontAutorefArg => { @@ -583,8 +584,16 @@ fn trans_arg_expr(bcx: block, // callee is actually invoked. scratch.add_clean(bcx); vec::push(*temp_cleanups, scratch.val); - val = scratch.val; - } + + match arg_datum.appropriate_mode() { + ByValue => { + val = Load(bcx, scratch.val); + } + ByRef => { + val = scratch.val; + } + } + } } } } diff --git a/src/rustc/middle/trans/common.rs b/src/rustc/middle/trans/common.rs index ae539f5ebc6fc..7896cbbcb5644 100644 --- a/src/rustc/middle/trans/common.rs +++ b/src/rustc/middle/trans/common.rs @@ -1125,7 +1125,10 @@ fn get_param(fndecl: ValueRef, param: uint) -> ValueRef { enum mono_param_id { mono_precise(ty::t, Option<~[mono_id]>), mono_any, - mono_repr(uint /* size */, uint /* align */), + mono_repr(uint /* size */, + uint /* align */, + bool /* is_float */, + datum::DatumMode), } type mono_id_ = {def: ast::def_id, params: ~[mono_param_id]}; @@ -1140,8 +1143,10 @@ impl mono_param_id: cmp::Eq { ty_a == ty_b && ids_a == ids_b } (mono_any, mono_any) => true, - (mono_repr(size_a, align_a), mono_repr(size_b, align_b)) => { - size_a == size_b && align_a == align_b + (mono_repr(size_a, align_a, is_float_a, mode_a), + mono_repr(size_b, align_b, is_float_b, mode_b)) => { + size_a == size_b && align_a == align_b && + is_float_a == is_float_b && mode_a == mode_b } (mono_precise(*), _) => false, (mono_any, _) => false, @@ -1159,8 +1164,10 @@ impl mono_param_id : cmp::Eq { ty_a == ty_b && ids_a == ids_b } (mono_any, mono_any) => true, - (mono_repr(size_a, align_a), mono_repr(size_b, align_b)) => { - size_a == size_b && align_a == align_b + (mono_repr(size_a, align_a, is_float_a, mode_a), + mono_repr(size_b, align_b, is_float_b, mode_b)) => { + size_a == size_b && align_a == align_b && + is_float_a == is_float_b && mode_a == mode_b } (mono_precise(*), _) => false, (mono_any, _) => false, @@ -1194,8 +1201,8 @@ impl mono_param_id : to_bytes::IterBytes { mono_any => 1u8.iter_bytes(lsb0, f), - mono_repr(a,b) => - to_bytes::iter_bytes_3(&2u8, &a, &b, lsb0, f) + mono_repr(ref a, ref b, ref c, ref d) => + to_bytes::iter_bytes_5(&2u8, a, b, c, d, lsb0, f) } } } diff --git a/src/rustc/middle/trans/datum.rs b/src/rustc/middle/trans/datum.rs index a95fed84bafe7..730f068cbb21c 100644 --- a/src/rustc/middle/trans/datum.rs +++ b/src/rustc/middle/trans/datum.rs @@ -138,6 +138,29 @@ impl DatumMode { } } +#[cfg(stage0)] +impl DatumMode: cmp::Eq { + pure fn eq(&&other: DatumMode) -> bool { + (self as uint) == (other as uint) + } + pure fn ne(&&other: DatumMode) -> bool { !self.eq(other) } +} + +#[cfg(stage1)] +#[cfg(stage2)] +impl DatumMode: cmp::Eq { + pure fn eq(other: &DatumMode) -> bool { + self as uint == (*other as uint) + } + pure fn ne(other: &DatumMode) -> bool { !self.eq(other) } +} + +impl DatumMode: to_bytes::IterBytes { + pure fn iter_bytes(lsb0: bool, f: to_bytes::Cb) { + (self as uint).iter_bytes(lsb0, f) + } +} + /// See `Datum Sources` section at the head of this module. enum DatumSource { FromRvalue, @@ -186,6 +209,22 @@ fn scratch_datum(bcx: block, ty: ty::t, zero: bool) -> Datum { Datum { val: scratch, ty: ty, mode: ByRef, source: FromRvalue } } +fn appropriate_mode(ty: ty::t) -> DatumMode { + /*! + * + * Indicates the "appropriate" mode for this value, + * which is either by ref or by value, depending + * on whether type is iimmediate or what. */ + + if ty::type_is_nil(ty) || ty::type_is_bot(ty) { + ByValue + } else if ty::type_is_immediate(ty) { + ByValue + } else { + ByRef + } +} + impl Datum { fn store_will_move() -> bool { match self.source { @@ -446,19 +485,9 @@ impl Datum { } fn appropriate_mode() -> DatumMode { - /*! - * - * Indicates the "appropriate" mode for this value, - * which is either by ref or by value, depending - * on whether type is iimmediate or what. */ + /*! See the `appropriate_mode()` function */ - if ty::type_is_nil(self.ty) || ty::type_is_bot(self.ty) { - ByValue - } else if ty::type_is_immediate(self.ty) { - ByValue - } else { - ByRef - } + appropriate_mode(self.ty) } fn to_appropriate_llval(bcx: block) -> ValueRef { diff --git a/src/rustc/middle/trans/foreign.rs b/src/rustc/middle/trans/foreign.rs index 71c342101d0d6..55d4c8fcd9adb 100644 --- a/src/rustc/middle/trans/foreign.rs +++ b/src/rustc/middle/trans/foreign.rs @@ -877,16 +877,28 @@ fn trans_intrinsic(ccx: @crate_ctxt, decl: ValueRef, item: @ast::foreign_item, fcx.llretptr); } ~"move_val" => { + // Create a datum reflecting the value being moved: + // + // - the datum will be by ref if the value is non-immediate; + // + // - the datum has a FromRvalue source because, that way, + // the `move_to()` method does not feel compelled to + // zero out the memory where the datum resides. Zeroing + // is not necessary since, for intrinsics, there is no + // cleanup to concern ourselves with. let tp_ty = substs.tys[0]; + let mode = appropriate_mode(tp_ty); let src = Datum {val: get_param(decl, first_real_arg + 1u), - ty: tp_ty, mode: ByRef, source: FromLvalue}; + ty: tp_ty, mode: mode, source: FromRvalue}; bcx = src.move_to(bcx, DROP_EXISTING, get_param(decl, first_real_arg)); } ~"move_val_init" => { + // See comments for `"move_val"`. let tp_ty = substs.tys[0]; + let mode = appropriate_mode(tp_ty); let src = Datum {val: get_param(decl, first_real_arg + 1u), - ty: tp_ty, mode: ByRef, source: FromLvalue}; + ty: tp_ty, mode: mode, source: FromRvalue}; bcx = src.move_to(bcx, INIT, get_param(decl, first_real_arg)); } ~"min_align_of" => { diff --git a/src/rustc/middle/trans/monomorphize.rs b/src/rustc/middle/trans/monomorphize.rs index f78a7452f4a28..7ce79fefa9fd3 100644 --- a/src/rustc/middle/trans/monomorphize.rs +++ b/src/rustc/middle/trans/monomorphize.rs @@ -264,20 +264,37 @@ fn make_mono_id(ccx: @crate_ctxt, item: ast::def_id, substs: ~[ty::t], vec::map2(precise_param_ids, uses, |id, uses| { match id { (a, b@Some(_)) => mono_precise(a, b), - (subst, None) => { - if uses == 0u { mono_any } - else if uses == type_use::use_repr && - !ty::type_needs_drop(ccx.tcx, subst) { - let llty = type_of::type_of(ccx, subst); - let size = shape::llsize_of_real(ccx, llty); - let align = shape::llalign_of_pref(ccx, llty); - // Special value for nil to prevent problems with undef - // return pointers. - if size == 1u && ty::type_is_nil(subst) { - mono_repr(0u, 0u) - } else { mono_repr(size, align) } - } else { mono_precise(subst, None) } - } + (subst, None) => { + if uses == 0u { + mono_any + } else if uses == type_use::use_repr && + !ty::type_needs_drop(ccx.tcx, subst) + { + let llty = type_of::type_of(ccx, subst); + let size = shape::llsize_of_real(ccx, llty); + let align = shape::llalign_of_pref(ccx, llty); + let mode = datum::appropriate_mode(subst); + + // FIXME(#3547)---scalars and floats are + // treated differently in most ABIs. But we + // should be doing something more detailed + // here. + let is_float = match ty::get(subst).sty { + ty::ty_float(_) => true, + _ => false + }; + + // Special value for nil to prevent problems + // with undef return pointers. + if size == 1u && ty::type_is_nil(subst) { + mono_repr(0u, 0u, is_float, mode) + } else { + mono_repr(size, align, is_float, mode) + } + } else { + mono_precise(subst, None) + } + } } }) } diff --git a/src/rustc/middle/trans/type_of.rs b/src/rustc/middle/trans/type_of.rs index fa523a4ab9ef0..ac1692d6c33c0 100644 --- a/src/rustc/middle/trans/type_of.rs +++ b/src/rustc/middle/trans/type_of.rs @@ -16,10 +16,16 @@ export type_of_non_gc_box; export type_of_rooted; fn type_of_explicit_arg(ccx: @crate_ctxt, arg: ty::arg) -> TypeRef { - let arg_ty = arg.ty; - let llty = type_of(ccx, arg_ty); + let llty = type_of(ccx, arg.ty); match ty::resolved_mode(ccx.tcx, arg.mode) { ast::by_val => llty, + ast::by_copy | ast::by_move => { + if ty::type_is_immediate(arg.ty) { + llty + } else { + T_ptr(llty) + } + } _ => T_ptr(llty) } } diff --git a/src/rustc/middle/trans/type_use.rs b/src/rustc/middle/trans/type_use.rs index ba99c8e396775..48bea3e1e0563 100644 --- a/src/rustc/middle/trans/type_use.rs +++ b/src/rustc/middle/trans/type_use.rs @@ -27,8 +27,9 @@ use syntax::ast_map; use common::*; type type_uses = uint; // Bitmask -const use_repr: uint = 1u; // Dependency on size/alignment and take/drop glue -const use_tydesc: uint = 2u; // Takes the tydesc, or compares +const use_repr: uint = 1u; /* Dependency on size/alignment/mode and + take/drop glue */ +const use_tydesc: uint = 2u; /* Takes the tydesc, or compares */ type ctx = {ccx: @crate_ctxt, uses: ~[mut type_uses]}; @@ -46,12 +47,17 @@ fn type_uses_for(ccx: @crate_ctxt, fn_id: def_id, n_tps: uint) let cx = {ccx: ccx, uses: vec::to_mut(vec::from_elem(n_tps, 0u))}; match ty::get(ty::lookup_item_type(cx.ccx.tcx, fn_id).ty).sty { - ty::ty_fn(ref fn_ty) => { - for vec::each(fn_ty.sig.inputs) |arg| { - if arg.mode == expl(by_val) { type_needs(cx, use_repr, arg.ty); } + ty::ty_fn(ref fn_ty) => { + for vec::each(fn_ty.sig.inputs) |arg| { + match ty::resolved_mode(ccx.tcx, arg.mode) { + by_val | by_move | by_copy => { + type_needs(cx, use_repr, arg.ty); + } + by_ref | by_mutbl_ref => {} + } + } } - } - _ => () + _ => () } if fn_id_loc.crate != local_crate {