Make + mode by-value if the type is immediate, by-ref otherwise

Fixes #3523

8 files changed, 190 insertions, 92 deletions

diff --git a/src/rustc/middle/trans/base.rs b/src/rustc/middle/trans/base.rs
index 8cf9be711a3..1601d583f00 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 b4865044961..946a55318eb 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 ae539f5ebc6..7896cbbcb56 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 a95fed84baf..730f068cbb2 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 71c342101d0..55d4c8fcd9a 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 f78a7452f4a..7ce79fefa9f 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 fa523a4ab9e..ac1692d6c33 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 ba99c8e3967..48bea3e1e05 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 {