From c849a0d087546d9ef5a47593287b2bc1ec5bfcaf Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 12 May 2020 12:55:55 +0200 Subject: [PATCH 1/4] Add a repro example for not propagating constants of partially const initialized variables --- ...mutable_variable_aggregate_partial_read.rs | 14 +++++ .../rustc.main.ConstProp.diff | 55 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read.rs create mode 100644 src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff diff --git a/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read.rs b/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read.rs new file mode 100644 index 0000000000000..4f43ec8c9470a --- /dev/null +++ b/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read.rs @@ -0,0 +1,14 @@ +// compile-flags: -O + +// EMIT_MIR rustc.main.ConstProp.diff +fn main() { + let mut x: (i32, i32) = foo(); + x.1 = 99; + x.0 = 42; + let y = x.1; +} + +#[inline(never)] +fn foo() -> (i32, i32) { + unimplemented!() +} diff --git a/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff new file mode 100644 index 0000000000000..b88bb9b491564 --- /dev/null +++ b/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff @@ -0,0 +1,55 @@ +- // MIR for `main` before ConstProp ++ // MIR for `main` after ConstProp + + fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/mutable_variable_aggregate_partial_read.rs:4:11: 4:11 + let mut _1: (i32, i32) as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 0 at $DIR/mutable_variable_aggregate_partial_read.rs:5:9: 5:14 + scope 1 { + debug x => _1; // in scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:5:9: 5:14 + let _2: i32; // in scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:9: 8:10 + scope 2 { + debug y => _2; // in scope 2 at $DIR/mutable_variable_aggregate_partial_read.rs:8:9: 8:10 + } + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/mutable_variable_aggregate_partial_read.rs:5:9: 5:14 + _1 = const foo() -> bb1; // scope 0 at $DIR/mutable_variable_aggregate_partial_read.rs:5:29: 5:34 + // ty::Const + // + ty: fn() -> (i32, i32) {foo} + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/mutable_variable_aggregate_partial_read.rs:5:29: 5:32 + // + literal: Const { ty: fn() -> (i32, i32) {foo}, val: Value(Scalar()) } + } + + bb1: { + (_1.1: i32) = const 99i32; // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:6:5: 6:13 + // ty::Const + // + ty: i32 + // + val: Value(Scalar(0x00000063)) + // mir::Constant + // + span: $DIR/mutable_variable_aggregate_partial_read.rs:6:11: 6:13 + // + literal: Const { ty: i32, val: Value(Scalar(0x00000063)) } + (_1.0: i32) = const 42i32; // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:7:5: 7:13 + // ty::Const + // + ty: i32 + // + val: Value(Scalar(0x0000002a)) + // mir::Constant + // + span: $DIR/mutable_variable_aggregate_partial_read.rs:7:11: 7:13 + // + literal: Const { ty: i32, val: Value(Scalar(0x0000002a)) } + StorageLive(_2); // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:9: 8:10 + _2 = (_1.1: i32); // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:13: 8:16 + _0 = const (); // scope 0 at $DIR/mutable_variable_aggregate_partial_read.rs:4:11: 9:2 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/mutable_variable_aggregate_partial_read.rs:4:11: 9:2 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_2); // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:9:1: 9:2 + StorageDead(_1); // scope 0 at $DIR/mutable_variable_aggregate_partial_read.rs:9:1: 9:2 + return; // scope 0 at $DIR/mutable_variable_aggregate_partial_read.rs:9:2: 9:2 + } + } + From de434d8e44b40451eff0020832bb9a9303b32067 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 12 May 2020 13:14:47 +0200 Subject: [PATCH 2/4] Propagate locals, even if they have unpropagatable assignments somewhere. --- src/librustc_mir/transform/const_prop.rs | 20 ++++++++++++++++++- .../rustc.main.ConstProp.diff | 15 +++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index e898f22ec230d..d66c909fe6cc5 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -775,6 +775,10 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { // Projections are fine, because `&mut foo.x` will be caught by // `MutatingUseContext::Borrow` elsewhere. MutatingUse(MutatingUseContext::Projection) + // These are just stores, where the storing is not propagatable, but there may be later + // mutations of the same local via `Store` + | MutatingUse(MutatingUseContext::Call) + // Actual store that can possibly even propagate a value | MutatingUse(MutatingUseContext::Store) => { if !self.found_assignment.insert(local) { match &mut self.can_const_prop[local] { @@ -799,7 +803,21 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { | NonMutatingUse(NonMutatingUseContext::Inspect) | NonMutatingUse(NonMutatingUseContext::Projection) | NonUse(_) => {} - _ => { + + // These could be propagated with a smarter analysis or just some careful thinking about + // whether they'd be fine right now. + MutatingUse(MutatingUseContext::AsmOutput) + | MutatingUse(MutatingUseContext::Yield) + | MutatingUse(MutatingUseContext::Drop) + | MutatingUse(MutatingUseContext::Retag) + // These can't ever be propagated under any scheme, as we can't reason about indirect + // mutation. + | NonMutatingUse(NonMutatingUseContext::SharedBorrow) + | NonMutatingUse(NonMutatingUseContext::ShallowBorrow) + | NonMutatingUse(NonMutatingUseContext::UniqueBorrow) + | NonMutatingUse(NonMutatingUseContext::AddressOf) + | MutatingUse(MutatingUseContext::Borrow) + | MutatingUse(MutatingUseContext::AddressOf) => { trace!("local {:?} can't be propagaged because it's used: {:?}", local, context); self.can_const_prop[local] = ConstPropMode::NoPropagation; } diff --git a/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff index b88bb9b491564..3cf05f1357835 100644 --- a/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff @@ -29,17 +29,26 @@ // + ty: i32 // + val: Value(Scalar(0x00000063)) // mir::Constant - // + span: $DIR/mutable_variable_aggregate_partial_read.rs:6:11: 6:13 +- // + span: $DIR/mutable_variable_aggregate_partial_read.rs:6:11: 6:13 ++ // + span: $DIR/mutable_variable_aggregate_partial_read.rs:6:5: 6:13 // + literal: Const { ty: i32, val: Value(Scalar(0x00000063)) } (_1.0: i32) = const 42i32; // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:7:5: 7:13 // ty::Const // + ty: i32 // + val: Value(Scalar(0x0000002a)) // mir::Constant - // + span: $DIR/mutable_variable_aggregate_partial_read.rs:7:11: 7:13 +- // + span: $DIR/mutable_variable_aggregate_partial_read.rs:7:11: 7:13 ++ // + span: $DIR/mutable_variable_aggregate_partial_read.rs:7:5: 7:13 // + literal: Const { ty: i32, val: Value(Scalar(0x0000002a)) } StorageLive(_2); // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:9: 8:10 - _2 = (_1.1: i32); // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:13: 8:16 +- _2 = (_1.1: i32); // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:13: 8:16 ++ _2 = const 99i32; // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:13: 8:16 ++ // ty::Const ++ // + ty: i32 ++ // + val: Value(Scalar(0x00000063)) ++ // mir::Constant ++ // + span: $DIR/mutable_variable_aggregate_partial_read.rs:8:13: 8:16 ++ // + literal: Const { ty: i32, val: Value(Scalar(0x00000063)) } _0 = const (); // scope 0 at $DIR/mutable_variable_aggregate_partial_read.rs:4:11: 9:2 // ty::Const // + ty: () From 0a785cdf34e2eba2200677aa2a397bc3ddeed270 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 12 May 2020 13:23:01 +0200 Subject: [PATCH 3/4] Add some more sanity tests and add a debug log message for it --- src/librustc_mir/transform/const_prop.rs | 5 ++ .../mutable_variable_unprop_assign.rs | 15 ++++ .../rustc.main.ConstProp.diff | 74 +++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 src/test/mir-opt/const_prop/mutable_variable_unprop_assign.rs create mode 100644 src/test/mir-opt/const_prop/mutable_variable_unprop_assign/rustc.main.ConstProp.diff diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index d66c909fe6cc5..88dfa90ac21dc 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -893,6 +893,11 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { // ``` // FIXME: we overzealously erase the entire local, because that's easier to // implement. + trace!( + "propagation into {:?} failed. + Nuking the entire site from orbit, it's the only way to be sure", + place, + ); Self::remove_const(&mut self.ecx, place.local); } } diff --git a/src/test/mir-opt/const_prop/mutable_variable_unprop_assign.rs b/src/test/mir-opt/const_prop/mutable_variable_unprop_assign.rs new file mode 100644 index 0000000000000..40f801b1b5e58 --- /dev/null +++ b/src/test/mir-opt/const_prop/mutable_variable_unprop_assign.rs @@ -0,0 +1,15 @@ +// compile-flags: -O + +// EMIT_MIR rustc.main.ConstProp.diff +fn main() { + let a = foo(); + let mut x: (i32, i32) = (1, 2); + x.1 = a; + let y = x.1; + let z = x.0; // this could theoretically be allowed, but we can't handle it right now +} + +#[inline(never)] +fn foo() -> i32 { + unimplemented!() +} diff --git a/src/test/mir-opt/const_prop/mutable_variable_unprop_assign/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/mutable_variable_unprop_assign/rustc.main.ConstProp.diff new file mode 100644 index 0000000000000..738343c655e36 --- /dev/null +++ b/src/test/mir-opt/const_prop/mutable_variable_unprop_assign/rustc.main.ConstProp.diff @@ -0,0 +1,74 @@ +- // MIR for `main` before ConstProp ++ // MIR for `main` after ConstProp + + fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/mutable_variable_unprop_assign.rs:4:11: 4:11 + let _1: i32; // in scope 0 at $DIR/mutable_variable_unprop_assign.rs:5:9: 5:10 + let mut _3: i32; // in scope 0 at $DIR/mutable_variable_unprop_assign.rs:7:11: 7:12 + scope 1 { + debug a => _1; // in scope 1 at $DIR/mutable_variable_unprop_assign.rs:5:9: 5:10 + let mut _2: (i32, i32) as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 1 at $DIR/mutable_variable_unprop_assign.rs:6:9: 6:14 + scope 2 { + debug x => _2; // in scope 2 at $DIR/mutable_variable_unprop_assign.rs:6:9: 6:14 + let _4: i32; // in scope 2 at $DIR/mutable_variable_unprop_assign.rs:8:9: 8:10 + scope 3 { + debug y => _4; // in scope 3 at $DIR/mutable_variable_unprop_assign.rs:8:9: 8:10 + let _5: i32; // in scope 3 at $DIR/mutable_variable_unprop_assign.rs:9:9: 9:10 + scope 4 { + debug z => _5; // in scope 4 at $DIR/mutable_variable_unprop_assign.rs:9:9: 9:10 + } + } + } + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/mutable_variable_unprop_assign.rs:5:9: 5:10 + _1 = const foo() -> bb1; // scope 0 at $DIR/mutable_variable_unprop_assign.rs:5:13: 5:18 + // ty::Const + // + ty: fn() -> i32 {foo} + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/mutable_variable_unprop_assign.rs:5:13: 5:16 + // + literal: Const { ty: fn() -> i32 {foo}, val: Value(Scalar()) } + } + + bb1: { + StorageLive(_2); // scope 1 at $DIR/mutable_variable_unprop_assign.rs:6:9: 6:14 + _2 = (const 1i32, const 2i32); // scope 1 at $DIR/mutable_variable_unprop_assign.rs:6:29: 6:35 + // ty::Const + // + ty: i32 + // + val: Value(Scalar(0x00000001)) + // mir::Constant +- // + span: $DIR/mutable_variable_unprop_assign.rs:6:30: 6:31 ++ // + span: $DIR/mutable_variable_unprop_assign.rs:6:29: 6:35 + // + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) } + // ty::Const + // + ty: i32 + // + val: Value(Scalar(0x00000002)) + // mir::Constant +- // + span: $DIR/mutable_variable_unprop_assign.rs:6:33: 6:34 ++ // + span: $DIR/mutable_variable_unprop_assign.rs:6:29: 6:35 + // + literal: Const { ty: i32, val: Value(Scalar(0x00000002)) } + StorageLive(_3); // scope 2 at $DIR/mutable_variable_unprop_assign.rs:7:11: 7:12 + _3 = _1; // scope 2 at $DIR/mutable_variable_unprop_assign.rs:7:11: 7:12 + (_2.1: i32) = move _3; // scope 2 at $DIR/mutable_variable_unprop_assign.rs:7:5: 7:12 + StorageDead(_3); // scope 2 at $DIR/mutable_variable_unprop_assign.rs:7:11: 7:12 + StorageLive(_4); // scope 2 at $DIR/mutable_variable_unprop_assign.rs:8:9: 8:10 + _4 = (_2.1: i32); // scope 2 at $DIR/mutable_variable_unprop_assign.rs:8:13: 8:16 + StorageLive(_5); // scope 3 at $DIR/mutable_variable_unprop_assign.rs:9:9: 9:10 + _5 = (_2.0: i32); // scope 3 at $DIR/mutable_variable_unprop_assign.rs:9:13: 9:16 + _0 = const (); // scope 0 at $DIR/mutable_variable_unprop_assign.rs:4:11: 10:2 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/mutable_variable_unprop_assign.rs:4:11: 10:2 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_5); // scope 3 at $DIR/mutable_variable_unprop_assign.rs:10:1: 10:2 + StorageDead(_4); // scope 2 at $DIR/mutable_variable_unprop_assign.rs:10:1: 10:2 + StorageDead(_2); // scope 1 at $DIR/mutable_variable_unprop_assign.rs:10:1: 10:2 + StorageDead(_1); // scope 0 at $DIR/mutable_variable_unprop_assign.rs:10:1: 10:2 + return; // scope 0 at $DIR/mutable_variable_unprop_assign.rs:10:2: 10:2 + } + } + From 27c818bc56afd28b60dd9f71af11773d177745c8 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sat, 16 May 2020 22:12:01 -0400 Subject: [PATCH 4/4] Bless mir-opt tests to account for #72220 --- .../rustc.main.ConstProp.diff | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff index 3cf05f1357835..6834bb6bdd4f7 100644 --- a/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/mutable_variable_aggregate_partial_read/rustc.main.ConstProp.diff @@ -29,16 +29,14 @@ // + ty: i32 // + val: Value(Scalar(0x00000063)) // mir::Constant -- // + span: $DIR/mutable_variable_aggregate_partial_read.rs:6:11: 6:13 -+ // + span: $DIR/mutable_variable_aggregate_partial_read.rs:6:5: 6:13 + // + span: $DIR/mutable_variable_aggregate_partial_read.rs:6:11: 6:13 // + literal: Const { ty: i32, val: Value(Scalar(0x00000063)) } (_1.0: i32) = const 42i32; // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:7:5: 7:13 // ty::Const // + ty: i32 // + val: Value(Scalar(0x0000002a)) // mir::Constant -- // + span: $DIR/mutable_variable_aggregate_partial_read.rs:7:11: 7:13 -+ // + span: $DIR/mutable_variable_aggregate_partial_read.rs:7:5: 7:13 + // + span: $DIR/mutable_variable_aggregate_partial_read.rs:7:11: 7:13 // + literal: Const { ty: i32, val: Value(Scalar(0x0000002a)) } StorageLive(_2); // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:9: 8:10 - _2 = (_1.1: i32); // scope 1 at $DIR/mutable_variable_aggregate_partial_read.rs:8:13: 8:16