-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
New mir-opt pass to simplify gotos with const values #77486
Closed
Closed
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
f24c0ea
New mir-opt pass to simplify gotos with const values
simonvandel 4f5226a
Handle const eval failure
simonvandel a6960c8
Use try block
simonvandel d8712dc
Add fixme about a possible future relaxation
simonvandel c7fa844
Introduce StatementKind::as_assign
simonvandel e3b8d5c
Introduce TerminatorKind::as_switch
simonvandel 75b7447
Introduce TerminatorKind::as_goto
simonvandel 2803ce0
Clearer destructuring
simonvandel 15e0098
Add a more direct for not_equals optimization in InstCombine
simonvandel bf511c1
Tidy fix
simonvandel 1fe09b0
Ref instead of box
simonvandel 569f01c
pattern match directly on constant
simonvandel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
//! This pass optimizes the following sequence | ||
//! ```rust | ||
//! bb2: { | ||
//! _2 = const true; | ||
//! goto -> bb3; | ||
//! } | ||
//! | ||
//! bb3: { | ||
//! switchInt(_2) -> [false: bb4, otherwise: bb5]; | ||
//! } | ||
//! ``` | ||
//! into | ||
//! ```rust | ||
//! bb2: { | ||
//! _2 = const true; | ||
//! goto -> bb5; | ||
//! } | ||
//! ``` | ||
|
||
use crate::transform::MirPass; | ||
use rustc_middle::mir::*; | ||
use rustc_middle::ty::TyCtxt; | ||
use rustc_middle::{mir::visit::Visitor, ty::ParamEnv}; | ||
|
||
use super::simplify::{simplify_cfg, simplify_locals}; | ||
|
||
pub struct ConstGoto; | ||
|
||
impl<'tcx> MirPass<'tcx> for ConstGoto { | ||
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { | ||
trace!("Running ConstGoto on {:?}", body.source); | ||
let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id()); | ||
let mut opt_finder = | ||
ConstGotoOptimizationFinder { tcx, body, optimizations: vec![], param_env }; | ||
opt_finder.visit_body(body); | ||
let should_simplify = !opt_finder.optimizations.is_empty(); | ||
for opt in opt_finder.optimizations { | ||
let terminator = body.basic_blocks_mut()[opt.bb_with_goto].terminator_mut(); | ||
let new_goto = TerminatorKind::Goto { target: opt.target_to_use_in_goto }; | ||
debug!("SUCCESS: replacing `{:?}` with `{:?}`", terminator.kind, new_goto); | ||
terminator.kind = new_goto; | ||
} | ||
|
||
// if we applied optimizations, we potentially have some cfg to cleanup to | ||
// make it easier for further passes | ||
if should_simplify { | ||
simplify_cfg(body); | ||
simplify_locals(body, tcx); | ||
} | ||
} | ||
} | ||
|
||
impl<'a, 'tcx> Visitor<'tcx> for ConstGotoOptimizationFinder<'a, 'tcx> { | ||
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { | ||
let _: Option<_> = try { | ||
let target = terminator.kind.as_goto()?; | ||
// We only apply this optimization if the last statement is a const assignment | ||
let last_statement = self.body.basic_blocks()[location.block].statements.last()?; | ||
|
||
if let (place, Rvalue::Use(Operand::Constant(_const))) = | ||
last_statement.kind.as_assign()? | ||
{ | ||
// We found a constant being assigned to `place`. | ||
// Now check that the target of this Goto switches on this place. | ||
let target_bb = &self.body.basic_blocks()[target]; | ||
|
||
// FIXME(simonvandel): We are conservative here when we don't allow | ||
// any statements in the target basic block. | ||
// This could probably be relaxed to allow `StorageDead`s which could be | ||
// copied to the predecessor of this block. | ||
if !target_bb.statements.is_empty() { | ||
None? | ||
} | ||
|
||
let target_bb_terminator = target_bb.terminator(); | ||
let (discr, switch_ty, targets) = target_bb_terminator.kind.as_switch()?; | ||
if discr.place() == Some(*place) { | ||
// We now know that the Switch matches on the const place, and it is statementless | ||
// Now find which value in the Switch matches the const value. | ||
let const_value = | ||
_const.literal.try_eval_bits(self.tcx, self.param_env, switch_ty)?; | ||
let found_value_idx_option = targets | ||
.iter() | ||
.enumerate() | ||
.find(|(_, (value, _))| const_value == *value) | ||
.map(|(idx, _)| idx); | ||
|
||
let target_to_use_in_goto = | ||
if let Some(found_value_idx) = found_value_idx_option { | ||
targets.iter().nth(found_value_idx).unwrap().1 | ||
} else { | ||
// If we did not find the const value in values, it must be the otherwise case | ||
targets.otherwise() | ||
}; | ||
|
||
self.optimizations.push(OptimizationToApply { | ||
bb_with_goto: location.block, | ||
target_to_use_in_goto, | ||
}); | ||
} | ||
} | ||
Some(()) | ||
}; | ||
|
||
self.super_terminator(terminator, location); | ||
} | ||
} | ||
|
||
struct OptimizationToApply { | ||
bb_with_goto: BasicBlock, | ||
target_to_use_in_goto: BasicBlock, | ||
} | ||
|
||
pub struct ConstGotoOptimizationFinder<'a, 'tcx> { | ||
tcx: TyCtxt<'tcx>, | ||
body: &'a Body<'tcx>, | ||
param_env: ParamEnv<'tcx>, | ||
optimizations: Vec<OptimizationToApply>, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
52 changes: 52 additions & 0 deletions
52
src/test/mir-opt/const_goto.issue_77355_opt.ConstGoto.diff
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
- // MIR for `issue_77355_opt` before ConstGoto | ||
+ // MIR for `issue_77355_opt` after ConstGoto | ||
|
||
fn issue_77355_opt(_1: Foo) -> u64 { | ||
debug num => _1; // in scope 0 at $DIR/const_goto.rs:11:20: 11:23 | ||
let mut _0: u64; // return place in scope 0 at $DIR/const_goto.rs:11:33: 11:36 | ||
- let mut _2: bool; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL | ||
- let mut _3: isize; // in scope 0 at $DIR/const_goto.rs:12:22: 12:28 | ||
+ let mut _2: isize; // in scope 0 at $DIR/const_goto.rs:12:22: 12:28 | ||
|
||
bb0: { | ||
- StorageLive(_2); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL | ||
- _3 = discriminant(_1); // scope 0 at $DIR/const_goto.rs:12:22: 12:28 | ||
- switchInt(move _3) -> [1_isize: bb2, 2_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/const_goto.rs:12:22: 12:28 | ||
+ _2 = discriminant(_1); // scope 0 at $DIR/const_goto.rs:12:22: 12:28 | ||
+ switchInt(move _2) -> [1_isize: bb2, 2_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/const_goto.rs:12:22: 12:28 | ||
} | ||
|
||
bb1: { | ||
- _2 = const false; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL | ||
- goto -> bb3; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL | ||
- } | ||
- | ||
- bb2: { | ||
- _2 = const true; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL | ||
- goto -> bb3; // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL | ||
- } | ||
- | ||
- bb3: { | ||
- switchInt(_2) -> [false: bb4, otherwise: bb5]; // scope 0 at $DIR/const_goto.rs:12:5: 12:57 | ||
- } | ||
- | ||
- bb4: { | ||
_0 = const 42_u64; // scope 0 at $DIR/const_goto.rs:12:53: 12:55 | ||
- goto -> bb6; // scope 0 at $DIR/const_goto.rs:12:5: 12:57 | ||
+ goto -> bb3; // scope 0 at $DIR/const_goto.rs:12:5: 12:57 | ||
} | ||
|
||
- bb5: { | ||
+ bb2: { | ||
_0 = const 23_u64; // scope 0 at $DIR/const_goto.rs:12:41: 12:43 | ||
- goto -> bb6; // scope 0 at $DIR/const_goto.rs:12:5: 12:57 | ||
+ goto -> bb3; // scope 0 at $DIR/const_goto.rs:12:5: 12:57 | ||
} | ||
|
||
- bb6: { | ||
- StorageDead(_2); // scope 0 at $DIR/const_goto.rs:13:1: 13:2 | ||
+ bb3: { | ||
return; // scope 0 at $DIR/const_goto.rs:13:2: 13:2 | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
pub enum Foo { | ||
A, | ||
B, | ||
C, | ||
D, | ||
E, | ||
F, | ||
} | ||
|
||
// EMIT_MIR const_goto.issue_77355_opt.ConstGoto.diff | ||
fn issue_77355_opt(num: Foo) -> u64 { | ||
if matches!(num, Foo::B | Foo::C) { 23 } else { 42 } | ||
} | ||
fn main() { | ||
issue_77355_opt(Foo::A); | ||
} |
51 changes: 51 additions & 0 deletions
51
src/test/mir-opt/const_goto_const_eval_fail.f.ConstGoto.diff
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
- // MIR for `f` before ConstGoto | ||
+ // MIR for `f` after ConstGoto | ||
|
||
fn f() -> u64 { | ||
let mut _0: u64; // return place in scope 0 at $DIR/const_goto_const_eval_fail.rs:6:44: 6:47 | ||
let mut _1: bool; // in scope 0 at $DIR/const_goto_const_eval_fail.rs:7:11: 12:6 | ||
let mut _2: i32; // in scope 0 at $DIR/const_goto_const_eval_fail.rs:8:15: 8:16 | ||
|
||
bb0: { | ||
StorageLive(_1); // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:11: 12:6 | ||
StorageLive(_2); // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:15: 8:16 | ||
_2 = const A; // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:15: 8:16 | ||
switchInt(_2) -> [1_i32: bb2, 2_i32: bb2, 3_i32: bb2, otherwise: bb1]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:9:13: 9:14 | ||
} | ||
|
||
bb1: { | ||
_1 = const true; // scope 0 at $DIR/const_goto_const_eval_fail.rs:10:18: 10:22 | ||
goto -> bb3; // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:9: 11:10 | ||
} | ||
|
||
bb2: { | ||
_1 = const B; // scope 0 at $DIR/const_goto_const_eval_fail.rs:9:26: 9:27 | ||
- goto -> bb3; // scope 0 at $DIR/const_goto_const_eval_fail.rs:8:9: 11:10 | ||
+ switchInt(_1) -> [false: bb4, otherwise: bb3]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:13:9: 13:14 | ||
} | ||
|
||
bb3: { | ||
- switchInt(_1) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/const_goto_const_eval_fail.rs:13:9: 13:14 | ||
- } | ||
- | ||
- bb4: { | ||
_0 = const 2_u64; // scope 0 at $DIR/const_goto_const_eval_fail.rs:14:17: 14:18 | ||
- goto -> bb6; // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 15:6 | ||
+ goto -> bb5; // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 15:6 | ||
} | ||
|
||
- bb5: { | ||
+ bb4: { | ||
_0 = const 1_u64; // scope 0 at $DIR/const_goto_const_eval_fail.rs:13:18: 13:19 | ||
- goto -> bb6; // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 15:6 | ||
+ goto -> bb5; // scope 0 at $DIR/const_goto_const_eval_fail.rs:7:5: 15:6 | ||
} | ||
|
||
- bb6: { | ||
+ bb5: { | ||
StorageDead(_2); // scope 0 at $DIR/const_goto_const_eval_fail.rs:16:1: 16:2 | ||
StorageDead(_1); // scope 0 at $DIR/const_goto_const_eval_fail.rs:16:1: 16:2 | ||
return; // scope 0 at $DIR/const_goto_const_eval_fail.rs:16:2: 16:2 | ||
} | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @rust-lang/wg-mir-opt we need to figure out a way to dump the mir of an optimizations both before its internal cleanup and after. The test diffs are very noisy if we do the cleanup together with the optimization.
Maybe we could not do these immediate cleanups, and instead leave a flag in the mir body that states that it needs a simplification and the following cleanup optimizations just skip if the flag is not set?