Auto merge of rust-lang#131244 - clubby789:match-branches-unreachable…

…, r=DianQK Consider empty-unreachable otherwise branches in MatchBranchSimplification Fixes rust-lang#131219
rust-lang-ci · Dec 28, 2024 · 4e0bc49 · 4e0bc49
2 parents 8b3f7ac + e32ec45
commit 4e0bc49
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 29 deletions.
diff --git a/compiler/rustc_data_structures/src/packed.rs b/compiler/rustc_data_structures/src/packed.rs
@@ -18,6 +18,13 @@ impl Pu128 {
     }
 }
 
+impl From<Pu128> for u128 {
+    #[inline]
+    fn from(value: Pu128) -> Self {
+        value.get()
+    }
+}
+
 impl From<u128> for Pu128 {
     #[inline]
     fn from(value: u128) -> Self {

diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs
@@ -67,6 +67,17 @@ impl SwitchTargets {
         &mut self.targets
     }
 
+    /// Returns a slice with all considered values (not including the fallback).
+    #[inline]
+    pub fn all_values(&self) -> &[Pu128] {
+        &self.values
+    }
+
+    #[inline]
+    pub fn all_values_mut(&mut self) -> &mut [Pu128] {
+        &mut self.values
+    }
+
     /// Finds the `BasicBlock` to which this `SwitchInt` will branch given the
     /// specific value. This cannot fail, as it'll return the `otherwise`
     /// branch if there's not a specific match for the value.

diff --git a/compiler/rustc_mir_transform/src/match_branches.rs b/compiler/rustc_mir_transform/src/match_branches.rs
@@ -7,6 +7,7 @@ use rustc_middle::mir::*;
 use rustc_middle::ty::layout::{IntegerExt, TyAndLayout};
 use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt};
 use rustc_type_ir::TyKind::*;
+use tracing::instrument;
 
 use super::simplify::simplify_cfg;
 
@@ -51,7 +52,7 @@ impl<'tcx> crate::MirPass<'tcx> for MatchBranchSimplification {
 }
 
 trait SimplifyMatch<'tcx> {
-    /// Simplifies a match statement, returning true if the simplification succeeds, false
+    /// Simplifies a match statement, returning `Some` if the simplification succeeds, `None`
     /// otherwise. Generic code is written here, and we generally don't need a custom
     /// implementation.
     fn simplify(
@@ -159,6 +160,7 @@ struct SimplifyToIf;
 /// }
 /// ```
 impl<'tcx> SimplifyMatch<'tcx> for SimplifyToIf {
+    #[instrument(level = "debug", skip(self, tcx), ret)]
     fn can_simplify(
         &mut self,
         tcx: TyCtxt<'tcx>,
@@ -167,12 +169,15 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToIf {
         bbs: &IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
         _discr_ty: Ty<'tcx>,
     ) -> Option<()> {
-        if targets.iter().len() != 1 {
-            return None;
-        }
+        let (first, second) = match targets.all_targets() {
+            &[first, otherwise] => (first, otherwise),
+            &[first, second, otherwise] if bbs[otherwise].is_empty_unreachable() => (first, second),
+            _ => {
+                return None;
+            }
+        };
+
         // We require that the possible target blocks all be distinct.
-        let (_, first) = targets.iter().next().unwrap();
-        let second = targets.otherwise();
         if first == second {
             return None;
         }
@@ -221,8 +226,14 @@ impl<'tcx> SimplifyMatch<'tcx> for SimplifyToIf {
         discr_local: Local,
         discr_ty: Ty<'tcx>,
     ) {
-        let (val, first) = targets.iter().next().unwrap();
-        let second = targets.otherwise();
+        let ((val, first), second) = match (targets.all_targets(), targets.all_values()) {
+            (&[first, otherwise], &[val]) => ((val, first), otherwise),
+            (&[first, second, otherwise], &[val, _]) if bbs[otherwise].is_empty_unreachable() => {
+                ((val, first), second)
+            }
+            _ => unreachable!(),
+        };
+
         // We already checked that first and second are different blocks,
         // and bb_idx has a different terminator from both of them.
         let first = &bbs[first];
@@ -297,7 +308,7 @@ struct SimplifyToExp {
     transform_kinds: Vec<TransformKind>,
 }
 
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Debug)]
 enum ExpectedTransformKind<'a, 'tcx> {
     /// Identical statements.
     Same(&'a StatementKind<'tcx>),
@@ -362,6 +373,7 @@ impl From<ExpectedTransformKind<'_, '_>> for TransformKind {
 /// }
 /// ```
 impl<'tcx> SimplifyMatch<'tcx> for SimplifyToExp {
+    #[instrument(level = "debug", skip(self, tcx), ret)]
     fn can_simplify(
         &mut self,
         tcx: TyCtxt<'tcx>,

diff --git a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-abort.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-abort.mir
@@ -25,20 +25,20 @@ fn num_to_digit(_1: char) -> u32 {
     bb1: {
         StorageLive(_3);
         _3 = discriminant(_2);
-        switchInt(move _3) -> [1: bb2, 0: bb6, otherwise: bb8];
+        StorageDead(_2);
+        switchInt(move _3) -> [1: bb2, otherwise: bb7];
     }
 
     bb2: {
         StorageDead(_3);
-        StorageDead(_2);
         StorageLive(_4);
         _4 = char::methods::<impl char>::to_digit(move _1, const 8_u32) -> [return: bb3, unwind unreachable];
     }
 
     bb3: {
         StorageLive(_5);
         _5 = discriminant(_4);
-        switchInt(move _5) -> [0: bb4, 1: bb5, otherwise: bb8];
+        switchInt(move _5) -> [0: bb4, 1: bb5, otherwise: bb6];
     }
 
     bb4: {
@@ -49,21 +49,20 @@ fn num_to_digit(_1: char) -> u32 {
         _0 = move ((_4 as Some).0: u32);
         StorageDead(_5);
         StorageDead(_4);
-        goto -> bb7;
+        goto -> bb8;
     }
 
     bb6: {
-        StorageDead(_3);
-        StorageDead(_2);
-        _0 = const 0_u32;
-        goto -> bb7;
+        unreachable;
     }
 
     bb7: {
-        return;
+        StorageDead(_3);
+        _0 = const 0_u32;
+        goto -> bb8;
     }
 
     bb8: {
-        unreachable;
+        return;
     }
 }
diff --git a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-unwind.mir
@@ -25,20 +25,20 @@ fn num_to_digit(_1: char) -> u32 {
     bb1: {
         StorageLive(_3);
         _3 = discriminant(_2);
-        switchInt(move _3) -> [1: bb2, 0: bb6, otherwise: bb8];
+        StorageDead(_2);
+        switchInt(move _3) -> [1: bb2, otherwise: bb7];
     }
 
     bb2: {
         StorageDead(_3);
-        StorageDead(_2);
         StorageLive(_4);
         _4 = char::methods::<impl char>::to_digit(move _1, const 8_u32) -> [return: bb3, unwind continue];
     }
 
     bb3: {
         StorageLive(_5);
         _5 = discriminant(_4);
-        switchInt(move _5) -> [0: bb4, 1: bb5, otherwise: bb8];
+        switchInt(move _5) -> [0: bb4, 1: bb5, otherwise: bb6];
     }
 
     bb4: {
@@ -49,21 +49,20 @@ fn num_to_digit(_1: char) -> u32 {
         _0 = move ((_4 as Some).0: u32);
         StorageDead(_5);
         StorageDead(_4);
-        goto -> bb7;
+        goto -> bb8;
     }
 
     bb6: {
-        StorageDead(_3);
-        StorageDead(_2);
-        _0 = const 0_u32;
-        goto -> bb7;
+        unreachable;
     }
 
     bb7: {
-        return;
+        StorageDead(_3);
+        _0 = const 0_u32;
+        goto -> bb8;
     }
 
     bb8: {
-        unreachable;
+        return;
     }
 }
diff --git a/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff b/tests/mir-opt/matches_reduce_branches.my_is_some.MatchBranchSimplification.diff
@@ -0,0 +1,37 @@
+- // MIR for `my_is_some` before MatchBranchSimplification
++ // MIR for `my_is_some` after MatchBranchSimplification
+
+  fn my_is_some(_1: Option<()>) -> bool {
+      debug bar => _1;
+      let mut _0: bool;
+      let mut _2: isize;
++     let mut _3: isize;
+
+      bb0: {
+          _2 = discriminant(_1);
+-         switchInt(move _2) -> [0: bb2, 1: bb3, otherwise: bb1];
+-     }
+- 
+-     bb1: {
+-         unreachable;
+-     }
+- 
+-     bb2: {
+-         _0 = const false;
+-         goto -> bb4;
+-     }
+- 
+-     bb3: {
+-         _0 = const true;
+-         goto -> bb4;
+-     }
+- 
+-     bb4: {
++         StorageLive(_3);
++         _3 = move _2;
++         _0 = Ne(copy _3, const 0_isize);
++         StorageDead(_3);
+          return;
+      }
+  }
+
diff --git a/tests/mir-opt/matches_reduce_branches.rs b/tests/mir-opt/matches_reduce_branches.rs
@@ -19,6 +19,18 @@ fn foo(bar: Option<()>) {
     }
 }
 
+// EMIT_MIR matches_reduce_branches.my_is_some.MatchBranchSimplification.diff
+// Test for #131219.
+fn my_is_some(bar: Option<()>) -> bool {
+    // CHECK-LABEL: fn my_is_some(
+    // CHECK: = Ne
+    // CHECK: return
+    match bar {
+        Some(_) => true,
+        None => false,
+    }
+}
+
 // EMIT_MIR matches_reduce_branches.bar.MatchBranchSimplification.diff
 fn bar(i: i32) -> (bool, bool, bool, bool) {
     // CHECK-LABEL: fn bar(
@@ -651,4 +663,6 @@ fn main() {
     let _: u8 = match_trunc_u16_u8_failed(EnumAu16::u0_0x0000);
 
     let _ = match_i128_u128(EnumAi128::A);
+
+    let _ = my_is_some(None);
 }