Skip to content

Commit

Permalink
Auto merge of rust-lang#75537 - tmiasko:match-branch-simplify, r=oli-obk
Browse files Browse the repository at this point in the history 
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 rust-lang#75508.

r? @oli-obk  / @JulianKnodt
  • Loading branch information
@bors
bors committed Aug 15, 2020
2 parents 80fb3f3 + af9b9e4 commit b9db927
Show file tree
Hide file tree
Showing 12 changed files with 649 additions and 63 deletions.
11 changes: 11 additions & 0 deletions src/librustc_index/vec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,17 @@ impl<I: Idx, T> IndexVec<I, T> {
}
}

/// Returns mutable references to three distinct elements or panics otherwise.
#[inline]
pub fn pick3_mut(&mut self, a: I, b: I, c: I) -> (&mut T, &mut T, &mut T) {
let (ai, bi, ci) = (a.index(), b.index(), c.index());
assert!(ai != bi && bi != ci && ci != ai);
let len = self.raw.len();
assert!(ai < len && bi < len && ci < len);
let ptr = self.raw.as_mut_ptr();
unsafe { (&mut *ptr.add(ai), &mut *ptr.add(bi), &mut *ptr.add(ci)) }
}

pub fn convert_index_type<Ix: Idx>(self) -> IndexVec<Ix, T> {
IndexVec { raw: self.raw, _marker: PhantomData }
}
Expand Down
116 changes: 79 additions & 37 deletions src/librustc_mir/transform/match_branches.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,37 @@ use rustc_middle::ty::TyCtxt;

pub struct MatchBranchSimplification;

// What's the intent of this pass?
// If one block is found that switches between blocks which both go to the same place
// AND both of these blocks set a similar const in their ->
// condense into 1 block based on discriminant AND goto the destination afterwards
/// If a source block is found that switches between two blocks that are exactly
/// the same modulo const bool assignments (e.g., one assigns true another false
/// to the same place), merge a target block statements into the source block,
/// using Eq / Ne comparison with switch value where const bools value differ.
///
/// For example:
///
/// ```rust
/// bb0: {
/// switchInt(move _3) -> [42_isize: bb1, otherwise: bb2];
/// }
///
/// bb1: {
/// _2 = const true;
/// goto -> bb3;
/// }
///
/// bb2: {
/// _2 = const false;
/// goto -> bb3;
/// }
/// ```
///
/// into:
///
/// ```rust
/// bb0: {
/// _2 = Eq(move _3, const 42_isize);
/// goto -> bb3;
/// }
/// ```
impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) {
Expand All @@ -16,12 +43,12 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
'outer: for bb_idx in bbs.indices() {
let (discr, val, switch_ty, first, second) = match bbs[bb_idx].terminator().kind {
TerminatorKind::SwitchInt {
discr: Operand::Move(ref place),
discr: Operand::Copy(ref place) | Operand::Move(ref place),
switch_ty,
ref targets,
ref values,
..
} if targets.len() == 2 && values.len() == 1 => {
} if targets.len() == 2 && values.len() == 1 && targets[0] != targets[1] => {
(place, values[0], switch_ty, targets[0], targets[1])
}
// Only optimize switch int statements
Expand All @@ -42,49 +69,64 @@ impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
}
for (f, s) in first_stmts.iter().zip(scnd_stmts.iter()) {
match (&f.kind, &s.kind) {
// If two statements are exactly the same just ignore them.
(f_s, s_s) if f_s == s_s => (),
// If two statements are exactly the same, we can optimize.
(f_s, s_s) if f_s == s_s => {}

// If two statements are const bool assignments to the same place, we can optimize.
(
StatementKind::Assign(box (lhs_f, Rvalue::Use(Operand::Constant(f_c)))),
StatementKind::Assign(box (lhs_s, Rvalue::Use(Operand::Constant(s_c)))),
) if lhs_f == lhs_s => {
if let Some(f_c) = f_c.literal.try_eval_bool(tcx, param_env) {
// This should also be a bool because it's writing to the same place
let s_c = s_c.literal.try_eval_bool(tcx, param_env).unwrap();
if f_c != s_c {
// have to check this here because f_c & s_c might have
// different spans.
continue;
}
}
continue 'outer;
}
// If there are not exclusively assignments, then ignore this
) if lhs_f == lhs_s
&& f_c.literal.ty.is_bool()
&& s_c.literal.ty.is_bool()
&& f_c.literal.try_eval_bool(tcx, param_env).is_some()
&& s_c.literal.try_eval_bool(tcx, param_env).is_some() => {}

// Otherwise we cannot optimize. Try another block.
_ => continue 'outer,
}
}
// Take owenership of items now that we know we can optimize.
// Take ownership of items now that we know we can optimize.
let discr = discr.clone();
let (from, first) = bbs.pick2_mut(bb_idx, first);

let new_stmts = first.statements.iter().cloned().map(|mut s| {
if let StatementKind::Assign(box (_, ref mut rhs)) = s.kind {
if let Rvalue::Use(Operand::Constant(c)) = rhs {
let size = tcx.layout_of(param_env.and(switch_ty)).unwrap().size;
let const_cmp = Operand::const_from_scalar(
tcx,
switch_ty,
crate::interpret::Scalar::from_uint(val, size),
rustc_span::DUMMY_SP,
);
if let Some(c) = c.literal.try_eval_bool(tcx, param_env) {
let op = if c { BinOp::Eq } else { BinOp::Ne };
*rhs = Rvalue::BinaryOp(op, Operand::Move(discr), const_cmp);
// We already checked that first and second are different blocks,
// and bb_idx has a different terminator from both of them.
let (from, first, second) = bbs.pick3_mut(bb_idx, first, second);

let new_stmts = first.statements.iter().zip(second.statements.iter()).map(|(f, s)| {
match (&f.kind, &s.kind) {
(f_s, s_s) if f_s == s_s => (*f).clone(),

(
StatementKind::Assign(box (lhs, Rvalue::Use(Operand::Constant(f_c)))),
StatementKind::Assign(box (_, Rvalue::Use(Operand::Constant(s_c)))),
) => {
// From earlier loop we know that we are dealing with bool constants only:
let f_b = f_c.literal.try_eval_bool(tcx, param_env).unwrap();
let s_b = s_c.literal.try_eval_bool(tcx, param_env).unwrap();
if f_b == s_b {
// Same value in both blocks. Use statement as is.
(*f).clone()
} else {
// Different value between blocks. Make value conditional on switch condition.
let size = tcx.layout_of(param_env.and(switch_ty)).unwrap().size;
let const_cmp = Operand::const_from_scalar(
tcx,
switch_ty,
crate::interpret::Scalar::from_uint(val, size),
rustc_span::DUMMY_SP,
);
let op = if f_b { BinOp::Eq } else { BinOp::Ne };
let rhs = Rvalue::BinaryOp(op, Operand::Copy(discr.clone()), const_cmp);
Statement {
source_info: f.source_info,
kind: StatementKind::Assign(box (*lhs, rhs)),
}
}
}

_ => unreachable!(),
}
s
});
from.statements.extend(new_stmts);
from.terminator_mut().kind = first.terminator().kind.clone();
Expand Down
156 changes: 156 additions & 0 deletions src/test/mir-opt/matches_reduce_branches.bar.MatchBranchSimplification.diff.32bit
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
- // MIR for `bar` before MatchBranchSimplification
+ // MIR for `bar` after MatchBranchSimplification

fn bar(_1: i32) -> (bool, bool, bool, bool) {
debug i => _1; // in scope 0 at $DIR/matches_reduce_branches.rs:11:8: 11:9
let mut _0: (bool, bool, bool, bool); // return place in scope 0 at $DIR/matches_reduce_branches.rs:11:19: 11:43
let _2: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:12:9: 12:10
let _6: (); // in scope 0 at $DIR/matches_reduce_branches.rs:17:5: 32:6
let mut _7: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:34:6: 34:7
let mut _8: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:34:9: 34:10
let mut _9: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:34:12: 34:13
let mut _10: bool; // in scope 0 at $DIR/matches_reduce_branches.rs:34:15: 34:16
scope 1 {
debug a => _2; // in scope 1 at $DIR/matches_reduce_branches.rs:12:9: 12:10
let _3: bool; // in scope 1 at $DIR/matches_reduce_branches.rs:13:9: 13:10
scope 2 {
debug b => _3; // in scope 2 at $DIR/matches_reduce_branches.rs:13:9: 13:10
let _4: bool; // in scope 2 at $DIR/matches_reduce_branches.rs:14:9: 14:10
scope 3 {
debug c => _4; // in scope 3 at $DIR/matches_reduce_branches.rs:14:9: 14:10
let _5: bool; // in scope 3 at $DIR/matches_reduce_branches.rs:15:9: 15:10
scope 4 {
debug d => _5; // in scope 4 at $DIR/matches_reduce_branches.rs:15:9: 15:10
}
}
}
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/matches_reduce_branches.rs:12:9: 12:10
StorageLive(_3); // scope 1 at $DIR/matches_reduce_branches.rs:13:9: 13:10
StorageLive(_4); // scope 2 at $DIR/matches_reduce_branches.rs:14:9: 14:10
StorageLive(_5); // scope 3 at $DIR/matches_reduce_branches.rs:15:9: 15:10
StorageLive(_6); // scope 4 at $DIR/matches_reduce_branches.rs:17:5: 32:6
- switchInt(_1) -> [7_i32: bb2, otherwise: bb1]; // scope 4 at $DIR/matches_reduce_branches.rs:18:9: 18:10
+ _2 = Ne(_1, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:19:13: 19:22
+ // ty::Const
+ // + ty: i32
+ // + val: Value(Scalar(0x00000007))
+ // mir::Constant
+ // + span: $DIR/matches_reduce_branches.rs:1:1: 1:1
+ // + literal: Const { ty: i32, val: Value(Scalar(0x00000007)) }
+ _3 = Eq(_1, const 7_i32); // scope 4 at $DIR/matches_reduce_branches.rs:20:13: 20:21
+ // ty::Const
+ // + ty: i32
+ // + val: Value(Scalar(0x00000007))
+ // mir::Constant
+ // + span: $DIR/matches_reduce_branches.rs:1:1: 1:1
+ // + literal: Const { ty: i32, val: Value(Scalar(0x00000007)) }
+ _4 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:21:13: 21:22
+ // ty::Const
+ // + ty: bool
+ // + val: Value(Scalar(0x00))
+ // mir::Constant
+ // + span: $DIR/matches_reduce_branches.rs:21:17: 21:22
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
+ _5 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:22:13: 22:21
+ // ty::Const
+ // + ty: bool
+ // + val: Value(Scalar(0x01))
+ // mir::Constant
+ // + span: $DIR/matches_reduce_branches.rs:22:17: 22:21
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
+ goto -> bb3; // scope 4 at $DIR/matches_reduce_branches.rs:18:9: 18:10
}

bb1: {
_2 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:26:13: 26:21
// ty::Const
// + ty: bool
// + val: Value(Scalar(0x01))
// mir::Constant
// + span: $DIR/matches_reduce_branches.rs:26:17: 26:21
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
_3 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:27:13: 27:22
// ty::Const
// + ty: bool
// + val: Value(Scalar(0x00))
// mir::Constant
// + span: $DIR/matches_reduce_branches.rs:27:17: 27:22
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
_4 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:28:13: 28:22
// ty::Const
// + ty: bool
// + val: Value(Scalar(0x00))
// mir::Constant
// + span: $DIR/matches_reduce_branches.rs:28:17: 28:22
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
_5 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:29:13: 29:21
// ty::Const
// + ty: bool
// + val: Value(Scalar(0x01))
// mir::Constant
// + span: $DIR/matches_reduce_branches.rs:29:17: 29:21
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
goto -> bb3; // scope 4 at $DIR/matches_reduce_branches.rs:17:5: 32:6
}

bb2: {
_2 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:19:13: 19:22
// ty::Const
// + ty: bool
// + val: Value(Scalar(0x00))
// mir::Constant
// + span: $DIR/matches_reduce_branches.rs:19:17: 19:22
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
_3 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:20:13: 20:21
// ty::Const
// + ty: bool
// + val: Value(Scalar(0x01))
// mir::Constant
// + span: $DIR/matches_reduce_branches.rs:20:17: 20:21
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
_4 = const false; // scope 4 at $DIR/matches_reduce_branches.rs:21:13: 21:22
// ty::Const
// + ty: bool
// + val: Value(Scalar(0x00))
// mir::Constant
// + span: $DIR/matches_reduce_branches.rs:21:17: 21:22
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
_5 = const true; // scope 4 at $DIR/matches_reduce_branches.rs:22:13: 22:21
// ty::Const
// + ty: bool
// + val: Value(Scalar(0x01))
// mir::Constant
// + span: $DIR/matches_reduce_branches.rs:22:17: 22:21
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
goto -> bb3; // scope 4 at $DIR/matches_reduce_branches.rs:17:5: 32:6
}

bb3: {
StorageDead(_6); // scope 4 at $DIR/matches_reduce_branches.rs:32:6: 32:7
StorageLive(_7); // scope 4 at $DIR/matches_reduce_branches.rs:34:6: 34:7
_7 = _2; // scope 4 at $DIR/matches_reduce_branches.rs:34:6: 34:7
StorageLive(_8); // scope 4 at $DIR/matches_reduce_branches.rs:34:9: 34:10
_8 = _3; // scope 4 at $DIR/matches_reduce_branches.rs:34:9: 34:10
StorageLive(_9); // scope 4 at $DIR/matches_reduce_branches.rs:34:12: 34:13
_9 = _4; // scope 4 at $DIR/matches_reduce_branches.rs:34:12: 34:13
StorageLive(_10); // scope 4 at $DIR/matches_reduce_branches.rs:34:15: 34:16
_10 = _5; // scope 4 at $DIR/matches_reduce_branches.rs:34:15: 34:16
(_0.0: bool) = move _7; // scope 4 at $DIR/matches_reduce_branches.rs:34:5: 34:17
(_0.1: bool) = move _8; // scope 4 at $DIR/matches_reduce_branches.rs:34:5: 34:17
(_0.2: bool) = move _9; // scope 4 at $DIR/matches_reduce_branches.rs:34:5: 34:17
(_0.3: bool) = move _10; // scope 4 at $DIR/matches_reduce_branches.rs:34:5: 34:17
StorageDead(_10); // scope 4 at $DIR/matches_reduce_branches.rs:34:16: 34:17
StorageDead(_9); // scope 4 at $DIR/matches_reduce_branches.rs:34:16: 34:17
StorageDead(_8); // scope 4 at $DIR/matches_reduce_branches.rs:34:16: 34:17
StorageDead(_7); // scope 4 at $DIR/matches_reduce_branches.rs:34:16: 34:17
StorageDead(_5); // scope 3 at $DIR/matches_reduce_branches.rs:35:1: 35:2
StorageDead(_4); // scope 2 at $DIR/matches_reduce_branches.rs:35:1: 35:2
StorageDead(_3); // scope 1 at $DIR/matches_reduce_branches.rs:35:1: 35:2
StorageDead(_2); // scope 0 at $DIR/matches_reduce_branches.rs:35:1: 35:2
return; // scope 0 at $DIR/matches_reduce_branches.rs:35:2: 35:2
}
}

Loading

0 comments on commit b9db927

Please sign in to comment.