Skip to content

Commit b9db927

Browse files
borsbors
committed
Auto merge of #75537 - tmiasko:match-branch-simplify, r=oli-obk
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. Additionally: * Copy discriminant instead of moving it since it might be necessary to use its value more than once. * Optimize when switching on copy operand Based on #75508. r? @oli-obk / @JulianKnodt
2 parents 80fb3f3 + af9b9e4 commit b9db927

12 files changed

+649
-63
lines changed

‎src/librustc_index/vec.rs

+11
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,17 @@ impl<I: Idx, T> IndexVec<I, T> {
680680
}
681681
}
682682

683+
/// Returns mutable references to three distinct elements or panics otherwise.
684+
#[inline]
685+
pub fn pick3_mut(&mut self, a: I, b: I, c: I) -> (&mut T, &mut T, &mut T) {
686+
let (ai, bi, ci) = (a.index(), b.index(), c.index());
687+
assert!(ai != bi && bi != ci && ci != ai);
688+
let len = self.raw.len();
689+
assert!(ai < len && bi < len && ci < len);
690+
let ptr = self.raw.as_mut_ptr();
691+
unsafe { (&mut *ptr.add(ai), &mut *ptr.add(bi), &mut *ptr.add(ci)) }
692+
}
693+
683694
pub fn convert_index_type<Ix: Idx>(self) -> IndexVec<Ix, T> {
684695
IndexVec { raw: self.raw, _marker: PhantomData }
685696
}

‎src/librustc_mir/transform/match_branches.rs

+79-37
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>) {
@@ -16,12 +43,12 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
1643
'outer: for bb_idx in bbs.indices() {
1744
let (discr, val, switch_ty, first, second) = match bbs[bb_idx].terminator().kind {
1845
TerminatorKind::SwitchInt {
19-
discr: Operand::Move(ref place),
46+
discr: Operand::Copy(ref place) | Operand::Move(ref place),
2047
switch_ty,
2148
ref targets,
2249
ref values,
2350
..
24-
} if targets.len() == 2 && values.len() == 1 => {
51+
} if targets.len() == 2 && values.len() == 1 && targets[0] != targets[1] => {
2552
(place, values[0], switch_ty, targets[0], targets[1])
2653
}
2754
// Only optimize switch int statements
@@ -42,49 +69,64 @@ 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 => {
52-
if let Some(f_c) = f_c.literal.try_eval_bool(tcx, param_env) {
53-
// This should also be a bool because it's writing to the same place
54-
let s_c = s_c.literal.try_eval_bool(tcx, param_env).unwrap();
55-
if f_c != s_c {
56-
// have to check this here because f_c & s_c might have
57-
// different spans.
58-
continue;
59-
}
60-
}
61-
continue 'outer;
62-
}
63-
// 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.
6486
_ => continue 'outer,
6587
}
6688
}
67-
// Take owenership of items now that we know we can optimize.
89+
// Take ownership of items now that we know we can optimize.
6890
let discr = discr.clone();
69-
let (from, first) = bbs.pick2_mut(bb_idx, first);
7091

71-
let new_stmts = first.statements.iter().cloned().map(|mut s| {
72-
if let StatementKind::Assign(box (_, ref mut rhs)) = s.kind {
73-
if let Rvalue::Use(Operand::Constant(c)) = rhs {
74-
let size = tcx.layout_of(param_env.and(switch_ty)).unwrap().size;
75-
let const_cmp = Operand::const_from_scalar(
76-
tcx,
77-
switch_ty,
78-
crate::interpret::Scalar::from_uint(val, size),
79-
rustc_span::DUMMY_SP,
80-
);
81-
if let Some(c) = c.literal.try_eval_bool(tcx, param_env) {
82-
let op = if c { BinOp::Eq } else { BinOp::Ne };
83-
*rhs = Rvalue::BinaryOp(op, Operand::Move(discr), const_cmp);
92+
// We already checked that first and second are different blocks,
93+
// and bb_idx has a different terminator from both of them.
94+
let (from, first, second) = bbs.pick3_mut(bb_idx, first, second);
95+
96+
let new_stmts = first.statements.iter().zip(second.statements.iter()).map(|(f, s)| {
97+
match (&f.kind, &s.kind) {
98+
(f_s, s_s) if f_s == s_s => (*f).clone(),
99+
100+
(
101+
StatementKind::Assign(box (lhs, Rvalue::Use(Operand::Constant(f_c)))),
102+
StatementKind::Assign(box (_, Rvalue::Use(Operand::Constant(s_c)))),
103+
) => {
104+
// From earlier loop we know that we are dealing with bool constants only:
105+
let f_b = f_c.literal.try_eval_bool(tcx, param_env).unwrap();
106+
let s_b = s_c.literal.try_eval_bool(tcx, param_env).unwrap();
107+
if f_b == s_b {
108+
// Same value in both blocks. Use statement as is.
109+
(*f).clone()
110+
} else {
111+
// Different value between blocks. Make value conditional on switch condition.
112+
let size = tcx.layout_of(param_env.and(switch_ty)).unwrap().size;
113+
let const_cmp = Operand::const_from_scalar(
114+
tcx,
115+
switch_ty,
116+
crate::interpret::Scalar::from_uint(val, size),
117+
rustc_span::DUMMY_SP,
118+
);
119+
let op = if f_b { BinOp::Eq } else { BinOp::Ne };
120+
let rhs = 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+
}
84125
}
85126
}
127+
128+
_ => unreachable!(),
86129
}
87-
s
88130
});
89131
from.statements.extend(new_stmts);
90132
from.terminator_mut().kind = first.terminator().kind.clone();

‎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)