Skip to content

Commit 597b4c5

Browse files
jonas-schievinkjonas-schievink
authored
Rollup merge of #78191 - tmiasko:temp-match-branch-simplification, r=oli-obk
Introduce a temporary for discriminant value in MatchBranchSimplification The optimization introduces additional uses of the discriminant operand, but does not ensure that it is still valid to evaluate it or that it still evaluates to the same value. Evaluate it once at original position, and store the result in a new temporary. Follow up on #78151. The optimization remains disabled by default. Closes #78239.
2 parents e3808ed + a4dc92b commit 597b4c5

9 files changed

+309
-48
lines changed

‎compiler/rustc_mir/src/transform/match_branches.rs

+22-8
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,16 @@ pub struct MatchBranchSimplification;
3838
3939
impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
4040
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
41-
// FIXME: This optimization can result in unsoundness, because it introduces
42-
// additional uses of a place holding the discriminant value without ensuring that
43-
// it is valid to do so.
44-
if !tcx.sess.opts.debugging_opts.unsound_mir_opts {
41+
if tcx.sess.opts.debugging_opts.mir_opt_level <= 1 {
4542
return;
4643
}
4744

4845
let param_env = tcx.param_env(body.source.def_id());
49-
let bbs = body.basic_blocks_mut();
46+
let (bbs, local_decls) = body.basic_blocks_and_local_decls_mut();
5047
'outer: for bb_idx in bbs.indices() {
5148
let (discr, val, switch_ty, first, second) = match bbs[bb_idx].terminator().kind {
5249
TerminatorKind::SwitchInt {
53-
discr: Operand::Copy(ref place) | Operand::Move(ref place),
50+
discr: ref discr @ (Operand::Copy(_) | Operand::Move(_)),
5451
switch_ty,
5552
ref targets,
5653
..
@@ -59,7 +56,7 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
5956
if target == targets.otherwise() {
6057
continue;
6158
}
62-
(place, value, switch_ty, target, targets.otherwise())
59+
(discr, value, switch_ty, target, targets.otherwise())
6360
}
6461
// Only optimize switch int statements
6562
_ => continue,
@@ -99,6 +96,10 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
9996
// Take ownership of items now that we know we can optimize.
10097
let discr = discr.clone();
10198

99+
// Introduce a temporary for the discriminant value.
100+
let source_info = bbs[bb_idx].terminator().source_info;
101+
let discr_local = local_decls.push(LocalDecl::new(switch_ty, source_info.span));
102+
102103
// We already checked that first and second are different blocks,
103104
// and bb_idx has a different terminator from both of them.
104105
let (from, first, second) = bbs.pick3_mut(bb_idx, first, second);
@@ -127,7 +128,11 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
127128
rustc_span::DUMMY_SP,
128129
);
129130
let op = if f_b { BinOp::Eq } else { BinOp::Ne };
130-
let rhs = Rvalue::BinaryOp(op, Operand::Copy(discr.clone()), const_cmp);
131+
let rhs = Rvalue::BinaryOp(
132+
op,
133+
Operand::Copy(Place::from(discr_local)),
134+
const_cmp,
135+
);
131136
Statement {
132137
source_info: f.source_info,
133138
kind: StatementKind::Assign(box (*lhs, rhs)),
@@ -138,7 +143,16 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
138143
_ => unreachable!(),
139144
}
140145
});
146+
147+
from.statements
148+
.push(Statement { source_info, kind: StatementKind::StorageLive(discr_local) });
149+
from.statements.push(Statement {
150+
source_info,
151+
kind: StatementKind::Assign(box (Place::from(discr_local), Rvalue::Use(discr))),
152+
});
141153
from.statements.extend(new_stmts);
154+
from.statements
155+
.push(Statement { source_info, kind: StatementKind::StorageDead(discr_local) });
142156
from.terminator_mut().kind = first.terminator().kind.clone();
143157
}
144158
}

‎src/test/mir-opt/matches_reduce_branches.bar.MatchBranchSimplification.32bit.diff

+6-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
let mut _8: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:35:9: 35:10
1111
let mut _9: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:35:12: 35:13
1212
let mut _10: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:35:15: 35:16
13+
+ let mut _11: i32; // in scope 0 at $DIR/matches_reduce_branches.rs:19:9: 19:10
1314
scope 1 {
1415
debug a => _2; // in scope 1 at $DIR/matches_reduce_branches.rs:13:9: 13:10
1516
let _3: bool; // in scope 1 at $DIR/matches_reduce_branches.rs:14:9: 14:10
@@ -33,10 +34,13 @@
3334
StorageLive(_5); // scope 3 at $DIR/matches_reduce_branches.rs:16:9: 16:10
3435
StorageLive(_6); // scope 4 at $DIR/matches_reduce_branches.rs:18:5: 33:6
3536
- switchInt(_1) -> [7_i32: bb2, otherwise: bb1]; // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
36-
+ _2 = Ne(_1, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:20:13: 20:22
37-
+ _3 = Eq(_1, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:21:13: 21:21
37+
+ StorageLive(_11); // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
38+
+ _11 = _1; // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
39+
+ _2 = Ne(_11, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:20:13: 20:22
40+
+ _3 = Eq(_11, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:21:13: 21:21
3841
+ _4 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:22:13: 22:22
3942
+ _5 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:23:13: 23:21
43+
+ StorageDead(_11); // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
4044
+ goto -> bb3; // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
4145
}
4246

‎src/test/mir-opt/matches_reduce_branches.bar.MatchBranchSimplification.64bit.diff

+6-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
let mut _8: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:35:9: 35:10
1111
let mut _9: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:35:12: 35:13
1212
let mut _10: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:35:15: 35:16
13+
+ let mut _11: i32; // in scope 0 at $DIR/matches_reduce_branches.rs:19:9: 19:10
1314
scope 1 {
1415
debug a => _2; // in scope 1 at $DIR/matches_reduce_branches.rs:13:9: 13:10
1516
let _3: bool; // in scope 1 at $DIR/matches_reduce_branches.rs:14:9: 14:10
@@ -33,10 +34,13 @@
3334
StorageLive(_5); // scope 3 at $DIR/matches_reduce_branches.rs:16:9: 16:10
3435
StorageLive(_6); // scope 4 at $DIR/matches_reduce_branches.rs:18:5: 33:6
3536
- switchInt(_1) -> [7_i32: bb2, otherwise: bb1]; // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
36-
+ _2 = Ne(_1, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:20:13: 20:22
37-
+ _3 = Eq(_1, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:21:13: 21:21
37+
+ StorageLive(_11); // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
38+
+ _11 = _1; // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
39+
+ _2 = Ne(_11, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:20:13: 20:22
40+
+ _3 = Eq(_11, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:21:13: 21:21
3841
+ _4 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:22:13: 22:22
3942
+ _5 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:23:13: 23:21
43+
+ StorageDead(_11); // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
4044
+ goto -> bb3; // scope 4 at $DIR/matches_reduce_branches.rs:19:9: 19:10
4145
}
4246

‎src/test/mir-opt/matches_reduce_branches.foo.MatchBranchSimplification.32bit.diff

+5-1
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@
66
let mut _0: (); // return place in scope 0 at $DIR/matches_reduce_branches.rs:6:25: 6:25
77
let mut _2: bool; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
88
let mut _3: isize; // in scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
9+
+ let mut _4: isize; // in scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
910

1011
bb0: {
1112
StorageLive(_2); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
1213
_3 = discriminant(_1); // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
1314
- switchInt(move _3) -> [0_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
14-
+ _2 = Eq(_3, const 0_isize); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
15+
+ StorageLive(_4); // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
16+
+ _4 = move _3; // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
17+
+ _2 = Eq(_4, const 0_isize); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
18+
+ StorageDead(_4); // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
1519
+ goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
1620
}
1721

‎src/test/mir-opt/matches_reduce_branches.foo.MatchBranchSimplification.64bit.diff

+5-1
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@
66
let mut _0: (); // return place in scope 0 at $DIR/matches_reduce_branches.rs:6:25: 6:25
77
let mut _2: bool; // in scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
88
let mut _3: isize; // in scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
9+
+ let mut _4: isize; // in scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
910

1011
bb0: {
1112
StorageLive(_2); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
1213
_3 = discriminant(_1); // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
1314
- switchInt(move _3) -> [0_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
14-
+ _2 = Eq(_3, const 0_isize); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
15+
+ StorageLive(_4); // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
16+
+ _4 = move _3; // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
17+
+ _2 = Eq(_4, const 0_isize); // scope 0 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
18+
+ StorageDead(_4); // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
1519
+ goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:7:22: 7:26
1620
}
1721

‎src/test/mir-opt/matches_reduce_branches.match_nested_if.MatchBranchSimplification.32bit.diff

+116
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
- // MIR for `match_nested_if` before MatchBranchSimplification
2+
+ // MIR for `match_nested_if` after MatchBranchSimplification
3+
4+
fn match_nested_if() -> bool {
5+
let mut _0: bool; // return place in scope 0 at $DIR/matches_reduce_branches.rs:38:25: 38:29
6+
let _1: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:39:9: 39:12
7+
let mut _2: (); // in scope 0 at $DIR/matches_reduce_branches.rs:39:21: 39:23
8+
let mut _3: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
9+
let mut _4: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
10+
let mut _5: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
11+
let mut _6: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:24: 40:28
12+
+ let mut _7: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
13+
+ let mut _8: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
14+
+ let mut _9: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
15+
+ let mut _10: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
16+
scope 1 {
17+
debug val => _1; // in scope 1 at $DIR/matches_reduce_branches.rs:39:9: 39:12
18+
}
19+
20+
bb0: {
21+
StorageLive(_1); // scope 0 at $DIR/matches_reduce_branches.rs:39:9: 39:12
22+
StorageLive(_2); // scope 0 at $DIR/matches_reduce_branches.rs:39:21: 39:23
23+
StorageLive(_3); // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
24+
StorageLive(_4); // scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
25+
StorageLive(_5); // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
26+
StorageLive(_6); // scope 0 at $DIR/matches_reduce_branches.rs:40:24: 40:28
27+
_6 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:40:24: 40:28
28+
- switchInt(_6) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
29+
+ StorageLive(_7); // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
30+
+ _7 = _6; // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
31+
+ _5 = Ne(_7, const false); // scope 0 at $DIR/matches_reduce_branches.rs:40:42: 40:47
32+
+ StorageDead(_7); // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
33+
+ goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
34+
}
35+
36+
bb1: {
37+
_5 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:40:30: 40:34
38+
goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
39+
}
40+
41+
bb2: {
42+
_5 = const false; // scope 0 at $DIR/matches_reduce_branches.rs:40:42: 40:47
43+
goto -> bb3; // scope 0 at $DIR/matches_reduce_branches.rs:40:21: 40:48
44+
}
45+
46+
bb3: {
47+
StorageDead(_6); // scope 0 at $DIR/matches_reduce_branches.rs:40:47: 40:48
48+
- switchInt(_5) -> [false: bb4, otherwise: bb5]; // scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
49+
+ StorageLive(_8); // scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
50+
+ _8 = _5; // scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
51+
+ _4 = Ne(_8, const false); // scope 0 at $DIR/matches_reduce_branches.rs:40:62: 40:67
52+
+ StorageDead(_8); // scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
53+
+ goto -> bb6; // scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
54+
}
55+
56+
bb4: {
57+
_4 = const false; // scope 0 at $DIR/matches_reduce_branches.rs:40:62: 40:67
58+
goto -> bb6; // scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
59+
}
60+
61+
bb5: {
62+
_4 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:40:50: 40:54
63+
goto -> bb6; // scope 0 at $DIR/matches_reduce_branches.rs:40:18: 40:68
64+
}
65+
66+
bb6: {
67+
StorageDead(_5); // scope 0 at $DIR/matches_reduce_branches.rs:40:67: 40:68
68+
- switchInt(_4) -> [false: bb7, otherwise: bb8]; // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
69+
+ StorageLive(_9); // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
70+
+ _9 = _4; // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
71+
+ _3 = Ne(_9, const false); // scope 0 at $DIR/matches_reduce_branches.rs:40:82: 40:87
72+
+ StorageDead(_9); // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
73+
+ goto -> bb9; // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
74+
}
75+
76+
bb7: {
77+
_3 = const false; // scope 0 at $DIR/matches_reduce_branches.rs:40:82: 40:87
78+
goto -> bb9; // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
79+
}
80+
81+
bb8: {
82+
_3 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:40:70: 40:74
83+
goto -> bb9; // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
84+
}
85+
86+
bb9: {
87+
StorageDead(_4); // scope 0 at $DIR/matches_reduce_branches.rs:40:87: 40:88
88+
- switchInt(move _3) -> [false: bb11, otherwise: bb10]; // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
89+
+ StorageLive(_10); // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
90+
+ _10 = move _3; // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
91+
+ StorageDead(_3); // scope 0 at $DIR/matches_reduce_branches.rs:40:95: 40:96
92+
+ _1 = Ne(_10, const false); // scope 0 at $DIR/matches_reduce_branches.rs:41:14: 41:19
93+
+ StorageDead(_10); // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
94+
+ goto -> bb12; // scope 0 at $DIR/matches_reduce_branches.rs:40:15: 40:88
95+
}
96+
97+
bb10: {
98+
StorageDead(_3); // scope 0 at $DIR/matches_reduce_branches.rs:40:95: 40:96
99+
_1 = const true; // scope 0 at $DIR/matches_reduce_branches.rs:40:92: 40:96
100+
goto -> bb12; // scope 0 at $DIR/matches_reduce_branches.rs:39:15: 42:6
101+
}
102+
103+
bb11: {
104+
StorageDead(_3); // scope 0 at $DIR/matches_reduce_branches.rs:40:95: 40:96
105+
_1 = const false; // scope 0 at $DIR/matches_reduce_branches.rs:41:14: 41:19
106+
goto -> bb12; // scope 0 at $DIR/matches_reduce_branches.rs:39:15: 42:6
107+
}
108+
109+
bb12: {
110+
StorageDead(_2); // scope 0 at $DIR/matches_reduce_branches.rs:42:6: 42:7
111+
_0 = _1; // scope 1 at $DIR/matches_reduce_branches.rs:43:5: 43:8
112+
StorageDead(_1); // scope 0 at $DIR/matches_reduce_branches.rs:44:1: 44:2
113+
return; // scope 0 at $DIR/matches_reduce_branches.rs:44:2: 44:2
114+
}
115+
}
116+

0 commit comments

Comments
 (0)