Skip to content

Commit 1fe2e29

Browse files
tmiaskotmiasko
committed
MatchBranchSimplification: fix equal const bool assignments
The match branch simplification is applied when target blocks contain statements that are either equal or perform a const bool assignment with different values to the same place. Previously, when constructing new statements, only statements from a single block had been examined. This lead to a misoptimization when statements are equal because the assign the *same* const bool value to the same place. Fix the issue by examining statements from both blocks when deciding on replacement.
1 parent 4fae049 commit 1fe2e29

6 files changed

+447
-59
lines changed

‎src/librustc_mir/transform/match_branches.rs

+82-35
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,37 @@ use rustc_middle::ty::TyCtxt;
44

55
pub struct MatchBranchSimplification;
66

7-
// What's the intent of this pass?
8-
// If one block is found that switches between blocks which both go to the same place
9-
// AND both of these blocks set a similar const in their ->
10-
// condense into 1 block based on discriminant AND goto the destination afterwards
7+
/// If a source block is found that switches between two blocks that are exactly
8+
/// the same modulo const bool assignments (e.g., one assigns true another false
9+
/// to the same place), merge a target block statements into the source block,
10+
/// using Eq / Ne comparison with switch value where const bools value differ.
11+
///
12+
/// For example:
13+
///
14+
/// ```rust
15+
/// bb0: {
16+
/// switchInt(move _3) -> [42_isize: bb1, otherwise: bb2];
17+
/// }
18+
///
19+
/// bb1: {
20+
/// _2 = const true;
21+
/// goto -> bb3;
22+
/// }
23+
///
24+
/// bb2: {
25+
/// _2 = const false;
26+
/// goto -> bb3;
27+
/// }
28+
/// ```
29+
///
30+
/// into:
31+
///
32+
/// ```rust
33+
/// bb0: {
34+
/// _2 = Eq(move _3, const 42_isize);
35+
/// goto -> bb3;
36+
/// }
37+
/// ```
1138
1239
impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
1340
fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) {
@@ -42,48 +69,68 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
4269
}
4370
for (f, s) in first_stmts.iter().zip(scnd_stmts.iter()) {
4471
match (&f.kind, &s.kind) {
45-
// If two statements are exactly the same just ignore them.
46-
(f_s, s_s) if f_s == s_s => (),
72+
// If two statements are exactly the same, we can optimize.
73+
(f_s, s_s) if f_s == s_s => {}
4774

75+
// If two statements are const bool assignments to the same place, we can optimize.
4876
(
4977
StatementKind::Assign(box (lhs_f, Rvalue::Use(Operand::Constant(f_c)))),
5078
StatementKind::Assign(box (lhs_s, Rvalue::Use(Operand::Constant(s_c)))),
51-
) if lhs_f == lhs_s && f_c.literal.ty.is_bool() && s_c.literal.ty.is_bool() => {
52-
let f_c = f_c.literal.try_eval_bool(tcx, param_env).unwrap();
53-
let s_c = s_c.literal.try_eval_bool(tcx, param_env).unwrap();
54-
if f_c != s_c {
55-
// have to check this here because f_c & s_c might have
56-
// different spans.
57-
continue;
58-
}
59-
continue 'outer;
60-
}
61-
// If there are not exclusively assignments, then ignore this
79+
) if lhs_f == lhs_s
80+
&& f_c.literal.ty.is_bool()
81+
&& s_c.literal.ty.is_bool()
82+
&& f_c.literal.try_eval_bool(tcx, param_env).is_some()
83+
&& s_c.literal.try_eval_bool(tcx, param_env).is_some() => {}
84+
85+
// Otherwise we cannot optimize. Try another block.
6286
_ => continue 'outer,
6387
}
6488
}
65-
// Take owenership of items now that we know we can optimize.
89+
// Take ownership of items now that we know we can optimize.
6690
let discr = discr.clone();
67-
let (from, first) = bbs.pick2_mut(bb_idx, first);
6891

69-
let new_stmts = first.statements.iter().cloned().map(|mut s| {
70-
if let StatementKind::Assign(box (_, ref mut rhs)) = s.kind {
71-
if let Rvalue::Use(Operand::Constant(c)) = rhs {
72-
let size = tcx.layout_of(param_env.and(switch_ty)).unwrap().size;
73-
let const_cmp = Operand::const_from_scalar(
74-
tcx,
75-
switch_ty,
76-
crate::interpret::Scalar::from_uint(val, size),
77-
rustc_span::DUMMY_SP,
78-
);
79-
if let Some(c) = c.literal.try_eval_bool(tcx, param_env) {
80-
let op = if c { BinOp::Eq } else { BinOp::Ne };
81-
*rhs = Rvalue::BinaryOp(op, Operand::Copy(discr), const_cmp);
92+
let new_stmts = first_stmts
93+
.iter()
94+
.zip(scnd_stmts.iter())
95+
.map(|(f, s)| {
96+
match (&f.kind, &s.kind) {
97+
(f_s, s_s) if f_s == s_s => (*f).clone(),
98+
99+
(
100+
StatementKind::Assign(box (lhs, Rvalue::Use(Operand::Constant(f_c)))),
101+
StatementKind::Assign(box (_, Rvalue::Use(Operand::Constant(s_c)))),
102+
) => {
103+
// From earlier loop we know that we are dealing with bool constants only:
104+
let f_b = f_c.literal.try_eval_bool(tcx, param_env).unwrap();
105+
let s_b = s_c.literal.try_eval_bool(tcx, param_env).unwrap();
106+
if f_b == s_b {
107+
// Same value in both blocks. Use statement as is.
108+
(*f).clone()
109+
} else {
110+
// Different value between blocks. Make value conditional on switch condition.
111+
let size = tcx.layout_of(param_env.and(switch_ty)).unwrap().size;
112+
let const_cmp = Operand::const_from_scalar(
113+
tcx,
114+
switch_ty,
115+
crate::interpret::Scalar::from_uint(val, size),
116+
rustc_span::DUMMY_SP,
117+
);
118+
let op = if f_b { BinOp::Eq } else { BinOp::Ne };
119+
let rhs =
120+
Rvalue::BinaryOp(op, Operand::Copy(discr.clone()), const_cmp);
121+
Statement {
122+
source_info: f.source_info,
123+
kind: StatementKind::Assign(box (*lhs, rhs)),
124+
}
125+
}
82126
}
127+
128+
_ => unreachable!(),
83129
}
84-
}
85-
s
86-
});
130+
})
131+
.collect::<Vec<_>>();
132+
133+
let (from, first) = bbs.pick2_mut(bb_idx, first);
87134
from.statements.extend(new_stmts);
88135
from.terminator_mut().kind = first.terminator().kind.clone();
89136
}

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

+156
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
- // MIR for `bar` before MatchBranchSimplification
2+
+ // MIR for `bar` after MatchBranchSimplification
3+
4+
fn bar(_1: i32) -> (bool, bool, bool, bool) {
5+
debug i => _1; // in scope 0 at $DIR/matches_reduce_branches.rs:11:8: 11:9
6+
let mut _0: (bool, bool, bool, bool); // return place in scope 0 at $DIR/matches_reduce_branches.rs:11:19: 11:43
7+
let _2: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:12:9: 12:10
8+
let _6: (); // in scope 0 at $DIR/matches_reduce_branches.rs:17:5: 32:6
9+
let mut _7: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:34:6: 34:7
10+
let mut _8: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:34:9: 34:10
11+
let mut _9: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:34:12: 34:13
12+
let mut _10: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:34:15: 34:16
13+
scope 1 {
14+
debug a => _2; // in scope 1 at $DIR/matches_reduce_branches.rs:12:9: 12:10
15+
let _3: bool; // in scope 1 at $DIR/matches_reduce_branches.rs:13:9: 13:10
16+
scope 2 {
17+
debug b => _3; // in scope 2 at $DIR/matches_reduce_branches.rs:13:9: 13:10
18+
let _4: bool; // in scope 2 at $DIR/matches_reduce_branches.rs:14:9: 14:10
19+
scope 3 {
20+
debug c => _4; // in scope 3 at $DIR/matches_reduce_branches.rs:14:9: 14:10
21+
let _5: bool; // in scope 3 at $DIR/matches_reduce_branches.rs:15:9: 15:10
22+
scope 4 {
23+
debug d => _5; // in scope 4 at $DIR/matches_reduce_branches.rs:15:9: 15:10
24+
}
25+
}
26+
}
27+
}
28+
29+
bb0: {
30+
StorageLive(_2); // scope 0 at $DIR/matches_reduce_branches.rs:12:9: 12:10
31+
StorageLive(_3); // scope 1 at $DIR/matches_reduce_branches.rs:13:9: 13:10
32+
StorageLive(_4); // scope 2 at $DIR/matches_reduce_branches.rs:14:9: 14:10
33+
StorageLive(_5); // scope 3 at $DIR/matches_reduce_branches.rs:15:9: 15:10
34+
StorageLive(_6); // scope 4 at $DIR/matches_reduce_branches.rs:17:5: 32:6
35+
- switchInt(_1) -> [7_i32: bb2, otherwise: bb1]; // scope 4 at $DIR/matches_reduce_branches.rs:18:9: 18:10
36+
+ _2 = Ne(_1, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:19:13: 19:22
37+
+ // ty::Const
38+
+ // + ty: i32
39+
+ // + val: Value(Scalar(0x00000007))
40+
+ // mir::Constant
41+
+ // + span: $DIR/matches_reduce_branches.rs:1:1: 1:1
42+
+ // + literal: Const { ty: i32, val: Value(Scalar(0x00000007)) }
43+
+ _3 = Eq(_1, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:20:13: 20:21
44+
+ // ty::Const
45+
+ // + ty: i32
46+
+ // + val: Value(Scalar(0x00000007))
47+
+ // mir::Constant
48+
+ // + span: $DIR/matches_reduce_branches.rs:1:1: 1:1
49+
+ // + literal: Const { ty: i32, val: Value(Scalar(0x00000007)) }
50+
+ _4 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:21:13: 21:22
51+
+ // ty::Const
52+
+ // + ty: bool
53+
+ // + val: Value(Scalar(0x00))
54+
+ // mir::Constant
55+
+ // + span: $DIR/matches_reduce_branches.rs:21:17: 21:22
56+
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
57+
+ _5 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:22:13: 22:21
58+
+ // ty::Const
59+
+ // + ty: bool
60+
+ // + val: Value(Scalar(0x01))
61+
+ // mir::Constant
62+
+ // + span: $DIR/matches_reduce_branches.rs:22:17: 22:21
63+
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
64+
+ goto -> bb3; // scope 4 at $DIR/matches_reduce_branches.rs:18:9: 18:10
65+
}
66+
67+
bb1: {
68+
_2 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:26:13: 26:21
69+
// ty::Const
70+
// + ty: bool
71+
// + val: Value(Scalar(0x01))
72+
// mir::Constant
73+
// + span: $DIR/matches_reduce_branches.rs:26:17: 26:21
74+
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
75+
_3 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:27:13: 27:22
76+
// ty::Const
77+
// + ty: bool
78+
// + val: Value(Scalar(0x00))
79+
// mir::Constant
80+
// + span: $DIR/matches_reduce_branches.rs:27:17: 27:22
81+
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
82+
_4 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:28:13: 28:22
83+
// ty::Const
84+
// + ty: bool
85+
// + val: Value(Scalar(0x00))
86+
// mir::Constant
87+
// + span: $DIR/matches_reduce_branches.rs:28:17: 28:22
88+
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
89+
_5 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:29:13: 29:21
90+
// ty::Const
91+
// + ty: bool
92+
// + val: Value(Scalar(0x01))
93+
// mir::Constant
94+
// + span: $DIR/matches_reduce_branches.rs:29:17: 29:21
95+
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
96+
goto -> bb3; // scope 4 at $DIR/matches_reduce_branches.rs:17:5: 32:6
97+
}
98+
99+
bb2: {
100+
_2 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:19:13: 19:22
101+
// ty::Const
102+
// + ty: bool
103+
// + val: Value(Scalar(0x00))
104+
// mir::Constant
105+
// + span: $DIR/matches_reduce_branches.rs:19:17: 19:22
106+
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
107+
_3 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:20:13: 20:21
108+
// ty::Const
109+
// + ty: bool
110+
// + val: Value(Scalar(0x01))
111+
// mir::Constant
112+
// + span: $DIR/matches_reduce_branches.rs:20:17: 20:21
113+
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
114+
_4 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:21:13: 21:22
115+
// ty::Const
116+
// + ty: bool
117+
// + val: Value(Scalar(0x00))
118+
// mir::Constant
119+
// + span: $DIR/matches_reduce_branches.rs:21:17: 21:22
120+
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
121+
_5 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:22:13: 22:21
122+
// ty::Const
123+
// + ty: bool
124+
// + val: Value(Scalar(0x01))
125+
// mir::Constant
126+
// + span: $DIR/matches_reduce_branches.rs:22:17: 22:21
127+
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
128+
goto -> bb3; // scope 4 at $DIR/matches_reduce_branches.rs:17:5: 32:6
129+
}
130+
131+
bb3: {
132+
StorageDead(_6); // scope 4 at $DIR/matches_reduce_branches.rs:32:6: 32:7
133+
StorageLive(_7); // scope 4 at $DIR/matches_reduce_branches.rs:34:6: 34:7
134+
_7 = _2; // scope 4 at $DIR/matches_reduce_branches.rs:34:6: 34:7
135+
StorageLive(_8); // scope 4 at $DIR/matches_reduce_branches.rs:34:9: 34:10
136+
_8 = _3; // scope 4 at $DIR/matches_reduce_branches.rs:34:9: 34:10
137+
StorageLive(_9); // scope 4 at $DIR/matches_reduce_branches.rs:34:12: 34:13
138+
_9 = _4; // scope 4 at $DIR/matches_reduce_branches.rs:34:12: 34:13
139+
StorageLive(_10); // scope 4 at $DIR/matches_reduce_branches.rs:34:15: 34:16
140+
_10 = _5; // scope 4 at $DIR/matches_reduce_branches.rs:34:15: 34:16
141+
(_0.0: bool) = move _7; // scope 4 at $DIR/matches_reduce_branches.rs:34:5: 34:17
142+
(_0.1: bool) = move _8; // scope 4 at $DIR/matches_reduce_branches.rs:34:5: 34:17
143+
(_0.2: bool) = move _9; // scope 4 at $DIR/matches_reduce_branches.rs:34:5: 34:17
144+
(_0.3: bool) = move _10; // scope 4 at $DIR/matches_reduce_branches.rs:34:5: 34:17
145+
StorageDead(_10); // scope 4 at $DIR/matches_reduce_branches.rs:34:16: 34:17
146+
StorageDead(_9); // scope 4 at $DIR/matches_reduce_branches.rs:34:16: 34:17
147+
StorageDead(_8); // scope 4 at $DIR/matches_reduce_branches.rs:34:16: 34:17
148+
StorageDead(_7); // scope 4 at $DIR/matches_reduce_branches.rs:34:16: 34:17
149+
StorageDead(_5); // scope 3 at $DIR/matches_reduce_branches.rs:35:1: 35:2
150+
StorageDead(_4); // scope 2 at $DIR/matches_reduce_branches.rs:35:1: 35:2
151+
StorageDead(_3); // scope 1 at $DIR/matches_reduce_branches.rs:35:1: 35:2
152+
StorageDead(_2); // scope 0 at $DIR/matches_reduce_branches.rs:35:1: 35:2
153+
return; // scope 0 at $DIR/matches_reduce_branches.rs:35:2: 35:2
154+
}
155+
}
156+

0 commit comments

Comments
 (0)