Auto merge of #25959 - pnkfelix:fsk-hack-move-val-init, r=nikomatsakis

Hack the move_val_init intrinsic to trans directly into the destination address.

This is to remove an intermediate (and unnecessary) alloca on the stack that one otherwise suffers when using this intrinsic.

This is part of the `box` protocol work; in particular, this is meant to address the `ptr::write` codegen issues alluded to at this comment: 

  #22086 (comment)

cc #22181

Author: bors

src/librustc_trans/trans/intrinsic.rs

@@ -265,6 +265,48 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
         }
     }
 
+    // For `move_val_init` we can evaluate the destination address
+    // (the first argument) and then trans the source value (the
+    // second argument) directly into the resulting destination
+    // address.
+    if &name[..] == "move_val_init" {
+        if let callee::ArgExprs(ref exprs) = args {
+            let (dest_expr, source_expr) = if exprs.len() != 2 {
+                ccx.sess().bug("expected two exprs as arguments for `move_val_init` intrinsic");
+            } else {
+                (&exprs[0], &exprs[1])
+            };
+            let arg_tys = ty::erase_late_bound_regions(bcx.tcx(), &ty::ty_fn_args(callee_ty));
+
+            // evaluate destination address
+            let lldest_addr = unpack_result!(bcx, {
+                let dest_datum = unpack_datum!(bcx, expr::trans(bcx, dest_expr));
+                callee::trans_arg_datum(bcx,
+                                        arg_tys[0],
+                                        dest_datum,
+                                        cleanup::CustomScope(cleanup_scope),
+                                        callee::DontAutorefArg)
+            });
+
+            // `expr::trans_into(bcx, expr, dest)` is equiv to
+            //
+            //    `trans(bcx, expr).store_to_dest(dest)`,
+            //
+            // which for `dest == expr::SaveIn(addr)`, is equivalent to:
+            //
+            //    `trans(bcx, expr).store_to(bcx, addr)`.
+            let lldest = expr::Dest::SaveIn(lldest_addr);
+            bcx = expr::trans_into(bcx, source_expr, lldest);
+
+            let llresult = C_nil(ccx);
+            fcx.pop_and_trans_custom_cleanup_scope(bcx, cleanup_scope);
+
+            return Result::new(bcx, llresult);
+        } else {
+            ccx.sess().bug("expected two exprs as arguments for `move_val_init` intrinsic");
+        }
+    }
+
     // Push the arguments.
     let mut llargs = Vec::new();
     bcx = callee::trans_args(bcx,
@@ -356,22 +398,6 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
             let lltp_ty = type_of::type_of(ccx, tp_ty);
             C_uint(ccx, machine::llalign_of_pref(ccx, lltp_ty))
         }
-        (_, "move_val_init") => {
-            // Create a datum reflecting the value being moved.
-            // Use `appropriate_mode` so that the datum is by ref
-            // if the value is non-immediate. Note that, with
-            // intrinsics, there are no argument cleanups to
-            // concern ourselves with, so we can use an rvalue datum.
-            let tp_ty = *substs.types.get(FnSpace, 0);
-            let mode = appropriate_rvalue_mode(ccx, tp_ty);
-            let src = Datum {
-                val: llargs[1],
-                ty: tp_ty,
-                kind: Rvalue::new(mode)
-            };
-            bcx = src.store_to(bcx, llargs[0]);
-            C_nil(ccx)
-        }
         (_, "drop_in_place") => {
             let tp_ty = *substs.types.get(FnSpace, 0);
             glue::drop_ty(bcx, llargs[0], tp_ty, call_debug_location);

src/test/run-pass/intrinsic-move-val-cleanups.rs

@@ -0,0 +1,194 @@
+// Copyright 2015 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// This test is checking that the move_val_init intrinsic is
+// respecting cleanups for both of its argument expressions.
+//
+// In other words, if either DEST or SOURCE in
+//
+//   `intrinsics::move_val_init(DEST, SOURCE)
+//
+// introduce temporaries that require cleanup, and SOURCE panics, then
+// make sure the cleanups still occur.
+
+#![feature(core, std_misc)]
+
+use std::cell::RefCell;
+use std::intrinsics;
+use std::sync::{Arc, LockResult, Mutex, MutexGuard};
+use std::thread;
+
+type LogEntry = (&'static str, i32);
+type Guarded = RefCell<Vec<LogEntry>>;
+#[derive(Clone)]
+struct Log(Arc<Mutex<Guarded>>);
+struct Acquired<'a>(MutexGuard<'a, Guarded>);
+type LogState = (MutexWas, &'static [LogEntry]);
+
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+enum MutexWas { Poisoned, NotPoisoned }
+
+impl Log {
+    fn lock(&self) -> LockResult<MutexGuard<RefCell<Vec<LogEntry>>>> { self.0.lock() }
+    fn acquire(&self) -> Acquired { Acquired(self.0.lock().unwrap()) }
+}
+
+impl<'a> Acquired<'a> {
+    fn log(&self, s: &'static str, i: i32) { self.0.borrow_mut().push((s, i)); }
+}
+
+const TEST1_EXPECT: LogState = (MutexWas::NotPoisoned,
+                                &[("double-check non-poisoning path", 1)
+                                  ]);
+
+fn test1(log: Log) {
+    {
+        let acq = log.acquire();
+        acq.log("double-check non-poisoning path", 1);
+    }
+    panic!("every test ends in a panic");
+}
+
+const TEST2_EXPECT: LogState = (MutexWas::Poisoned,
+                                &[("double-check poisoning path", 1),
+                                  ("and multiple log entries", 2),
+                                  ]);
+fn test2(log: Log) {
+    let acq = log.acquire();
+    acq.log("double-check poisoning path", 1);
+    acq.log("and multiple log entries", 2);
+    panic!("every test ends in a panic");
+}
+
+struct LogOnDrop<'a>(&'a Acquired<'a>, &'static str, i32);
+impl<'a> Drop for LogOnDrop<'a> {
+    fn drop(&mut self) {
+        self.0.log(self.1, self.2);
+    }
+}
+
+const TEST3_EXPECT: LogState = (MutexWas::Poisoned,
+                                &[("double-check destructors can log", 1),
+                                  ("drop d2", 2),
+                                  ("drop d1", 3),
+                                  ]);
+fn test3(log: Log) {
+    let acq = log.acquire();
+    acq.log("double-check destructors can log", 1);
+    let _d1 = LogOnDrop(&acq, "drop d1", 3);
+    let _d2 = LogOnDrop(&acq, "drop d2", 2);
+    panic!("every test ends in a panic");
+}
+
+// The *real* tests of panic-handling for move_val_init intrinsic
+// start here.
+
+const TEST4_EXPECT: LogState = (MutexWas::Poisoned,
+                                &[("neither arg panics", 1),
+                                  ("drop temp LOD", 2),
+                                  ("drop temp LOD", 3),
+                                  ("drop dest_b", 4),
+                                  ("drop dest_a", 5),
+                                  ]);
+fn test4(log: Log) {
+    let acq = log.acquire();
+    acq.log("neither arg panics", 1);
+    let mut dest_a = LogOnDrop(&acq, "a will be overwritten, not dropped", 0);
+    let mut dest_b = LogOnDrop(&acq, "b will be overwritten, not dropped", 0);
+    unsafe {
+        intrinsics::move_val_init({ LogOnDrop(&acq, "drop temp LOD", 2); &mut dest_a },
+                                  LogOnDrop(&acq, "drop dest_a", 5));
+        intrinsics::move_val_init(&mut dest_b, { LogOnDrop(&acq, "drop temp LOD", 3);
+                                                 LogOnDrop(&acq, "drop dest_b", 4) });
+    }
+    panic!("every test ends in a panic");
+}
+
+
+// Check that move_val_init(PANIC, SOURCE_EXPR) never evaluates SOURCE_EXPR
+const TEST5_EXPECT: LogState = (MutexWas::Poisoned,
+                                &[("first arg panics", 1),
+                                  ("drop orig dest_a", 2),
+                                  ]);
+fn test5(log: Log) {
+    let acq = log.acquire();
+    acq.log("first arg panics", 1);
+    let mut _dest_a = LogOnDrop(&acq, "drop orig dest_a", 2);
+    unsafe {
+        intrinsics::move_val_init({ panic!("every test ends in a panic") },
+                                  LogOnDrop(&acq, "we never get here", 0));
+    }
+}
+
+// Check that move_val_init(DEST_EXPR, PANIC) cleans up temps from DEST_EXPR.
+const TEST6_EXPECT: LogState = (MutexWas::Poisoned,
+                                &[("second arg panics", 1),
+                                  ("drop temp LOD", 2),
+                                  ("drop orig dest_a", 3),
+                                  ]);
+fn test6(log: Log) {
+    let acq = log.acquire();
+    acq.log("second arg panics", 1);
+    let mut dest_a = LogOnDrop(&acq, "drop orig dest_a", 3);
+    unsafe {
+        intrinsics::move_val_init({ LogOnDrop(&acq, "drop temp LOD", 2); &mut dest_a },
+                                  { panic!("every test ends in a panic"); });
+    }
+}
+
+// Check that move_val_init(DEST_EXPR, COMPLEX_PANIC) cleans up temps from COMPLEX_PANIC.
+const TEST7_EXPECT: LogState = (MutexWas::Poisoned,
+                                &[("second arg panics", 1),
+                                  ("drop temp LOD", 2),
+                                  ("drop temp LOD", 3),
+                                  ("drop orig dest_a", 4),
+                                  ]);
+fn test7(log: Log) {
+    let acq = log.acquire();
+    acq.log("second arg panics", 1);
+    let mut dest_a = LogOnDrop(&acq, "drop orig dest_a", 4);
+    unsafe {
+        intrinsics::move_val_init({ LogOnDrop(&acq, "drop temp LOD", 2); &mut dest_a },
+                                  { LogOnDrop(&acq, "drop temp LOD", 3);
+                                    panic!("every test ends in a panic"); });
+    }
+}
+
+const TEST_SUITE: &'static [(&'static str, fn (Log), LogState)] =
+    &[("test1", test1, TEST1_EXPECT),
+      ("test2", test2, TEST2_EXPECT),
+      ("test3", test3, TEST3_EXPECT),
+      ("test4", test4, TEST4_EXPECT),
+      ("test5", test5, TEST5_EXPECT),
+      ("test6", test6, TEST6_EXPECT),
+      ("test7", test7, TEST7_EXPECT),
+      ];
+
+fn main() {
+    for &(name, test, expect) in TEST_SUITE {
+        let log = Log(Arc::new(Mutex::new(RefCell::new(Vec::new()))));
+        let ret = { let log = log.clone(); thread::spawn(move || test(log)).join() };
+        assert!(ret.is_err(), "{} must end with panic", name);
+        {
+            let l = log.lock();
+            match l {
+                Ok(acq) => {
+                    assert_eq!((MutexWas::NotPoisoned, &acq.borrow()[..]), expect);
+                    println!("{} (unpoisoned) log: {:?}", name, *acq);
+                }
+                Err(e) => {
+                    let acq = e.into_inner();
+                    assert_eq!((MutexWas::Poisoned, &acq.borrow()[..]), expect);
+                    println!("{} (poisoned) log: {:?}", name, *acq);
+                }
+            }
+        }
+    }
+}