From c06876c90437c7eecd1dc246d3515d6211397f16 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 22 Apr 2020 21:31:51 -0400 Subject: [PATCH 1/2] [const-prop] Remove `ConstPropMode::NoPropagation` This mode is unnecessary because it's always ok to evaluate the right-hand side of assignments even if the left-hand side should not have reads propagated. --- src/librustc_mir/transform/const_prop.rs | 47 +++++++++---------- .../rustc.main.ConstProp.diff | 6 ++- .../32bit/rustc.main.SimplifyArmIdentity.diff | 9 ++-- .../64bit/rustc.main.SimplifyArmIdentity.diff | 9 ++-- .../rustc.main.SimplifyArmIdentity.diff | 11 ++--- .../rustc.map.SimplifyLocals.diff | 8 +++- 6 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 83ed2fc2d439b..4dbe6642a3043 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -721,8 +721,6 @@ enum ConstPropMode { OnlyInsideOwnBlock, /// The `Local` can be propagated into but reads cannot be propagated. OnlyPropagateInto, - /// No propagation is allowed at all. - NoPropagation, } struct CanConstProp { @@ -793,7 +791,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { "local {:?} can't be propagated because of multiple assignments", local, ); - *other = ConstPropMode::NoPropagation; + *other = ConstPropMode::OnlyPropagateInto; } } } @@ -820,7 +818,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { | 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; + self.can_const_prop[local] = ConstPropMode::OnlyPropagateInto; } } } @@ -852,31 +850,28 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { if let Ok(place_layout) = self.tcx.layout_of(self.param_env.and(place_ty)) { let can_const_prop = self.can_const_prop[place.local]; if let Some(()) = self.const_prop(rval, place_layout, source_info, place) { - if can_const_prop != ConstPropMode::NoPropagation { - // This will return None for variables that are from other blocks, - // so it should be okay to propagate from here on down. - if let Some(value) = self.get_const(place) { - if self.should_const_prop(value) { - trace!("replacing {:?} with {:?}", rval, value); - self.replace_with_const(rval, value, source_info); - if can_const_prop == ConstPropMode::FullConstProp - || can_const_prop == ConstPropMode::OnlyInsideOwnBlock - { - trace!("propagated into {:?}", place); - } - } - if can_const_prop == ConstPropMode::OnlyInsideOwnBlock { - trace!( - "found local restricted to its block. Will remove it from const-prop after block is finished. Local: {:?}", - place.local - ); - self.locals_of_current_block.insert(place.local); + // This will return None for variables that are from other blocks, + // so it should be okay to propagate from here on down. + if let Some(value) = self.get_const(place) { + if self.should_const_prop(value) { + trace!("replacing {:?} with {:?}", rval, value); + self.replace_with_const(rval, value, source_info); + if can_const_prop == ConstPropMode::FullConstProp + || can_const_prop == ConstPropMode::OnlyInsideOwnBlock + { + trace!("propagated into {:?}", place); } } } - if can_const_prop == ConstPropMode::OnlyPropagateInto - || can_const_prop == ConstPropMode::NoPropagation - { + if can_const_prop == ConstPropMode::OnlyInsideOwnBlock { + trace!( + "found local restricted to its block. Will remove it from const-prop after block is finished. Local: {:?}", + place.local + ); + self.locals_of_current_block.insert(place.local); + } + + if can_const_prop == ConstPropMode::OnlyPropagateInto { trace!("can't propagate into {:?}", place); if place.local != RETURN_PLACE { Self::remove_const(&mut self.ecx, place.local); diff --git a/src/test/mir-opt/const_prop/mutable_variable_aggregate_mut_ref/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/mutable_variable_aggregate_mut_ref/rustc.main.ConstProp.diff index 44203ac327ab1..0d703068d41f4 100644 --- a/src/test/mir-opt/const_prop/mutable_variable_aggregate_mut_ref/rustc.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/mutable_variable_aggregate_mut_ref/rustc.main.ConstProp.diff @@ -23,13 +23,15 @@ // + ty: i32 // + val: Value(Scalar(0x0000002a)) // mir::Constant - // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:18: 5:20 +- // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:18: 5:20 ++ // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:17: 5:25 // + literal: Const { ty: i32, val: Value(Scalar(0x0000002a)) } // ty::Const // + ty: i32 // + val: Value(Scalar(0x0000002b)) // mir::Constant - // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:22: 5:24 +- // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:22: 5:24 ++ // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:17: 5:25 // + literal: Const { ty: i32, val: Value(Scalar(0x0000002b)) } StorageLive(_2); // scope 1 at $DIR/mutable_variable_aggregate_mut_ref.rs:6:9: 6:10 _2 = &mut _1; // scope 1 at $DIR/mutable_variable_aggregate_mut_ref.rs:6:13: 6:19 diff --git a/src/test/mir-opt/simplify-arm-identity/32bit/rustc.main.SimplifyArmIdentity.diff b/src/test/mir-opt/simplify-arm-identity/32bit/rustc.main.SimplifyArmIdentity.diff index dfd6d6f0f2ecd..94759dca038b1 100644 --- a/src/test/mir-opt/simplify-arm-identity/32bit/rustc.main.SimplifyArmIdentity.diff +++ b/src/test/mir-opt/simplify-arm-identity/32bit/rustc.main.SimplifyArmIdentity.diff @@ -39,14 +39,13 @@ } bb1: { - ((_2 as Foo).0: u8) = const 0u8; // scope 1 at $DIR/simplify-arm-identity.rs:21:21: 21:32 + _2 = const Dst::Foo(0u8); // scope 1 at $DIR/simplify-arm-identity.rs:21:21: 21:32 // ty::Const - // + ty: u8 + // + ty: Dst // + val: Value(Scalar(0x00)) // mir::Constant - // + span: $DIR/simplify-arm-identity.rs:21:30: 21:31 - // + literal: Const { ty: u8, val: Value(Scalar(0x00)) } - discriminant(_2) = 0; // scope 1 at $DIR/simplify-arm-identity.rs:21:21: 21:32 + // + span: $DIR/simplify-arm-identity.rs:21:21: 21:32 + // + literal: Const { ty: Dst, val: Value(Scalar(0x00)) } goto -> bb4; // scope 1 at $DIR/simplify-arm-identity.rs:19:18: 22:6 } diff --git a/src/test/mir-opt/simplify-arm-identity/64bit/rustc.main.SimplifyArmIdentity.diff b/src/test/mir-opt/simplify-arm-identity/64bit/rustc.main.SimplifyArmIdentity.diff index f2bbd19586993..ba21f16b685d4 100644 --- a/src/test/mir-opt/simplify-arm-identity/64bit/rustc.main.SimplifyArmIdentity.diff +++ b/src/test/mir-opt/simplify-arm-identity/64bit/rustc.main.SimplifyArmIdentity.diff @@ -39,14 +39,13 @@ } bb1: { - ((_2 as Foo).0: u8) = const 0u8; // scope 1 at $DIR/simplify-arm-identity.rs:21:21: 21:32 + _2 = const Dst::Foo(0u8); // scope 1 at $DIR/simplify-arm-identity.rs:21:21: 21:32 // ty::Const - // + ty: u8 + // + ty: Dst // + val: Value(Scalar(0x00)) // mir::Constant - // + span: $DIR/simplify-arm-identity.rs:21:30: 21:31 - // + literal: Const { ty: u8, val: Value(Scalar(0x00)) } - discriminant(_2) = 0; // scope 1 at $DIR/simplify-arm-identity.rs:21:21: 21:32 + // + span: $DIR/simplify-arm-identity.rs:21:21: 21:32 + // + literal: Const { ty: Dst, val: Value(Scalar(0x00)) } goto -> bb4; // scope 1 at $DIR/simplify-arm-identity.rs:19:18: 22:6 } diff --git a/src/test/mir-opt/simplify-arm-identity/rustc.main.SimplifyArmIdentity.diff b/src/test/mir-opt/simplify-arm-identity/rustc.main.SimplifyArmIdentity.diff index b2517cb7012b4..e7373391b79c7 100644 --- a/src/test/mir-opt/simplify-arm-identity/rustc.main.SimplifyArmIdentity.diff +++ b/src/test/mir-opt/simplify-arm-identity/rustc.main.SimplifyArmIdentity.diff @@ -33,15 +33,14 @@ } bb1: { - ((_2 as Foo).0: u8) = const 0u8; // scope 1 at $DIR/simplify-arm-identity.rs:20:21: 20:32 + _2 = const Dst::Foo(0u8); // bb1[0]: scope 1 at $DIR/simplify-arm-identity.rs:20:21: 20:32 // ty::Const - // + ty: u8 + // + ty: Dst // + val: Value(Scalar(0x00)) // mir::Constant - // + span: $DIR/simplify-arm-identity.rs:20:30: 20:31 - // + literal: Const { ty: u8, val: Value(Scalar(0x00)) } - discriminant(_2) = 0; // scope 1 at $DIR/simplify-arm-identity.rs:20:21: 20:32 - goto -> bb4; // scope 1 at $DIR/simplify-arm-identity.rs:18:18: 21:6 + // + span: $DIR/simplify-arm-identity.rs:20:21: 20:32 + // + literal: Const { ty: Dst, val: Value(Scalar(0x00)) } + goto -> bb4; // bb1[1]: scope 1 at $DIR/simplify-arm-identity.rs:18:18: 21:6 } bb2: { diff --git a/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/rustc.map.SimplifyLocals.diff b/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/rustc.map.SimplifyLocals.diff index 0ca54af85e3b6..a97fa98a7b09e 100644 --- a/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/rustc.map.SimplifyLocals.diff +++ b/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/rustc.map.SimplifyLocals.diff @@ -24,7 +24,13 @@ } bb2: { - discriminant(_0) = 0; // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:3:17: 3:21 + _0 = const std::option::Option::>::None; // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:3:17: 3:21 + // ty::Const + // + ty: std::option::Option> + // + val: Value(Scalar(0x0000000000000000)) + // mir::Constant + // + span: $DIR/simplify-locals-removes-unused-discriminant-reads.rs:3:17: 3:21 + // + literal: Const { ty: std::option::Option>, val: Value(Scalar(0x0000000000000000)) } goto -> bb3; // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:2:5: 5:6 } From 2f49d554ff1afd1633a01d6b84192ad0147d9097 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 5 May 2020 09:44:08 -0400 Subject: [PATCH 2/2] Add EMIR_MIR_FOR_EACH_BIT_WIDTH to failing test --- ...ocals-removes-unused-discriminant-reads.rs | 1 + .../32bit/rustc.map.SimplifyLocals.diff | 42 +++++++++++++++++++ .../{ => 64bit}/rustc.map.SimplifyLocals.diff | 0 3 files changed, 43 insertions(+) create mode 100644 src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/32bit/rustc.map.SimplifyLocals.diff rename src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/{ => 64bit}/rustc.map.SimplifyLocals.diff (100%) diff --git a/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads.rs b/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads.rs index 067fa879b4038..7047b542aa607 100644 --- a/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads.rs +++ b/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads.rs @@ -9,4 +9,5 @@ fn main() { map(None); } +// EMIT_MIR_FOR_EACH_BIT_WIDTH // EMIT_MIR rustc.map.SimplifyLocals.diff diff --git a/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/32bit/rustc.map.SimplifyLocals.diff b/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/32bit/rustc.map.SimplifyLocals.diff new file mode 100644 index 0000000000000..2f78671763d51 --- /dev/null +++ b/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/32bit/rustc.map.SimplifyLocals.diff @@ -0,0 +1,42 @@ +- // MIR for `map` before SimplifyLocals ++ // MIR for `map` after SimplifyLocals + + fn map(_1: std::option::Option>) -> std::option::Option> { + debug x => _1; // in scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:1:8: 1:9 + let mut _0: std::option::Option>; // return place in scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:1:31: 1:46 + let mut _2: isize; // in scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:3:9: 3:13 + let _3: std::boxed::Box<()>; // in scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:4:14: 4:15 +- let mut _4: std::boxed::Box<()>; // in scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:4:25: 4:26 +- let mut _5: isize; // in scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:6:1: 6:2 +- let mut _6: isize; // in scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:6:1: 6:2 + scope 1 { + debug x => _3; // in scope 1 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:4:14: 4:15 + } + + bb0: { + _2 = discriminant(_1); // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:3:9: 3:13 + switchInt(move _2) -> [0isize: bb2, otherwise: bb1]; // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:3:9: 3:13 + } + + bb1: { + _0 = move _1; // scope 1 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:4:20: 4:27 + goto -> bb3; // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:2:5: 5:6 + } + + bb2: { + _0 = const std::option::Option::>::None; // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:3:17: 3:21 + // ty::Const + // + ty: std::option::Option> + // + val: Value(Scalar(0x00000000)) + // mir::Constant + // + span: $DIR/simplify-locals-removes-unused-discriminant-reads.rs:3:17: 3:21 + // + literal: Const { ty: std::option::Option>, val: Value(Scalar(0x00000000)) } + goto -> bb3; // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:2:5: 5:6 + } + + bb3: { +- _5 = discriminant(_1); // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:6:1: 6:2 + return; // scope 0 at $DIR/simplify-locals-removes-unused-discriminant-reads.rs:6:2: 6:2 + } + } + diff --git a/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/rustc.map.SimplifyLocals.diff b/src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/64bit/rustc.map.SimplifyLocals.diff similarity index 100% rename from src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/rustc.map.SimplifyLocals.diff rename to src/test/mir-opt/simplify-locals-removes-unused-discriminant-reads/64bit/rustc.map.SimplifyLocals.diff