Skip to content

Allow StorageDead(s) of the target block could be copied to the branching block #93987

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 16 additions & 6 deletions compiler/rustc_mir_transform/src/const_goto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ impl<'tcx> MirPass<'tcx> for ConstGoto {
let new_goto = TerminatorKind::Goto { target: opt.target_to_use_in_goto };
debug!("SUCCESS: replacing `{:?}` with `{:?}`", terminator.kind, new_goto);
terminator.kind = new_goto;

let bridge_bb_statement = &body.basic_blocks()[opt.bridge_bb].statements;
if !bridge_bb_statement.is_empty() {
let storagedeads = bridge_bb_statement.clone();
body.basic_blocks_mut()[opt.bb_with_goto].statements.extend(storagedeads);
Comment on lines +47 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let bridge_bb_statement = &body.basic_blocks()[opt.bridge_bb].statements;
if !bridge_bb_statement.is_empty() {
let storagedeads = bridge_bb_statement.clone();
body.basic_blocks_mut()[opt.bb_with_goto].statements.extend(storagedeads);
let bridge_bb_statements = &body.basic_blocks()[opt.bridge_bb].statements;
if !bridge_bb_statements.is_empty() {
let storagedeads = bridge_bb_statements.into_iter().cloned();
body.basic_blocks_mut()[opt.bb_with_goto].statements.extend(storagedeads);

debug!("SUCCESS: extending terminator's basic block with briding basic block.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
debug!("SUCCESS: extending terminator's basic block with briding basic block.");
debug!("SUCCESS: extending terminator's basic block with bridging basic block.");

}
}

// if we applied optimizations, we potentially have some cfg to cleanup to
Expand All @@ -67,12 +74,13 @@ impl<'tcx> Visitor<'tcx> for ConstGotoOptimizationFinder<'_, 'tcx> {
// 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() {
let target_bb_statements = &target_bb.statements;
if !target_bb_statements.is_empty()
&& !target_bb_statements.iter().all(|statement| match statement.kind {
StatementKind::StorageDead(_) => true,
_ => false,
})
{
None?
}

Expand All @@ -86,6 +94,7 @@ impl<'tcx> Visitor<'tcx> for ConstGotoOptimizationFinder<'_, 'tcx> {
let target_to_use_in_goto = targets.target_for_value(const_value);
self.optimizations.push(OptimizationToApply {
bb_with_goto: location.block,
bridge_bb: target,
target_to_use_in_goto,
});
}
Expand All @@ -99,6 +108,7 @@ impl<'tcx> Visitor<'tcx> for ConstGotoOptimizationFinder<'_, 'tcx> {

struct OptimizationToApply {
bb_with_goto: BasicBlock,
bridge_bb: BasicBlock,
target_to_use_in_goto: BasicBlock,
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
- // MIR for `match_nested_if` before MatchBranchSimplification
+ // MIR for `match_nested_if` after MatchBranchSimplification

fn match_nested_if() -> bool {
let mut _0: bool; // return place in scope 0 at $DIR/const_goto.rs:16:25: 16:29
let _1: bool; // in scope 0 at $DIR/const_goto.rs:17:9: 17:12
let mut _2: bool; // in scope 0 at $DIR/const_goto.rs:18:24: 18:28
+ let mut _3: bool; // in scope 0 at $DIR/const_goto.rs:18:24: 18:28
scope 1 {
debug val => _1; // in scope 1 at $DIR/const_goto.rs:17:9: 17:12
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/const_goto.rs:17:9: 17:12
StorageLive(_2); // scope 0 at $DIR/const_goto.rs:18:24: 18:28
_2 = const true; // scope 0 at $DIR/const_goto.rs:18:24: 18:28
- switchInt(move _2) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/const_goto.rs:18:24: 18:28
- }
-
- bb1: {
+ StorageLive(_3); // scope 0 at $DIR/const_goto.rs:18:24: 18:28
+ _3 = move _2; // scope 0 at $DIR/const_goto.rs:18:24: 18:28
StorageDead(_2); // scope 0 at $DIR/const_goto.rs:18:51: 18:52
- _1 = const true; // scope 0 at $DIR/const_goto.rs:24:13: 24:17
- goto -> bb3; // scope 0 at $DIR/const_goto.rs:24:13: 24:17
- }
-
- bb2: {
- StorageDead(_2); // scope 0 at $DIR/const_goto.rs:18:51: 18:52
- _1 = const false; // scope 0 at $DIR/const_goto.rs:26:14: 26:19
- goto -> bb3; // scope 0 at $DIR/const_goto.rs:26:14: 26:19
- }
-
- bb3: {
+ _1 = Ne(_3, const false); // scope 0 at $DIR/const_goto.rs:26:14: 26:19
+ StorageDead(_3); // scope 0 at $DIR/const_goto.rs:18:24: 18:28
_0 = _1; // scope 1 at $DIR/const_goto.rs:28:5: 28:8
StorageDead(_1); // scope 0 at $DIR/const_goto.rs:29:1: 29:2
return; // scope 0 at $DIR/const_goto.rs:29:2: 29:2
}
}

18 changes: 18 additions & 0 deletions src/test/mir-opt/const_goto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,24 @@ pub enum Foo {
fn issue_77355_opt(num: Foo) -> u64 {
if matches!(num, Foo::B | Foo::C) { 23 } else { 42 }
}

// EMIT_MIR const_goto.match_nested_if.MatchBranchSimplification.diff
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// EMIT_MIR const_goto.match_nested_if.MatchBranchSimplification.diff
// EMIT_MIR const_goto.match_nested_if.MatchBranchSimplification.diff
// EMIT_MIR const_goto.match_nested_if.ConstGoto.diff

fn match_nested_if() -> bool {
let val = match () {
() if if if if true { true } else { false } { true } else { false } {
true
} else {
false
} =>
{
true
}
_ => false,
};
val
}

fn main() {
issue_77355_opt(Foo::A);
let _ = match_nested_if();
}
Original file line number Diff line number Diff line change
@@ -1,85 +1,114 @@
- // MIR for `match_nested_if` before MatchBranchSimplification
+ // MIR for `match_nested_if` after MatchBranchSimplification

fn match_nested_if() -> bool {
let mut _0: bool; // return place in scope 0 at $DIR/matches_reduce_branches.rs:39:25: 39:29
let _1: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:9: 40:12
let mut _2: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
let mut _3: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
let mut _4: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
+ let mut _5: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
+ let mut _6: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
+ let mut _7: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
fn match_nested_if(_1: bool, _2: bool) -> bool {
debug flag0 => _1; // in scope 0 at $DIR/matches_reduce_branches.rs:39:20: 39:25
debug flag1 => _2; // in scope 0 at $DIR/matches_reduce_branches.rs:39:33: 39:38
let mut _0: bool; // return place in scope 0 at $DIR/matches_reduce_branches.rs:39:49: 39:53
let _3: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:9: 40:12
let mut _4: (); // in scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:23
let mut _5: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
let mut _6: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:78
let mut _7: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:53
let mut _8: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
let mut _9: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:42:13: 42:18
let mut _10: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:42:21: 42:26
let mut _11: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:44:13: 44:18
let mut _12: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:44:21: 44:26
+ let mut _13: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
scope 1 {
debug val => _1; // in scope 1 at $DIR/matches_reduce_branches.rs:40:9: 40:12
debug val => _3; // in scope 1 at $DIR/matches_reduce_branches.rs:40:9: 40:12
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/matches_reduce_branches.rs:40:9: 40:12
StorageLive(_2); // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
StorageLive(_3); // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
StorageLive(_4); // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
_4 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
- switchInt(move _4) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
- }
-
- bb1: {
- _3 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:41:31: 41:35
- goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
- }
-
- bb2: {
- _3 = const false; // scope 0 at $DIR/matches_reduce_branches.rs:41:45: 41:50
- goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
- }
-
- bb3: {
+ StorageLive(_5); // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
+ _5 = move _4; // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
+ _3 = Ne(_5, const false); // scope 0 at $DIR/matches_reduce_branches.rs:41:45: 41:50
+ StorageDead(_5); // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
StorageDead(_4); // scope 0 at $DIR/matches_reduce_branches.rs:41:51: 41:52
- switchInt(move _3) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
- }
-
- bb4: {
- _2 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:41:55: 41:59
- goto -> bb6; // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
- }
-
- bb5: {
- _2 = const false; // scope 0 at $DIR/matches_reduce_branches.rs:41:69: 41:74
- goto -> bb6; // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
- }
-
- bb6: {
+ StorageLive(_6); // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
+ _6 = move _3; // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
+ _2 = Ne(_6, const false); // scope 0 at $DIR/matches_reduce_branches.rs:41:69: 41:74
+ StorageDead(_6); // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:52
StorageDead(_3); // scope 0 at $DIR/matches_reduce_branches.rs:41:75: 41:76
- switchInt(move _2) -> [false: bb8, otherwise: bb7]; // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
StorageLive(_3); // scope 0 at $DIR/matches_reduce_branches.rs:40:9: 40:12
StorageLive(_4); // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:23
StorageLive(_5); // scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
StorageLive(_6); // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:78
StorageLive(_7); // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:53
StorageLive(_8); // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
_8 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
switchInt(move _8) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/matches_reduce_branches.rs:41:24: 41:28
}

bb1: {
_7 = _1; // scope 0 at $DIR/matches_reduce_branches.rs:41:31: 41:36
goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:53
}

bb2: {
_7 = _2; // scope 0 at $DIR/matches_reduce_branches.rs:41:46: 41:51
goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:53
}

bb3: {
StorageDead(_8); // scope 0 at $DIR/matches_reduce_branches.rs:41:52: 41:53
switchInt(move _7) -> [false: bb5, otherwise: bb4]; // scope 0 at $DIR/matches_reduce_branches.rs:41:21: 41:53
}

bb4: {
_6 = _2; // scope 0 at $DIR/matches_reduce_branches.rs:41:56: 41:61
goto -> bb6; // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:78
}

bb5: {
_6 = _1; // scope 0 at $DIR/matches_reduce_branches.rs:41:71: 41:76
goto -> bb6; // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:78
}

bb6: {
StorageDead(_7); // scope 0 at $DIR/matches_reduce_branches.rs:41:77: 41:78
switchInt(move _6) -> [false: bb8, otherwise: bb7]; // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:78
}

bb7: {
StorageLive(_9); // scope 0 at $DIR/matches_reduce_branches.rs:42:13: 42:18
_9 = _1; // scope 0 at $DIR/matches_reduce_branches.rs:42:13: 42:18
StorageLive(_10); // scope 0 at $DIR/matches_reduce_branches.rs:42:21: 42:26
_10 = _2; // scope 0 at $DIR/matches_reduce_branches.rs:42:21: 42:26
_5 = BitAnd(move _9, move _10); // scope 0 at $DIR/matches_reduce_branches.rs:42:13: 42:26
StorageDead(_10); // scope 0 at $DIR/matches_reduce_branches.rs:42:25: 42:26
StorageDead(_9); // scope 0 at $DIR/matches_reduce_branches.rs:42:25: 42:26
goto -> bb9; // scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
}

bb8: {
StorageLive(_11); // scope 0 at $DIR/matches_reduce_branches.rs:44:13: 44:18
_11 = _1; // scope 0 at $DIR/matches_reduce_branches.rs:44:13: 44:18
StorageLive(_12); // scope 0 at $DIR/matches_reduce_branches.rs:44:21: 44:26
_12 = _2; // scope 0 at $DIR/matches_reduce_branches.rs:44:21: 44:26
_5 = BitOr(move _11, move _12); // scope 0 at $DIR/matches_reduce_branches.rs:44:13: 44:26
StorageDead(_12); // scope 0 at $DIR/matches_reduce_branches.rs:44:25: 44:26
StorageDead(_11); // scope 0 at $DIR/matches_reduce_branches.rs:44:25: 44:26
goto -> bb9; // scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
}

bb9: {
- switchInt(move _5) -> [false: bb11, otherwise: bb10]; // scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
- }
-
- bb7: {
+ StorageLive(_7); // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
+ _7 = move _2; // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
StorageDead(_2); // scope 0 at $DIR/matches_reduce_branches.rs:45:9: 45:10
- _1 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:47:13: 47:17
- goto -> bb9; // scope 0 at $DIR/matches_reduce_branches.rs:47:13: 47:17
- bb10: {
+ StorageLive(_13); // scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
+ _13 = move _5; // scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
StorageDead(_6); // scope 0 at $DIR/matches_reduce_branches.rs:45:9: 45:10
StorageDead(_5); // scope 0 at $DIR/matches_reduce_branches.rs:45:9: 45:10
- _3 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:47:13: 47:17
- goto -> bb12; // scope 0 at $DIR/matches_reduce_branches.rs:47:13: 47:17
- }
-
- bb8: {
- StorageDead(_2); // scope 0 at $DIR/matches_reduce_branches.rs:45:9: 45:10
- _1 = const false; // scope 0 at $DIR/matches_reduce_branches.rs:49:14: 49:19
- goto -> bb9; // scope 0 at $DIR/matches_reduce_branches.rs:49:14: 49:19
- bb11: {
- StorageDead(_6); // scope 0 at $DIR/matches_reduce_branches.rs:45:9: 45:10
- StorageDead(_5); // scope 0 at $DIR/matches_reduce_branches.rs:45:9: 45:10
- _3 = const false; // scope 0 at $DIR/matches_reduce_branches.rs:49:14: 49:19
- goto -> bb12; // scope 0 at $DIR/matches_reduce_branches.rs:49:14: 49:19
- }
-
- bb9: {
+ _1 = Ne(_7, const false); // scope 0 at $DIR/matches_reduce_branches.rs:49:14: 49:19
+ StorageDead(_7); // scope 0 at $DIR/matches_reduce_branches.rs:41:18: 41:76
_0 = _1; // scope 1 at $DIR/matches_reduce_branches.rs:51:5: 51:8
StorageDead(_1); // scope 0 at $DIR/matches_reduce_branches.rs:52:1: 52:2
- bb12: {
+ _3 = Ne(_13, const false); // scope 0 at $DIR/matches_reduce_branches.rs:49:14: 49:19
+ StorageDead(_13); // scope 0 at $DIR/matches_reduce_branches.rs:41:15: 45:10
StorageDead(_4); // scope 0 at $DIR/matches_reduce_branches.rs:50:6: 50:7
_0 = _3; // scope 1 at $DIR/matches_reduce_branches.rs:51:5: 51:8
StorageDead(_3); // scope 0 at $DIR/matches_reduce_branches.rs:52:1: 52:2
return; // scope 0 at $DIR/matches_reduce_branches.rs:52:2: 52:2
}
}
Expand Down
Loading