Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable ConstGoto and SeparateConstSwitch passes by default #112040

Merged
merged 5 commits into from
Jun 1, 2023
Merged

Enable ConstGoto and SeparateConstSwitch passes by default #112040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc4d1e5
Stop marking locals as dead in ConstProp.
cjgillot May 21, 2023
4878688
Visit bodies in RPO for const-prop.
cjgillot May 28, 2023
89bf30e
Enable SeparateConstSwitch by default.
cjgillot May 21, 2023
e11e408
Enable ConstGoto pass by default.
cjgillot May 28, 2023
f74695b
Document handling of StorageDead.
cjgillot May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/const_goto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand Down
38 changes: 24 additions & 14 deletions compiler/rustc_mir_transform/src/const_prop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -886,14 +887,23 @@ 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) => {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
let frame = self.ecx.frame_mut();
frame.locals[local].value = LocalValue::Dead;
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.
_ => {}
}
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_mir_transform/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
&copy_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,
//
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/separate_const_switch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand Down
78 changes: 29 additions & 49 deletions tests/mir-opt/separate_const_switch.identity.SeparateConstSwitch.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,70 +9,61 @@
let mut _4: std::result::Result<i32, i32>; // 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<std::convert::Infallible, i32>; // 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<std::convert::Infallible, i32>; // 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<std::convert::Infallible, i32>; // 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] <Result<i32, i32> as FromResidual<Result<Infallible, i32>>>::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 <i32 as From<i32>>::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 <Result<i32, i32> 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<std::convert::Infallible, i32>; // in scope 5 at $SRC_DIR/core/src/result.rs:LL:COL
let mut _12: std::result::Result<std::convert::Infallible, i32>; // 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::<i32, i32>::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::<i32, i32>::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
}
Expand All @@ -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<std::convert::Infallible, i32>); // 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::<i32, i32>::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::<i32, i32>::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::<Infallible, i32>::Err(move _12); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
_3 = ControlFlow::<Result<Infallible, i32>, 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::<Infallible, i32>::Err(move _11); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
_3 = ControlFlow::<Result<Infallible, i32>, 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
}

Expand All @@ -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::<Result<Infallible, i32>, 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::<Result<Infallible, i32>, 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
}
}
Expand Down
21 changes: 3 additions & 18 deletions tests/mir-opt/separate_const_switch.too_complex.SeparateConstSwitch.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::<usize, i32>::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::<usize, i32>::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
}

Expand All @@ -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::<usize, i32>::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::<usize, i32>::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
}

Expand All @@ -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::<i32>::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::<i32>::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
}

Expand Down