Skip to content

Commit

Permalink
[mir-opt] Disable the ConsideredEqual logic in SimplifyBranchSame opt
Browse files Browse the repository at this point in the history
The logic is currently broken and we need to disable it to fix a beta
regression (see #76803)
  • Loading branch information
wesleywiser committed Sep 17, 2020
1 parent 95386b6 commit dbd7226
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 17 deletions.
11 changes: 9 additions & 2 deletions compiler/rustc_mir/src/transform/simplify_try.rs
Expand Up @@ -612,7 +612,7 @@ impl<'a, 'tcx> SimplifyBranchSameOptimizationFinder<'a, 'tcx> {
&& bb_l.terminator().kind == bb_r.terminator().kind;
let statement_check = || {
bb_l.statements.iter().zip(&bb_r.statements).try_fold(StatementEquality::TrivialEqual, |acc,(l,r)| {
let stmt_equality = self.statement_equality(*adt_matched_on, &l, bb_l_idx, &r, bb_r_idx);
let stmt_equality = self.statement_equality(*adt_matched_on, &l, bb_l_idx, &r, bb_r_idx, self.tcx.sess.opts.debugging_opts.mir_opt_level);
if matches!(stmt_equality, StatementEquality::NotEqual) {
// short circuit
None
Expand Down Expand Up @@ -672,6 +672,7 @@ impl<'a, 'tcx> SimplifyBranchSameOptimizationFinder<'a, 'tcx> {
x_bb_idx: BasicBlock,
y: &Statement<'tcx>,
y_bb_idx: BasicBlock,
mir_opt_level: usize,
) -> StatementEquality {
let helper = |rhs: &Rvalue<'tcx>,
place: &Place<'tcx>,
Expand All @@ -690,7 +691,13 @@ impl<'a, 'tcx> SimplifyBranchSameOptimizationFinder<'a, 'tcx> {

match rhs {
Rvalue::Use(operand) if operand.place() == Some(adt_matched_on) => {
StatementEquality::ConsideredEqual(side_to_choose)
// FIXME(76803): This logic is currently broken because it does not take into
// account the current discriminant value.
if mir_opt_level > 2 {
StatementEquality::ConsideredEqual(side_to_choose)
} else {
StatementEquality::NotEqual
}
}
_ => {
trace!(
Expand Down
27 changes: 12 additions & 15 deletions src/test/mir-opt/simplify_arm.id.SimplifyBranchSame.diff
Expand Up @@ -13,27 +13,24 @@

bb0: {
_2 = discriminant(_1); // scope 0 at $DIR/simplify-arm.rs:11:9: 11:16
- switchInt(move _2) -> [0_isize: bb1, 1_isize: bb3, otherwise: bb2]; // scope 0 at $DIR/simplify-arm.rs:11:9: 11:16
+ goto -> bb1; // scope 0 at $DIR/simplify-arm.rs:11:9: 11:16
switchInt(move _2) -> [0_isize: bb1, 1_isize: bb3, otherwise: bb2]; // scope 0 at $DIR/simplify-arm.rs:11:9: 11:16
}

bb1: {
- discriminant(_0) = 0; // scope 0 at $DIR/simplify-arm.rs:12:17: 12:21
- goto -> bb4; // scope 0 at $DIR/simplify-arm.rs:10:5: 13:6
- }
-
- bb2: {
- unreachable; // scope 0 at $DIR/simplify-arm.rs:10:11: 10:12
- }
-
- bb3: {
discriminant(_0) = 0; // scope 0 at $DIR/simplify-arm.rs:12:17: 12:21
goto -> bb4; // scope 0 at $DIR/simplify-arm.rs:10:5: 13:6
}
bb2: {
unreachable; // scope 0 at $DIR/simplify-arm.rs:10:11: 10:12
}
bb3: {
_0 = move _1; // scope 1 at $DIR/simplify-arm.rs:11:20: 11:27
- goto -> bb4; // scope 0 at $DIR/simplify-arm.rs:10:5: 13:6
+ goto -> bb2; // scope 0 at $DIR/simplify-arm.rs:10:5: 13:6
goto -> bb4; // scope 0 at $DIR/simplify-arm.rs:10:5: 13:6
}

- bb4: {
+ bb2: {
bb4: {
return; // scope 0 at $DIR/simplify-arm.rs:14:2: 14:2
}
}
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/mir/issue-76803-branches-not-same.rs
@@ -0,0 +1,19 @@
// run-pass

#[derive(Debug, Eq, PartialEq)]
pub enum Type {
A,
B,
}


pub fn encode(v: Type) -> Type {
match v {
Type::A => Type::B,
_ => v,
}
}

fn main() {
assert_eq!(Type::B, encode(Type::A));
}

0 comments on commit dbd7226

Please sign in to comment.