Remove overflow checks from ConstProp.

8 files changed, 88 insertions, 116 deletions

diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs
index 13f064aa72e..33ee90ffc11 100644
--- a/compiler/rustc_mir_transform/src/const_prop.rs
+++ b/compiler/rustc_mir_transform/src/const_prop.rs
@@ -15,7 +15,7 @@ use rustc_middle::mir::visit::{
 };
 use rustc_middle::mir::{
     BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind, Location,
-    Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp,
+    Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind,
     RETURN_PLACE,
 };
 use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout};
@@ -503,55 +503,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         }
     }
 
-    fn check_unary_op(&mut self, op: UnOp, arg: &Operand<'tcx>) -> Option<()> {
-        if self.use_ecx(|this| {
-            let val = this.ecx.read_immediate(&this.ecx.eval_operand(arg, None)?)?;
-            let (_res, overflow, _ty) = this.ecx.overflowing_unary_op(op, &val)?;
-            Ok(overflow)
-        })? {
-            // `AssertKind` only has an `OverflowNeg` variant, so make sure that is
-            // appropriate to use.
-            assert_eq!(op, UnOp::Neg, "Neg is the only UnOp that can overflow");
-            return None;
-        }
-
-        Some(())
-    }
-
-    fn check_binary_op(
-        &mut self,
-        op: BinOp,
-        left: &Operand<'tcx>,
-        right: &Operand<'tcx>,
-    ) -> Option<()> {
-        let r = self.use_ecx(|this| this.ecx.read_immediate(&this.ecx.eval_operand(right, None)?));
-        let l = self.use_ecx(|this| this.ecx.read_immediate(&this.ecx.eval_operand(left, None)?));
-        // Check for exceeding shifts *even if* we cannot evaluate the LHS.
-        if matches!(op, BinOp::Shr | BinOp::Shl) {
-            let r = r.clone()?;
-            // We need the type of the LHS. We cannot use `place_layout` as that is the type
-            // of the result, which for checked binops is not the same!
-            let left_ty = left.ty(self.local_decls, self.tcx);
-            let left_size = self.ecx.layout_of(left_ty).ok()?.size;
-            let right_size = r.layout.size;
-            let r_bits = r.to_scalar().to_bits(right_size).ok();
-            if r_bits.map_or(false, |b| b >= left_size.bits() as u128) {
-                return None;
-            }
-        }
-
-        if let (Some(l), Some(r)) = (&l, &r) {
-            // The remaining operators are handled through `overflowing_binary_op`.
-            if self.use_ecx(|this| {
-                let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
-                Ok(overflow)
-            })? {
-                return None;
-            }
-        }
-        Some(())
-    }
-
     fn propagate_operand(&mut self, operand: &mut Operand<'tcx>) {
         match *operand {
             Operand::Copy(l) | Operand::Move(l) => {
@@ -587,28 +538,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         //   2. Working around bugs in other parts of the compiler
         //        - In this case, we'll return `None` from this function to stop evaluation.
         match rvalue {
-            // Additional checking: give lints to the user if an overflow would occur.
-            // We do this here and not in the `Assert` terminator as that terminator is
-            // only sometimes emitted (overflow checks can be disabled), but we want to always
-            // lint.
-            Rvalue::UnaryOp(op, arg) => {
-                trace!("checking UnaryOp(op = {:?}, arg = {:?})", op, arg);
-                self.check_unary_op(*op, arg)?;
-            }
-            Rvalue::BinaryOp(op, box (left, right)) => {
-                trace!("checking BinaryOp(op = {:?}, left = {:?}, right = {:?})", op, left, right);
-                self.check_binary_op(*op, left, right)?;
-            }
-            Rvalue::CheckedBinaryOp(op, box (left, right)) => {
-                trace!(
-                    "checking CheckedBinaryOp(op = {:?}, left = {:?}, right = {:?})",
-                    op,
-                    left,
-                    right
-                );
-                self.check_binary_op(*op, left, right)?;
-            }
-
             // Do not try creating references (#67862)
             Rvalue::AddressOf(_, place) | Rvalue::Ref(_, _, place) => {
                 trace!("skipping AddressOf | Ref for {:?}", place);
@@ -638,7 +567,10 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             | Rvalue::Cast(..)
             | Rvalue::ShallowInitBox(..)
             | Rvalue::Discriminant(..)
-            | Rvalue::NullaryOp(..) => {}
+            | Rvalue::NullaryOp(..)
+            | Rvalue::UnaryOp(..)
+            | Rvalue::BinaryOp(..)
+            | Rvalue::CheckedBinaryOp(..) => {}
         }
 
         // FIXME we need to revisit this for #67176
@@ -1079,31 +1011,18 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
         // Do NOT early return in this function, it does some crucial fixup of the state at the end!
         match &mut terminator.kind {
             TerminatorKind::Assert { expected, ref mut cond, .. } => {
-                if let Some(ref value) = self.eval_operand(&cond) {
-                    trace!("assertion on {:?} should be {:?}", value, expected);
-                    let expected = Scalar::from_bool(*expected);
+                if let Some(ref value) = self.eval_operand(&cond)
                     // FIXME should be used use_ecx rather than a local match... but we have
                     // quite a few of these read_scalar/read_immediate that need fixing.
-                    if let Ok(value_const) = self.ecx.read_scalar(&value) {
-                        if expected != value_const {
-                            // Poison all places this operand references so that further code
-                            // doesn't use the invalid value
-                            match cond {
-                                Operand::Move(ref place) | Operand::Copy(ref place) => {
-                                    Self::remove_const(&mut self.ecx, place.local);
-                                }
-                                Operand::Constant(_) => {}
-                            }
-                        } else {
-                            if self.should_const_prop(value) {
-                                *cond = self.operand_from_scalar(
-                                    value_const,
-                                    self.tcx.types.bool,
-                                    source_info.span,
-                                );
-                            }
-                        }
-                    }
+                    && let Ok(value_const) = self.ecx.read_scalar(&value)
+                    && self.should_const_prop(value)
+                {
+                    trace!("assertion on {:?} should be {:?}", value, expected);
+                    *cond = self.operand_from_scalar(
+                        value_const,
+                        self.tcx.types.bool,
+                        source_info.span,
+                    );
                 }
             }
             TerminatorKind::SwitchInt { ref mut discr, .. } => {
diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs
index f3ca2337e59..19019e3ef74 100644
--- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs
+++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs
@@ -180,12 +180,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
                         let overflow = match overflow {
                             FlatSet::Top => FlatSet::Top,
                             FlatSet::Elem(overflow) => {
-                                if overflow {
-                                    // Overflow cannot be reliably propagated. See: https://github.com/rust-lang/rust/pull/101168#issuecomment-1288091446
-                                    FlatSet::Top
-                                } else {
-                                    self.wrap_scalar(Scalar::from_bool(false), self.tcx.types.bool)
-                                }
+                                self.wrap_scalar(Scalar::from_bool(overflow), self.tcx.types.bool)
                             }
                             FlatSet::Bottom => FlatSet::Bottom,
                         };
diff --git a/tests/mir-opt/const_prop/bad_op_div_by_zero.main.ConstProp.diff b/tests/mir-opt/const_prop/bad_op_div_by_zero.main.ConstProp.diff
index bea32a67ef4..900061a484b 100644
--- a/tests/mir-opt/const_prop/bad_op_div_by_zero.main.ConstProp.diff
+++ b/tests/mir-opt/const_prop/bad_op_div_by_zero.main.ConstProp.diff
@@ -24,9 +24,10 @@
           StorageLive(_3);                 // scope 1 at $DIR/bad_op_div_by_zero.rs:+2:18: +2:19
 -         _3 = _1;                         // scope 1 at $DIR/bad_op_div_by_zero.rs:+2:18: +2:19
 -         _4 = Eq(_3, const 0_i32);        // scope 1 at $DIR/bad_op_div_by_zero.rs:+2:14: +2:19
+-         assert(!move _4, "attempt to divide `{}` by zero", const 1_i32) -> bb1; // scope 1 at $DIR/bad_op_div_by_zero.rs:+2:14: +2:19
 +         _3 = const 0_i32;                // scope 1 at $DIR/bad_op_div_by_zero.rs:+2:18: +2:19
 +         _4 = const true;                 // scope 1 at $DIR/bad_op_div_by_zero.rs:+2:14: +2:19
-          assert(!move _4, "attempt to divide `{}` by zero", const 1_i32) -> bb1; // scope 1 at $DIR/bad_op_div_by_zero.rs:+2:14: +2:19
++         assert(!const true, "attempt to divide `{}` by zero", const 1_i32) -> bb1; // scope 1 at $DIR/bad_op_div_by_zero.rs:+2:14: +2:19
       }
   
       bb1: {
diff --git a/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.diff b/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.diff
new file mode 100644
index 00000000000..d03c23a3fb5
--- /dev/null
+++ b/tests/mir-opt/const_prop/inherit_overflow.main.ConstProp.diff
@@ -0,0 +1,39 @@
+- // MIR for `main` before ConstProp
++ // MIR for `main` after ConstProp
+  
+  fn main() -> () {
+      let mut _0: ();                      // return place in scope 0 at $DIR/inherit_overflow.rs:+0:11: +0:11
+      let mut _1: u8;                      // in scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+      let mut _2: u8;                      // in scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+      let mut _3: u8;                      // in scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+      scope 1 {
+      }
+      scope 2 (inlined <u8 as Add>::add) { // at $DIR/inherit_overflow.rs:8:13: 8:47
+          debug self => _2;                // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+          debug other => _3;               // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+          let mut _4: (u8, bool);          // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+      }
+  
+      bb0: {
+          StorageLive(_1);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          StorageLive(_2);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          _2 = const u8::MAX;              // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          StorageLive(_3);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          _3 = const 1_u8;                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+-         _4 = CheckedAdd(_2, _3);         // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+-         assert(!move (_4.1: bool), "attempt to compute `{} + {}`, which would overflow", _2, _3) -> bb1; // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
++         _4 = const (0_u8, true);         // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
++         assert(!const true, "attempt to compute `{} + {}`, which would overflow", _2, _3) -> bb1; // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+      }
+  
+      bb1: {
+-         _1 = move (_4.0: u8);            // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
++         _1 = const 0_u8;                 // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+          StorageDead(_3);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          StorageDead(_2);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          StorageDead(_1);                 // scope 0 at $DIR/inherit_overflow.rs:+3:47: +3:48
+          _0 = const ();                   // scope 0 at $DIR/inherit_overflow.rs:+0:11: +4:2
+          return;                          // scope 0 at $DIR/inherit_overflow.rs:+4:2: +4:2
+      }
+  }
+  
diff --git a/tests/mir-opt/const_prop/inherit_overflow.rs b/tests/mir-opt/const_prop/inherit_overflow.rs
new file mode 100644
index 00000000000..541a8c5c3af
--- /dev/null
+++ b/tests/mir-opt/const_prop/inherit_overflow.rs
@@ -0,0 +1,9 @@
+// unit-test: ConstProp
+// compile-flags: -Zmir-enable-passes=+Inline
+
+// EMIT_MIR inherit_overflow.main.ConstProp.diff
+fn main() {
+    // After inlining, this will contain a `CheckedBinaryOp`.
+    // Propagating the overflow is ok as codegen will just skip emitting the panic.
+    let _ = <u8 as std::ops::Add>::add(255, 1);
+}
diff --git a/tests/mir-opt/dataflow-const-prop/checked.main.DataflowConstProp.diff b/tests/mir-opt/dataflow-const-prop/checked.main.DataflowConstProp.diff
index a4ebd0c8c18..944afed8f46 100644
--- a/tests/mir-opt/dataflow-const-prop/checked.main.DataflowConstProp.diff
+++ b/tests/mir-opt/dataflow-const-prop/checked.main.DataflowConstProp.diff
@@ -61,7 +61,7 @@
 -         assert(!move (_10.1: bool), "attempt to compute `{} + {}`, which would overflow", move _9, const 1_i32) -> bb2; // scope 4 at $DIR/checked.rs:+6:13: +6:18
 +         _9 = const i32::MAX;             // scope 4 at $DIR/checked.rs:+6:13: +6:14
 +         _10 = CheckedAdd(const i32::MAX, const 1_i32); // scope 4 at $DIR/checked.rs:+6:13: +6:18
-+         assert(!move (_10.1: bool), "attempt to compute `{} + {}`, which would overflow", const i32::MAX, const 1_i32) -> bb2; // scope 4 at $DIR/checked.rs:+6:13: +6:18
++         assert(!const true, "attempt to compute `{} + {}`, which would overflow", const i32::MAX, const 1_i32) -> bb2; // scope 4 at $DIR/checked.rs:+6:13: +6:18
       }
   
       bb2: {
diff --git a/tests/mir-opt/dataflow-const-prop/inherit_overflow.main.DataflowConstProp.diff b/tests/mir-opt/dataflow-const-prop/inherit_overflow.main.DataflowConstProp.diff
index 33122f465fe..29781e9ce18 100644
--- a/tests/mir-opt/dataflow-const-prop/inherit_overflow.main.DataflowConstProp.diff
+++ b/tests/mir-opt/dataflow-const-prop/inherit_overflow.main.DataflowConstProp.diff
@@ -5,26 +5,34 @@
       let mut _0: ();                      // return place in scope 0 at $DIR/inherit_overflow.rs:+0:11: +0:11
       let mut _1: u8;                      // in scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
       let mut _2: u8;                      // in scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+      let mut _3: u8;                      // in scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
       scope 1 {
       }
-      scope 2 (inlined <u8 as Add>::add) { // at $DIR/inherit_overflow.rs:7:13: 7:47
-          debug self => _1;                // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
-          debug other => _2;               // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
-          let mut _3: (u8, bool);          // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+      scope 2 (inlined <u8 as Add>::add) { // at $DIR/inherit_overflow.rs:8:13: 8:47
+          debug self => _2;                // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+          debug other => _3;               // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+          let mut _4: (u8, bool);          // in scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
       }
   
       bb0: {
           StorageLive(_1);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
-          _1 = const u8::MAX;              // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
           StorageLive(_2);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
-          _2 = const 1_u8;                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
-          _3 = CheckedAdd(const u8::MAX, const 1_u8); // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
-          assert(!move (_3.1: bool), "attempt to compute `{} + {}`, which would overflow", const u8::MAX, const 1_u8) -> bb1; // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+          _2 = const u8::MAX;              // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          StorageLive(_3);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          _3 = const 1_u8;                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+-         _4 = CheckedAdd(_2, _3);         // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+-         assert(!move (_4.1: bool), "attempt to compute `{} + {}`, which would overflow", _2, _3) -> bb1; // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
++         _4 = CheckedAdd(const u8::MAX, const 1_u8); // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
++         assert(!const true, "attempt to compute `{} + {}`, which would overflow", const u8::MAX, const 1_u8) -> bb1; // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
       }
   
       bb1: {
+-         _1 = move (_4.0: u8);            // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
++         _1 = const 0_u8;                 // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
+          StorageDead(_3);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
           StorageDead(_2);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
-          StorageDead(_1);                 // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
+          StorageDead(_1);                 // scope 0 at $DIR/inherit_overflow.rs:+3:47: +3:48
+          _0 = const ();                   // scope 0 at $DIR/inherit_overflow.rs:+0:11: +4:2
           return;                          // scope 0 at $DIR/inherit_overflow.rs:+4:2: +4:2
       }
   }
diff --git a/tests/mir-opt/dataflow-const-prop/inherit_overflow.rs b/tests/mir-opt/dataflow-const-prop/inherit_overflow.rs
index 2f2d9d0102d..f4aba60f0c8 100644
--- a/tests/mir-opt/dataflow-const-prop/inherit_overflow.rs
+++ b/tests/mir-opt/dataflow-const-prop/inherit_overflow.rs
@@ -1,8 +1,9 @@
-// compile-flags: -Zunsound-mir-opts
+// unit-test: DataflowConstProp
+// compile-flags: -Zmir-enable-passes=+Inline
 
 // EMIT_MIR inherit_overflow.main.DataflowConstProp.diff
 fn main() {
-    // After inlining, this will contain a `CheckedBinaryOp`. The overflow
-    // must be ignored by the constant propagation to avoid triggering a panic.
+    // After inlining, this will contain a `CheckedBinaryOp`.
+    // Propagating the overflow is ok as codegen will just skip emitting the panic.
     let _ = <u8 as std::ops::Add>::add(255, 1);
 }