Auto merge of #150925 - dianqk:if-cmp, r=saethlin

bors · bors · commit 18ae99075575 · 2026-01-15T23:54:21.000Z
Only use SSA locals in SimplifyComparisonIntegral Fixes #150904. The place may be modified from the comparison statement to the switchInt terminator. Best reviewed commit by commit.
diff --git a/compiler/rustc_middle/src/mir/statement.rs b/compiler/rustc_middle/src/mir/statement.rs
@@ -374,6 +374,12 @@ impl<'tcx> Place<'tcx> {
         self.projection.iter().any(|elem| elem.is_indirect())
     }
 
+    /// Returns `true` if the `Place` always refers to the same memory region
+    /// whatever the state of the program.
+    pub fn is_stable_offset(&self) -> bool {
+        self.projection.iter().all(|elem| elem.is_stable_offset())
+    }
+
     /// Returns `true` if this `Place`'s first projection is `Deref`.
     ///
     /// This is useful because for MIR phases `AnalysisPhase::PostCleanup` and later,
diff --git a/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs b/compiler/rustc_mir_transform/src/simplify_comparison_integral.rs
@@ -9,6 +9,8 @@ use rustc_middle::mir::{
 use rustc_middle::ty::{Ty, TyCtxt};
 use tracing::trace;
 
+use crate::ssa::SsaLocals;
+
 /// Pass to convert `if` conditions on integrals into switches on the integral.
 /// For an example, it turns something like
 ///
@@ -27,17 +29,18 @@ pub(super) struct SimplifyComparisonIntegral;
 
 impl<'tcx> crate::MirPass<'tcx> for SimplifyComparisonIntegral {
     fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
-        sess.mir_opt_level() > 0
+        sess.mir_opt_level() > 1
     }
 
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
         trace!("Running SimplifyComparisonIntegral on {:?}", body.source);
 
+        let typing_env = body.typing_env(tcx);
+        let ssa = SsaLocals::new(tcx, body, typing_env);
         let helper = OptimizationFinder { body };
-        let opts = helper.find_optimizations();
+        let opts = helper.find_optimizations(&ssa);
         let mut storage_deads_to_insert = vec![];
         let mut storage_deads_to_remove: Vec<(usize, BasicBlock)> = vec![];
-        let typing_env = body.typing_env(tcx);
         for opt in opts {
             trace!("SUCCESS: Applying {:?}", opt);
             // replace terminator with a switchInt that switches on the integer directly
@@ -132,7 +135,7 @@ impl<'tcx> crate::MirPass<'tcx> for SimplifyComparisonIntegral {
 
             let terminator = bb.terminator_mut();
             terminator.kind =
-                TerminatorKind::SwitchInt { discr: Operand::Move(opt.to_switch_on), targets };
+                TerminatorKind::SwitchInt { discr: Operand::Copy(opt.to_switch_on), targets };
         }
 
         for (idx, bb_idx) in storage_deads_to_remove {
@@ -154,19 +157,18 @@ struct OptimizationFinder<'a, 'tcx> {
 }
 
 impl<'tcx> OptimizationFinder<'_, 'tcx> {
-    fn find_optimizations(&self) -> Vec<OptimizationInfo<'tcx>> {
+    fn find_optimizations(&self, ssa: &SsaLocals) -> Vec<OptimizationInfo<'tcx>> {
         self.body
             .basic_blocks
             .iter_enumerated()
             .filter_map(|(bb_idx, bb)| {
                 // find switch
-                let (place_switched_on, targets, place_switched_on_moved) =
-                    match &bb.terminator().kind {
-                        rustc_middle::mir::TerminatorKind::SwitchInt { discr, targets, .. } => {
-                            Some((discr.place()?, targets, discr.is_move()))
-                        }
-                        _ => None,
-                    }?;
+                let (discr, targets) = bb.terminator().kind.as_switch()?;
+                let place_switched_on = discr.place()?;
+                // Make sure that the place is not modified.
+                if !ssa.is_ssa(place_switched_on.local) || !place_switched_on.is_stable_offset() {
+                    return None;
+                }
 
                 // find the statement that assigns the place being switched on
                 bb.statements.iter().enumerate().rev().find_map(|(stmt_idx, stmt)| {
@@ -180,12 +182,12 @@ impl<'tcx> OptimizationFinder<'_, 'tcx> {
                                     box (left, right),
                                 ) => {
                                     let (branch_value_scalar, branch_value_ty, to_switch_on) =
-                                        find_branch_value_info(left, right)?;
+                                        find_branch_value_info(left, right, ssa)?;
 
                                     Some(OptimizationInfo {
                                         bin_op_stmt_idx: stmt_idx,
                                         bb_idx,
-                                        can_remove_bin_op_stmt: place_switched_on_moved,
+                                        can_remove_bin_op_stmt: discr.is_move(),
                                         to_switch_on,
                                         branch_value_scalar,
                                         branch_value_ty,
@@ -207,13 +209,18 @@ impl<'tcx> OptimizationFinder<'_, 'tcx> {
 fn find_branch_value_info<'tcx>(
     left: &Operand<'tcx>,
     right: &Operand<'tcx>,
+    ssa: &SsaLocals,
 ) -> Option<(Scalar, Ty<'tcx>, Place<'tcx>)> {
     // check that either left or right is a constant.
     // if any are, we can use the other to switch on, and the constant as a value in a switch
     use Operand::*;
     match (left, right) {
         (Constant(branch_value), Copy(to_switch_on) | Move(to_switch_on))
         | (Copy(to_switch_on) | Move(to_switch_on), Constant(branch_value)) => {
+            // Make sure that the place is not modified.
+            if !ssa.is_ssa(to_switch_on.local) || !to_switch_on.is_stable_offset() {
+                return None;
+            }
             let branch_value_ty = branch_value.const_.ty();
             // we only want to apply this optimization if we are matching on integrals (and chars),
             // as it is not possible to switch on floats
diff --git a/tests/debuginfo/dummy_span.rs b/tests/debuginfo/dummy_span.rs
@@ -1,6 +1,8 @@
 //@ min-lldb-version: 310
 
 //@ compile-flags:-g
+// FIXME: Investigate why test fails without SimplifyComparisonIntegral pass.
+//@ compile-flags: -Zmir-enable-passes=+SimplifyComparisonIntegral
 //@ ignore-backends: gcc
 
 // === GDB TESTS ===================================================================================
diff --git a/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.32bit.panic-abort.diff b/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.32bit.panic-abort.diff
@@ -74,7 +74,7 @@
           _23 = copy (((*_1).0: std::fmt::FormattingOptions).0: u32);
           _22 = BitAnd(move _23, const core::fmt::flags::PRECISION_FLAG);
           StorageDead(_23);
-          switchInt(move _22) -> [0: bb10, otherwise: bb11];
+          switchInt(copy _22) -> [0: bb10, otherwise: bb11];
       }
   
       bb4: {
diff --git a/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.32bit.panic-unwind.diff b/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.32bit.panic-unwind.diff
@@ -74,7 +74,7 @@
           _23 = copy (((*_1).0: std::fmt::FormattingOptions).0: u32);
           _22 = BitAnd(move _23, const core::fmt::flags::PRECISION_FLAG);
           StorageDead(_23);
-          switchInt(move _22) -> [0: bb10, otherwise: bb11];
+          switchInt(copy _22) -> [0: bb10, otherwise: bb11];
       }
   
       bb4: {
diff --git a/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.64bit.panic-abort.diff b/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.64bit.panic-abort.diff
@@ -74,7 +74,7 @@
           _23 = copy (((*_1).0: std::fmt::FormattingOptions).0: u32);
           _22 = BitAnd(move _23, const core::fmt::flags::PRECISION_FLAG);
           StorageDead(_23);
-          switchInt(move _22) -> [0: bb10, otherwise: bb11];
+          switchInt(copy _22) -> [0: bb10, otherwise: bb11];
       }
   
       bb4: {
diff --git a/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.64bit.panic-unwind.diff b/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.64bit.panic-unwind.diff
@@ -74,7 +74,7 @@
           _23 = copy (((*_1).0: std::fmt::FormattingOptions).0: u32);
           _22 = BitAnd(move _23, const core::fmt::flags::PRECISION_FLAG);
           StorageDead(_23);
-          switchInt(move _22) -> [0: bb10, otherwise: bb11];
+          switchInt(copy _22) -> [0: bb10, otherwise: bb11];
       }
   
       bb4: {
diff --git a/tests/mir-opt/if_condition_int.dont_opt_bool.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.dont_opt_bool.SimplifyComparisonIntegral.diff
@@ -9,7 +9,7 @@
       bb0: {
           StorageLive(_2);
           _2 = copy _1;
-          switchInt(move _2) -> [0: bb2, otherwise: bb1];
+          switchInt(copy _1) -> [0: bb2, otherwise: bb1];
       }
   
       bb1: {
diff --git a/tests/mir-opt/if_condition_int.dont_opt_floats.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.dont_opt_floats.SimplifyComparisonIntegral.diff
@@ -11,7 +11,7 @@
           StorageLive(_2);
           StorageLive(_3);
           _3 = copy _1;
-          _2 = Eq(move _3, const -42f32);
+          _2 = Eq(copy _1, const -42f32);
           switchInt(move _2) -> [0: bb2, otherwise: bb1];
       }
   
diff --git a/tests/mir-opt/if_condition_int.dont_remove_comparison.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.dont_remove_comparison.SimplifyComparisonIntegral.diff
@@ -15,43 +15,39 @@
       }
   
       bb0: {
-          StorageLive(_2);
+          nop;
           StorageLive(_3);
           _3 = copy _1;
--         _2 = Eq(move _3, const 17_i8);
--         StorageDead(_3);
+          _2 = Eq(copy _1, const 17_i8);
+          StorageDead(_3);
 -         switchInt(copy _2) -> [0: bb2, otherwise: bb1];
-+         _2 = Eq(copy _3, const 17_i8);
-+         nop;
-+         switchInt(move _3) -> [17: bb1, otherwise: bb2];
++         switchInt(copy _1) -> [17: bb1, otherwise: bb2];
       }
   
       bb1: {
-+         StorageDead(_3);
           StorageLive(_6);
           StorageLive(_7);
           _7 = copy _2;
-          _6 = move _7 as i32 (IntToInt);
+          _6 = copy _2 as i32 (IntToInt);
           StorageDead(_7);
           _0 = Add(const 100_i32, move _6);
           StorageDead(_6);
           goto -> bb3;
       }
   
       bb2: {
-+         StorageDead(_3);
           StorageLive(_4);
           StorageLive(_5);
           _5 = copy _2;
-          _4 = move _5 as i32 (IntToInt);
+          _4 = copy _2 as i32 (IntToInt);
           StorageDead(_5);
           _0 = Add(const 10_i32, move _4);
           StorageDead(_4);
           goto -> bb3;
       }
   
       bb3: {
-          StorageDead(_2);
+          nop;
           return;
       }
   }
diff --git a/tests/mir-opt/if_condition_int.on_non_ssa_cmp.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.on_non_ssa_cmp.SimplifyComparisonIntegral.diff
@@ -0,0 +1,24 @@
+- // MIR for `on_non_ssa_cmp` before SimplifyComparisonIntegral
++ // MIR for `on_non_ssa_cmp` after SimplifyComparisonIntegral
+  
+  fn on_non_ssa_cmp(_1: u64) -> i32 {
+      let mut _0: i32;
+      let mut _2: bool;
+  
+      bb0: {
+          _2 = Eq(copy _1, const 42_u64);
+          _1 = const 43_u64;
+          switchInt(copy _2) -> [1: bb1, otherwise: bb2];
+      }
+  
+      bb1: {
+          _0 = const 0_i32;
+          return;
+      }
+  
+      bb2: {
+          _0 = const 1_i32;
+          return;
+      }
+  }
+  
diff --git a/tests/mir-opt/if_condition_int.on_non_ssa_place.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.on_non_ssa_place.SimplifyComparisonIntegral.diff
@@ -0,0 +1,24 @@
+- // MIR for `on_non_ssa_place` before SimplifyComparisonIntegral
++ // MIR for `on_non_ssa_place` after SimplifyComparisonIntegral
+  
+  fn on_non_ssa_place(_1: [u64; 10], _2: usize) -> i32 {
+      let mut _0: i32;
+      let mut _3: bool;
+  
+      bb0: {
+          _3 = Eq(copy _1[_2], const 42_u64);
+          _2 = const 10_usize;
+          switchInt(copy _3) -> [1: bb1, otherwise: bb2];
+      }
+  
+      bb1: {
+          _0 = const 0_i32;
+          return;
+      }
+  
+      bb2: {
+          _0 = const 1_i32;
+          return;
+      }
+  }
+  
diff --git a/tests/mir-opt/if_condition_int.on_non_ssa_switch.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.on_non_ssa_switch.SimplifyComparisonIntegral.diff
@@ -0,0 +1,24 @@
+- // MIR for `on_non_ssa_switch` before SimplifyComparisonIntegral
++ // MIR for `on_non_ssa_switch` after SimplifyComparisonIntegral
+  
+  fn on_non_ssa_switch(_1: u64) -> i32 {
+      let mut _0: i32;
+      let mut _2: bool;
+  
+      bb0: {
+          _2 = Eq(copy _1, const 42_u64);
+          _2 = const false;
+          switchInt(copy _2) -> [1: bb1, otherwise: bb2];
+      }
+  
+      bb1: {
+          _0 = const 0_i32;
+          return;
+      }
+  
+      bb2: {
+          _0 = const 1_i32;
+          return;
+      }
+  }
+  
diff --git a/tests/mir-opt/if_condition_int.opt_char.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.opt_char.SimplifyComparisonIntegral.diff
@@ -11,10 +11,10 @@
           StorageLive(_2);
           StorageLive(_3);
           _3 = copy _1;
--         _2 = Eq(move _3, const 'x');
+-         _2 = Eq(copy _1, const 'x');
 -         switchInt(move _2) -> [0: bb2, otherwise: bb1];
 +         nop;
-+         switchInt(move _3) -> [120: bb1, otherwise: bb2];
++         switchInt(copy _1) -> [120: bb1, otherwise: bb2];
       }
   
       bb1: {
diff --git a/tests/mir-opt/if_condition_int.opt_i8.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.opt_i8.SimplifyComparisonIntegral.diff
@@ -11,10 +11,10 @@
           StorageLive(_2);
           StorageLive(_3);
           _3 = copy _1;
--         _2 = Eq(move _3, const 42_i8);
+-         _2 = Eq(copy _1, const 42_i8);
 -         switchInt(move _2) -> [0: bb2, otherwise: bb1];
 +         nop;
-+         switchInt(move _3) -> [42: bb1, otherwise: bb2];
++         switchInt(copy _1) -> [42: bb1, otherwise: bb2];
       }
   
       bb1: {
diff --git a/tests/mir-opt/if_condition_int.opt_multiple_ifs.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.opt_multiple_ifs.SimplifyComparisonIntegral.diff
@@ -13,10 +13,10 @@
           StorageLive(_2);
           StorageLive(_3);
           _3 = copy _1;
--         _2 = Eq(move _3, const 42_u32);
+-         _2 = Eq(copy _1, const 42_u32);
 -         switchInt(move _2) -> [0: bb2, otherwise: bb1];
 +         nop;
-+         switchInt(move _3) -> [42: bb1, otherwise: bb2];
++         switchInt(copy _1) -> [42: bb1, otherwise: bb2];
       }
   
       bb1: {
@@ -30,10 +30,10 @@
           StorageLive(_4);
           StorageLive(_5);
           _5 = copy _1;
--         _4 = Ne(move _5, const 21_u32);
+-         _4 = Ne(copy _1, const 21_u32);
 -         switchInt(move _4) -> [0: bb4, otherwise: bb3];
 +         nop;
-+         switchInt(move _5) -> [21: bb4, otherwise: bb3];
++         switchInt(copy _1) -> [21: bb4, otherwise: bb3];
       }
   
       bb3: {
diff --git a/tests/mir-opt/if_condition_int.opt_negative.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.opt_negative.SimplifyComparisonIntegral.diff
@@ -11,10 +11,10 @@
           StorageLive(_2);
           StorageLive(_3);
           _3 = copy _1;
--         _2 = Eq(move _3, const -42_i32);
+-         _2 = Eq(copy _1, const -42_i32);
 -         switchInt(move _2) -> [0: bb2, otherwise: bb1];
 +         nop;
-+         switchInt(move _3) -> [4294967254: bb1, otherwise: bb2];
++         switchInt(copy _1) -> [4294967254: bb1, otherwise: bb2];
       }
   
       bb1: {
diff --git a/tests/mir-opt/if_condition_int.opt_u32.SimplifyComparisonIntegral.diff b/tests/mir-opt/if_condition_int.opt_u32.SimplifyComparisonIntegral.diff
@@ -11,10 +11,10 @@
           StorageLive(_2);
           StorageLive(_3);
           _3 = copy _1;
--         _2 = Eq(move _3, const 42_u32);
+-         _2 = Eq(copy _1, const 42_u32);
 -         switchInt(move _2) -> [0: bb2, otherwise: bb1];
 +         nop;
-+         switchInt(move _3) -> [42: bb1, otherwise: bb2];
++         switchInt(copy _1) -> [42: bb1, otherwise: bb2];
       }
   
       bb1: {
diff --git a/tests/mir-opt/if_condition_int.rs b/tests/mir-opt/if_condition_int.rs
diff --git a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.32bit.panic-abort.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.32bit.panic-abort.mir
diff --git a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.32bit.panic-unwind.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.32bit.panic-unwind.mir
diff --git a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.64bit.panic-abort.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.64bit.panic-abort.mir
diff --git a/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.64bit.panic-unwind.mir b/tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.64bit.panic-unwind.mir

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@`
`74`	`74`	`_23 = copy (((*_1).0: std::fmt::FormattingOptions).0: u32);`
`75`	`75`	`_22 = BitAnd(move _23, const core::fmt::flags::PRECISION_FLAG);`
`76`	`76`	`StorageDead(_23);`
`77`		`- switchInt(move _22) -> [0: bb10, otherwise: bb11];`
	`77`	`+ switchInt(copy _22) -> [0: bb10, otherwise: bb11];`
`78`	`78`	`}`
`79`	`79`
`80`	`80`	`bb4: {`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@`
`9`	`9`	`bb0: {`
`10`	`10`	`StorageLive(_2);`
`11`	`11`	`_2 = copy _1;`
`12`		`- switchInt(move _2) -> [0: bb2, otherwise: bb1];`
	`12`	`+ switchInt(copy _1) -> [0: bb2, otherwise: bb1];`
`13`	`13`	`}`
`14`	`14`
`15`	`15`	`bb1: {`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@`
`11`	`11`	`StorageLive(_2);`
`12`	`12`	`StorageLive(_3);`
`13`	`13`	`_3 = copy _1;`
`14`		`- _2 = Eq(move _3, const -42f32);`
	`14`	`+ _2 = Eq(copy _1, const -42f32);`
`15`	`15`	`switchInt(move _2) -> [0: bb2, otherwise: bb1];`
`16`	`16`	`}`
`17`	`17`