From 6db68938ac3cb510f196c2649cea397aba403c9c Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 15 Nov 2017 03:35:35 +0200 Subject: [PATCH] MIR: hide .rodata constants vs by-ref ABI clash in trans. --- src/librustc/mir/mod.rs | 7 ++++--- src/librustc_mir/build/expr/into.rs | 8 +------- src/librustc_trans/mir/block.rs | 11 ++++++++++- src/test/mir-opt/nll/liveness-call-subtlety.rs | 12 +++--------- .../nll/region-liveness-drop-no-may-dangle.rs | 2 +- .../nll/region-liveness-two-disjoint-uses.rs | 2 +- src/test/run-pass/mir_trans_calls.rs | 17 ++++++++++++++++- 7 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index bea273c84a9..3cc159978a8 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -684,9 +684,10 @@ pub enum TerminatorKind<'tcx> { Call { /// The function that’s being called func: Operand<'tcx>, - /// Arguments the function is called with. These are owned by the callee, which is free to - /// modify them. This is important as "by-value" arguments might be passed by-reference at - /// the ABI level. + /// Arguments the function is called with. + /// These are owned by the callee, which is free to modify them. + /// This allows the memory occupied by "by-value" arguments to be + /// reused across function calls without duplicating the contents. args: Vec>, /// Destination for the return value. If some, the call is converging. destination: Option<(Lvalue<'tcx>, BasicBlock)>, diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 280e1c81966..cdbcb43370f 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -247,13 +247,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } else { let args: Vec<_> = args.into_iter() - .map(|arg| { - let scope = this.local_scope(); - // Function arguments are owned by the callee, so we need as_temp() - // instead of as_operand() to enforce copies - let operand = unpack!(block = this.as_temp(block, scope, arg)); - Operand::Consume(Lvalue::Local(operand)) - }) + .map(|arg| unpack!(block = this.as_local_operand(block, arg))) .collect(); let success = this.cfg.start_new_block(); diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index 11d992bd4cf..bd26c961bb2 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -524,7 +524,16 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> { } } - let op = self.trans_operand(&bcx, arg); + let mut op = self.trans_operand(&bcx, arg); + + // The callee needs to own the argument memory if we pass it + // by-ref, so make a local copy of non-immediate constants. + if let (&mir::Operand::Constant(_), Ref(..)) = (arg, op.val) { + let tmp = LvalueRef::alloca(&bcx, op.ty, "const"); + self.store_operand(&bcx, tmp.llval, tmp.alignment.to_align(), op); + op.val = Ref(tmp.llval, tmp.alignment); + } + self.trans_argument(&bcx, op, &mut llargs, &fn_ty, &mut idx, &mut llfn, &def); } diff --git a/src/test/mir-opt/nll/liveness-call-subtlety.rs b/src/test/mir-opt/nll/liveness-call-subtlety.rs index 2de3e7d704d..59a1d4891f6 100644 --- a/src/test/mir-opt/nll/liveness-call-subtlety.rs +++ b/src/test/mir-opt/nll/liveness-call-subtlety.rs @@ -31,21 +31,15 @@ fn main() { // | Live variables at bb0[0]: [] // StorageLive(_1); // | Live variables at bb0[1]: [] -// StorageLive(_2); -// | Live variables at bb0[2]: [] -// _2 = const 22usize; -// | Live variables at bb0[3]: [_2] -// _1 = const >::new(_2) -> bb1; +// _1 = const >::new(const 22usize) -> bb1; // } // END rustc.main.nll.0.mir // START rustc.main.nll.0.mir // | Live variables on entry to bb1: [_1 (drop)] // bb1: { // | Live variables at bb1[0]: [_1 (drop)] -// StorageDead(_2); +// StorageLive(_2); // | Live variables at bb1[1]: [_1 (drop)] -// StorageLive(_3); -// | Live variables at bb1[2]: [_1 (drop)] -// _3 = const can_panic() -> [return: bb2, unwind: bb4]; +// _2 = const can_panic() -> [return: bb2, unwind: bb4]; // } // END rustc.main.nll.0.mir diff --git a/src/test/mir-opt/nll/region-liveness-drop-no-may-dangle.rs b/src/test/mir-opt/nll/region-liveness-drop-no-may-dangle.rs index 2968ae7d485..aaeebe74951 100644 --- a/src/test/mir-opt/nll/region-liveness-drop-no-may-dangle.rs +++ b/src/test/mir-opt/nll/region-liveness-drop-no-may-dangle.rs @@ -46,5 +46,5 @@ impl Drop for Wrap { // END RUST SOURCE // START rustc.main.nll.0.mir -// | '_#5r: {bb1[3], bb1[4], bb1[5], bb2[0], bb2[1], bb2[2], bb3[0], bb3[1], bb3[2], bb4[0], bb4[1], bb4[2], bb6[0], bb7[0], bb7[1], bb7[2], bb8[0]} +// | '_#5r: {bb1[3], bb1[4], bb1[5], bb2[0], bb2[1], bb2[2], bb3[0], bb4[0], bb4[1], bb4[2], bb6[0], bb7[0], bb7[1], bb8[0]} // END rustc.main.nll.0.mir diff --git a/src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs b/src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs index 736d2902ec6..5c28746126a 100644 --- a/src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs +++ b/src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs @@ -45,5 +45,5 @@ fn main() { // ... // _2 = &'_#1r _1[_3]; // ... -// _2 = &'_#3r (*_11); +// _2 = &'_#3r (*_10); // END rustc.main.nll.0.mir diff --git a/src/test/run-pass/mir_trans_calls.rs b/src/test/run-pass/mir_trans_calls.rs index d429c681bbe..d02e3287bc3 100644 --- a/src/test/run-pass/mir_trans_calls.rs +++ b/src/test/run-pass/mir_trans_calls.rs @@ -8,7 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(fn_traits)] +#![feature(fn_traits, test)] + +extern crate test; fn test1(a: isize, b: (i32, i32), c: &[i32]) -> (isize, (i32, i32), &[i32]) { // Test passing a number of arguments including a fat pointer. @@ -156,6 +158,16 @@ fn test_fn_nested_pair(x: &((f32, f32), u32)) -> (f32, f32) { (z.0, z.1) } +fn test_fn_const_arg_by_ref(mut a: [u64; 4]) -> u64 { + // Mutate the by-reference argument, which won't work with + // a non-immediate constant unless it's copied to the stack. + let a = test::black_box(&mut a); + a[0] += a[1]; + a[0] += a[2]; + a[0] += a[3]; + a[0] +} + fn main() { assert_eq!(test1(1, (2, 3), &[4, 5, 6]), (1, (2, 3), &[4, 5, 6][..])); assert_eq!(test2(98), 98); @@ -182,4 +194,7 @@ fn main() { assert_eq!(test_fn_ignored_pair_0(), ()); assert_eq!(test_fn_ignored_pair_named(), (Foo, Foo)); assert_eq!(test_fn_nested_pair(&((1.0, 2.0), 0)), (1.0, 2.0)); + + const ARRAY: [u64; 4] = [1, 2, 3, 4]; + assert_eq!(test_fn_const_arg_by_ref(ARRAY), 1 + 2 + 3 + 4); }