From cc4d1e581e33c5563126bfc5b5eec57e479e0a9c Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 21 May 2023 13:33:56 +0000 Subject: [PATCH 1/5] Stop marking locals as dead in ConstProp. --- compiler/rustc_mir_transform/src/const_prop.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index a5d18fff89bd7..f7ab11e60c4b1 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -886,13 +886,7 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { } } StatementKind::StorageLive(local) => { - let frame = self.ecx.frame_mut(); - frame.locals[local].value = - LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)); - } - StatementKind::StorageDead(local) => { - let frame = self.ecx.frame_mut(); - frame.locals[local].value = LocalValue::Dead; + Self::remove_const(&mut self.ecx, local); } _ => {} } From 48786886fb486d70bb677ac8e3ce1647abc4c27a Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 28 May 2023 13:58:14 +0000 Subject: [PATCH 2/5] Visit bodies in RPO for const-prop. --- compiler/rustc_mir_transform/src/const_prop.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index f7ab11e60c4b1..39e26e4a8e44f 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -103,7 +103,14 @@ impl<'tcx> MirPass<'tcx> for ConstProp { // That would require a uniform one-def no-mutation analysis // and RPO (or recursing when needing the value of a local). let mut optimization_finder = ConstPropagator::new(body, dummy_body, tcx); - optimization_finder.visit_body(body); + + // Traverse the body in reverse post-order, to ensure that `FullConstProp` locals are + // assigned before being read. + let postorder = body.basic_blocks.postorder().to_vec(); + for bb in postorder.into_iter().rev() { + let data = &mut body.basic_blocks.as_mut_preserves_cfg()[bb]; + optimization_finder.visit_basic_block_data(bb, data); + } trace!("ConstProp done for {:?}", def_id); } @@ -790,12 +797,6 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { self.tcx } - fn visit_body(&mut self, body: &mut Body<'tcx>) { - for (bb, data) in body.basic_blocks.as_mut_preserves_cfg().iter_enumerated_mut() { - self.visit_basic_block_data(bb, data); - } - } - fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) { self.super_operand(operand, location); From 89bf30e73d970ba7d0a202d35c16566a27dcf527 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 21 May 2023 13:33:40 +0000 Subject: [PATCH 3/5] Enable SeparateConstSwitch by default. --- compiler/rustc_mir_transform/src/lib.rs | 5 +- .../src/separate_const_switch.rs | 2 +- ...t_switch.identity.SeparateConstSwitch.diff | 78 +++++++------------ ...witch.too_complex.SeparateConstSwitch.diff | 21 +---- 4 files changed, 37 insertions(+), 69 deletions(-) diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 54c138b6fbd4c..7d9f6c38e36a4 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -559,10 +559,13 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { // inst combine is after MatchBranchSimplification to clean up Ne(_1, false) &multiple_return_terminators::MultipleReturnTerminators, &instsimplify::InstSimplify, - &separate_const_switch::SeparateConstSwitch, &simplify::SimplifyLocals::BeforeConstProp, ©_prop::CopyProp, &ref_prop::ReferencePropagation, + // Perform `SeparateConstSwitch` after SSA-based analyses, as cloning blocks may + // destroy the SSA property. It should still happen before const-propagation, so the + // latter pass will leverage the created opportunities. + &separate_const_switch::SeparateConstSwitch, &const_prop::ConstProp, &dataflow_const_prop::DataflowConstProp, // diff --git a/compiler/rustc_mir_transform/src/separate_const_switch.rs b/compiler/rustc_mir_transform/src/separate_const_switch.rs index 2479856b727da..f35a5fb42768a 100644 --- a/compiler/rustc_mir_transform/src/separate_const_switch.rs +++ b/compiler/rustc_mir_transform/src/separate_const_switch.rs @@ -46,7 +46,7 @@ pub struct SeparateConstSwitch; impl<'tcx> MirPass<'tcx> for SeparateConstSwitch { fn is_enabled(&self, sess: &rustc_session::Session) -> bool { - sess.mir_opt_level() >= 4 + sess.mir_opt_level() >= 2 } fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { diff --git a/tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff b/tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff index bdf1de468b398..7f0e50a23f972 100644 --- a/tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff +++ b/tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff @@ -9,70 +9,61 @@ let mut _4: std::result::Result; // in scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:9 let mut _5: isize; // in scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 let _6: std::result::Result; // in scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 - let mut _7: !; // in scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 - let mut _8: std::result::Result; // in scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 - let _9: i32; // in scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 + let mut _7: std::result::Result; // in scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 + let _8: i32; // in scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 scope 1 { debug residual => _6; // in scope 1 at $DIR/separate_const_switch.rs:+1:9: +1:10 scope 2 { scope 8 (inlined #[track_caller] as FromResidual>>::from_residual) { // at $DIR/separate_const_switch.rs:25:8: 25:10 - debug residual => _8; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL - let _14: i32; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL - let mut _15: i32; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL + debug residual => _6; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL + let _13: i32; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL + let mut _14: i32; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL scope 9 { - debug e => _14; // in scope 9 at $SRC_DIR/core/src/result.rs:LL:COL + debug e => _13; // in scope 9 at $SRC_DIR/core/src/result.rs:LL:COL scope 10 (inlined >::from) { // at $SRC_DIR/core/src/result.rs:LL:COL - debug t => _14; // in scope 10 at $SRC_DIR/core/src/convert/mod.rs:LL:COL + debug t => _13; // in scope 10 at $SRC_DIR/core/src/convert/mod.rs:LL:COL } } } } } scope 3 { - debug val => _9; // in scope 3 at $DIR/separate_const_switch.rs:+1:8: +1:10 + debug val => _8; // in scope 3 at $DIR/separate_const_switch.rs:+1:8: +1:10 scope 4 { } } scope 5 (inlined as Try>::branch) { // at $DIR/separate_const_switch.rs:25:8: 25:10 - debug self => _4; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL - let mut _10: isize; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL + debug self => _1; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL + let mut _9: isize; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL + let _10: i32; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL let _11: i32; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL - let _12: i32; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL - let mut _13: std::result::Result; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL + let mut _12: std::result::Result; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL scope 6 { - debug v => _11; // in scope 6 at $SRC_DIR/core/src/result.rs:LL:COL + debug v => _10; // in scope 6 at $SRC_DIR/core/src/result.rs:LL:COL } scope 7 { - debug e => _12; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL + debug e => _11; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL } } bb0: { - StorageLive(_2); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 StorageLive(_3); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 - StorageLive(_4); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:9 - _4 = _1; // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:9 + StorageLive(_10); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 StorageLive(_11); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 - StorageLive(_12); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 - _10 = discriminant(_4); // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL - switchInt(move _10) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL + _9 = discriminant(_1); // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL + switchInt(move _9) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL } bb1: { - StorageDead(_12); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 StorageDead(_11); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 - StorageDead(_4); // scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 + StorageDead(_10); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 _5 = discriminant(_3); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 switchInt(move _5) -> [0: bb2, 1: bb4, otherwise: bb3]; // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 } bb2: { - StorageLive(_9); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 - _9 = ((_3 as Continue).0: i32); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 - _2 = _9; // scope 4 at $DIR/separate_const_switch.rs:+1:8: +1:10 - StorageDead(_9); // scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 - _0 = Result::::Ok(move _2); // scope 0 at $DIR/separate_const_switch.rs:+1:5: +1:11 - StorageDead(_2); // scope 0 at $DIR/separate_const_switch.rs:+1:10: +1:11 + _8 = ((_3 as Continue).0: i32); // scope 0 at $DIR/separate_const_switch.rs:+1:8: +1:10 + _0 = Result::::Ok(_8); // scope 0 at $DIR/separate_const_switch.rs:+1:5: +1:11 StorageDead(_3); // scope 0 at $DIR/separate_const_switch.rs:+2:1: +2:2 return; // scope 0 at $DIR/separate_const_switch.rs:+2:2: +2:2 } @@ -82,30 +73,19 @@ } bb4: { - StorageLive(_6); // scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 _6 = ((_3 as Break).0: std::result::Result); // scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 - StorageLive(_8); // scope 2 at $DIR/separate_const_switch.rs:+1:9: +1:10 - _8 = _6; // scope 2 at $DIR/separate_const_switch.rs:+1:9: +1:10 - StorageLive(_14); // scope 2 at $DIR/separate_const_switch.rs:+1:8: +1:10 - _14 = move ((_8 as Err).0: i32); // scope 8 at $SRC_DIR/core/src/result.rs:LL:COL - StorageLive(_15); // scope 9 at $SRC_DIR/core/src/result.rs:LL:COL - _15 = move _14; // scope 10 at $SRC_DIR/core/src/convert/mod.rs:LL:COL - _0 = Result::::Err(move _15); // scope 9 at $SRC_DIR/core/src/result.rs:LL:COL - StorageDead(_15); // scope 9 at $SRC_DIR/core/src/result.rs:LL:COL - StorageDead(_14); // scope 2 at $DIR/separate_const_switch.rs:+1:8: +1:10 - StorageDead(_8); // scope 2 at $DIR/separate_const_switch.rs:+1:9: +1:10 - StorageDead(_6); // scope 0 at $DIR/separate_const_switch.rs:+1:9: +1:10 - StorageDead(_2); // scope 0 at $DIR/separate_const_switch.rs:+1:10: +1:11 + _13 = ((_6 as Err).0: i32); // scope 8 at $SRC_DIR/core/src/result.rs:LL:COL + _0 = Result::::Err(move _13); // scope 9 at $SRC_DIR/core/src/result.rs:LL:COL StorageDead(_3); // scope 0 at $DIR/separate_const_switch.rs:+2:1: +2:2 return; // scope 0 at $DIR/separate_const_switch.rs:+2:2: +2:2 } bb5: { - _12 = move ((_4 as Err).0: i32); // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL - StorageLive(_13); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL - _13 = Result::::Err(move _12); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL - _3 = ControlFlow::, i32>::Break(move _13); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL - StorageDead(_13); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL + _11 = ((_1 as Err).0: i32); // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL + StorageLive(_12); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL + _12 = Result::::Err(move _11); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL + _3 = ControlFlow::, i32>::Break(move _12); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL + StorageDead(_12); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL goto -> bb1; // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL } @@ -114,8 +94,8 @@ } bb7: { - _11 = move ((_4 as Ok).0: i32); // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL - _3 = ControlFlow::, i32>::Continue(move _11); // scope 6 at $SRC_DIR/core/src/result.rs:LL:COL + _10 = ((_1 as Ok).0: i32); // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL + _3 = ControlFlow::, i32>::Continue(move _10); // scope 6 at $SRC_DIR/core/src/result.rs:LL:COL goto -> bb1; // scope 5 at $SRC_DIR/core/src/result.rs:LL:COL } } diff --git a/tests/mir-opt/separate_const_switch.too_complex.SeparateConstSwitch.diff b/tests/mir-opt/separate_const_switch.too_complex.SeparateConstSwitch.diff index b5e0a66d83f6e..f86a96dec4155 100644 --- a/tests/mir-opt/separate_const_switch.too_complex.SeparateConstSwitch.diff +++ b/tests/mir-opt/separate_const_switch.too_complex.SeparateConstSwitch.diff @@ -34,13 +34,8 @@ } bb1: { - StorageLive(_6); // scope 0 at $DIR/separate_const_switch.rs:+8:17: +8:18 _6 = ((_1 as Err).0: usize); // scope 0 at $DIR/separate_const_switch.rs:+8:17: +8:18 - StorageLive(_7); // scope 2 at $DIR/separate_const_switch.rs:+8:42: +8:43 - _7 = _6; // scope 2 at $DIR/separate_const_switch.rs:+8:42: +8:43 - _2 = ControlFlow::::Break(move _7); // scope 2 at $DIR/separate_const_switch.rs:+8:23: +8:44 - StorageDead(_7); // scope 2 at $DIR/separate_const_switch.rs:+8:43: +8:44 - StorageDead(_6); // scope 0 at $DIR/separate_const_switch.rs:+8:43: +8:44 + _2 = ControlFlow::::Break(_6); // scope 2 at $DIR/separate_const_switch.rs:+8:23: +8:44 goto -> bb4; // scope 0 at $DIR/separate_const_switch.rs:+8:43: +8:44 } @@ -49,13 +44,8 @@ } bb3: { - StorageLive(_4); // scope 0 at $DIR/separate_const_switch.rs:+7:16: +7:17 _4 = ((_1 as Ok).0: i32); // scope 0 at $DIR/separate_const_switch.rs:+7:16: +7:17 - StorageLive(_5); // scope 1 at $DIR/separate_const_switch.rs:+7:44: +7:45 - _5 = _4; // scope 1 at $DIR/separate_const_switch.rs:+7:44: +7:45 - _2 = ControlFlow::::Continue(move _5); // scope 1 at $DIR/separate_const_switch.rs:+7:22: +7:46 - StorageDead(_5); // scope 1 at $DIR/separate_const_switch.rs:+7:45: +7:46 - StorageDead(_4); // scope 0 at $DIR/separate_const_switch.rs:+7:45: +7:46 + _2 = ControlFlow::::Continue(_4); // scope 1 at $DIR/separate_const_switch.rs:+7:22: +7:46 goto -> bb4; // scope 0 at $DIR/separate_const_switch.rs:+7:45: +7:46 } @@ -73,13 +63,8 @@ } bb6: { - StorageLive(_9); // scope 0 at $DIR/separate_const_switch.rs:+11:31: +11:32 _9 = ((_2 as Continue).0: i32); // scope 0 at $DIR/separate_const_switch.rs:+11:31: +11:32 - StorageLive(_10); // scope 3 at $DIR/separate_const_switch.rs:+11:42: +11:43 - _10 = _9; // scope 3 at $DIR/separate_const_switch.rs:+11:42: +11:43 - _0 = Option::::Some(move _10); // scope 3 at $DIR/separate_const_switch.rs:+11:37: +11:44 - StorageDead(_10); // scope 3 at $DIR/separate_const_switch.rs:+11:43: +11:44 - StorageDead(_9); // scope 0 at $DIR/separate_const_switch.rs:+11:43: +11:44 + _0 = Option::::Some(_9); // scope 3 at $DIR/separate_const_switch.rs:+11:37: +11:44 goto -> bb7; // scope 0 at $DIR/separate_const_switch.rs:+11:43: +11:44 } From e11e4081df2a23914e1552b82b8e4b8c7bc457cb Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 28 May 2023 13:51:57 +0000 Subject: [PATCH 4/5] Enable ConstGoto pass by default. --- compiler/rustc_mir_transform/src/const_goto.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/const_goto.rs b/compiler/rustc_mir_transform/src/const_goto.rs index da101ca7ad279..024bea620982a 100644 --- a/compiler/rustc_mir_transform/src/const_goto.rs +++ b/compiler/rustc_mir_transform/src/const_goto.rs @@ -28,7 +28,7 @@ pub struct ConstGoto; impl<'tcx> MirPass<'tcx> for ConstGoto { fn is_enabled(&self, sess: &rustc_session::Session) -> bool { - sess.mir_opt_level() >= 4 + sess.mir_opt_level() >= 2 } fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { From f74695b01005e63583c32f0334ee8eb9557dec00 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 31 May 2023 17:28:24 +0000 Subject: [PATCH 5/5] Document handling of StorageDead. --- compiler/rustc_mir_transform/src/const_prop.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 39e26e4a8e44f..40c7b5a164597 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -889,6 +889,21 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { StatementKind::StorageLive(local) => { Self::remove_const(&mut self.ecx, local); } + // We do not need to mark dead locals as such. For `FullConstProp` locals, + // this allows to propagate the single assigned value in this case: + // ``` + // let x = SOME_CONST; + // if a { + // f(copy x); + // StorageDead(x); + // } else { + // g(copy x); + // StorageDead(x); + // } + // ``` + // + // This may propagate a constant where the local would be uninit or dead. + // In both cases, this does not matter, as those reads would be UB anyway. _ => {} } }