From dbd7226d299ac23647975068bea1168cf56e0feb Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 17 Sep 2020 09:29:39 -0400 Subject: [PATCH] [mir-opt] Disable the `ConsideredEqual` logic in SimplifyBranchSame opt The logic is currently broken and we need to disable it to fix a beta regression (see #76803) --- .../rustc_mir/src/transform/simplify_try.rs | 11 +++++-- .../simplify_arm.id.SimplifyBranchSame.diff | 29 +++++++++---------- .../ui/mir/issue-76803-branches-not-same.rs | 19 ++++++++++++ 3 files changed, 41 insertions(+), 18 deletions(-) create mode 100644 src/test/ui/mir/issue-76803-branches-not-same.rs diff --git a/compiler/rustc_mir/src/transform/simplify_try.rs b/compiler/rustc_mir/src/transform/simplify_try.rs index 76a60c45575..9a11d927240 100644 --- a/compiler/rustc_mir/src/transform/simplify_try.rs +++ b/compiler/rustc_mir/src/transform/simplify_try.rs @@ -612,7 +612,7 @@ fn find(&self) -> Vec { && 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 @@ -672,6 +672,7 @@ fn statement_equality( x_bb_idx: BasicBlock, y: &Statement<'tcx>, y_bb_idx: BasicBlock, + mir_opt_level: usize, ) -> StatementEquality { let helper = |rhs: &Rvalue<'tcx>, place: &Place<'tcx>, @@ -690,7 +691,13 @@ fn statement_equality( 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!( diff --git a/src/test/mir-opt/simplify_arm.id.SimplifyBranchSame.diff b/src/test/mir-opt/simplify_arm.id.SimplifyBranchSame.diff index 81a0e6ba0b4..06f359da2e7 100644 --- a/src/test/mir-opt/simplify_arm.id.SimplifyBranchSame.diff +++ b/src/test/mir-opt/simplify_arm.id.SimplifyBranchSame.diff @@ -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: { - _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 + 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 } -- bb4: { -+ bb2: { + 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 + } + + bb4: { return; // scope 0 at $DIR/simplify-arm.rs:14:2: 14:2 } } diff --git a/src/test/ui/mir/issue-76803-branches-not-same.rs b/src/test/ui/mir/issue-76803-branches-not-same.rs new file mode 100644 index 00000000000..a6a57622005 --- /dev/null +++ b/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)); +}