Rollup merge of #74491 - xldenis:constant-binop-opt, r=oli-obk

Optimize away BitAnd and BitOr when possible

This PR lets `const_prop` optimize away `a | true == true` , `a & false == false` and `a * 0 = 0`. While I was writing this I've realized that constant propagation misses a lot of opportunities. For example:  https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2a4b45e772f214210a36749b27223bb0

Constant propagation doesn't seem to... propagate constants, additionally the way constant propagation is currently setup makes it tricky to add cases like `a | false == a`.

I tried to organize `eval_rvalue_with_identities` to make the pattern of the optimizations easier to see but it still obscurs what should be a simple peephole optmization.

cc @oli-obk

diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs
index 3073bf53afd7869486a54daa9f7d13a8f1a86e4c..b3dedd5b8223ebe208ea8e559dfc5dddd218c806 100644 (file)
--- a/src/librustc_mir/transform/const_prop.rs
+++ b/src/librustc_mir/transform/const_prop.rs
@@ -28,9 +28,9 @@
 
 use crate::const_eval::error_to_const_error;
 use crate::interpret::{
-    self, compile_time_machine, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx, LocalState,
-    LocalValue, MemPlace, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Pointer,
-    ScalarMaybeUninit, StackPopCleanup,
+    self, compile_time_machine, truncate, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx,
+    LocalState, LocalValue, MemPlace, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy,
+    Pointer, ScalarMaybeUninit, StackPopCleanup,
 };
 use crate::transform::{MirPass, MirSource};
 
@@ -527,11 +527,11 @@ fn check_binary_op(
         right: &Operand<'tcx>,
         source_info: SourceInfo,
     ) -> Option<()> {
-        let r =
-            self.use_ecx(|this| this.ecx.read_immediate(this.ecx.eval_operand(right, None)?))?;
+        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 op == BinOp::Shr || op == BinOp::Shl {
+            let r = r?;
             // 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);
@@ -564,21 +564,20 @@ fn check_binary_op(
             }
         }
 
-        let l = l?;
-
-        // 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)
-        })? {
-            self.report_assert_as_lint(
-                lint::builtin::ARITHMETIC_OVERFLOW,
-                source_info,
-                "this arithmetic operation will overflow",
-                AssertKind::Overflow(op, l.to_const_int(), r.to_const_int()),
-            )?;
+        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)
+            })? {
+                self.report_assert_as_lint(
+                    lint::builtin::ARITHMETIC_OVERFLOW,
+                    source_info,
+                    "this arithmetic operation will overflow",
+                    AssertKind::Overflow(op, l.to_const_int(), r.to_const_int()),
+                )?;
+            }
         }
-
         Some(())
     }
 
@@ -659,9 +658,74 @@ fn const_prop(
             return None;
         }
 
+        if self.tcx.sess.opts.debugging_opts.mir_opt_level >= 3 {
+            self.eval_rvalue_with_identities(rvalue, place)
+        } else {
+            self.use_ecx(|this| this.ecx.eval_rvalue_into_place(rvalue, place))
+        }
+    }
+
+    // Attempt to use albegraic identities to eliminate constant expressions
+    fn eval_rvalue_with_identities(
+        &mut self,
+        rvalue: &Rvalue<'tcx>,
+        place: Place<'tcx>,
+    ) -> Option<()> {
         self.use_ecx(|this| {
-            trace!("calling eval_rvalue_into_place(rvalue = {:?}, place = {:?})", rvalue, place);
-            this.ecx.eval_rvalue_into_place(rvalue, place)?;
+            match rvalue {
+                Rvalue::BinaryOp(op, left, right) | Rvalue::CheckedBinaryOp(op, left, right) => {
+                    let l = this.ecx.eval_operand(left, None);
+                    let r = this.ecx.eval_operand(right, None);
+
+                    let const_arg = match (l, r) {
+                        (Ok(x), Err(_)) | (Err(_), Ok(x)) => this.ecx.read_immediate(x)?,
+                        (Err(e), Err(_)) => return Err(e),
+                        (Ok(_), Ok(_)) => {
+                            this.ecx.eval_rvalue_into_place(rvalue, place)?;
+                            return Ok(());
+                        }
+                    };
+
+                    let arg_value =
+                        this.ecx.force_bits(const_arg.to_scalar()?, const_arg.layout.size)?;
+                    let dest = this.ecx.eval_place(place)?;
+
+                    match op {
+                        BinOp::BitAnd => {
+                            if arg_value == 0 {
+                                this.ecx.write_immediate(*const_arg, dest)?;
+                            }
+                        }
+                        BinOp::BitOr => {
+                            if arg_value == truncate(u128::MAX, const_arg.layout.size)
+                                || (const_arg.layout.ty.is_bool() && arg_value == 1)
+                            {
+                                this.ecx.write_immediate(*const_arg, dest)?;
+                            }
+                        }
+                        BinOp::Mul => {
+                            if const_arg.layout.ty.is_integral() && arg_value == 0 {
+                                if let Rvalue::CheckedBinaryOp(_, _, _) = rvalue {
+                                    let val = Immediate::ScalarPair(
+                                        const_arg.to_scalar()?.into(),
+                                        Scalar::from_bool(false).into(),
+                                    );
+                                    this.ecx.write_immediate(val, dest)?;
+                                } else {
+                                    this.ecx.write_immediate(*const_arg, dest)?;
+                                }
+                            }
+                        }
+                        _ => {
+                            this.ecx.eval_rvalue_into_place(rvalue, place)?;
+                        }
+                    }
+                }
+                _ => {
+                    this.ecx.eval_rvalue_into_place(rvalue, place)?;
+                }
+            }
+
             Ok(())
         })
     }

diff --git a/src/test/mir-opt/const_prop/boolean_identities.rs b/src/test/mir-opt/const_prop/boolean_identities.rs
new file mode 100644 (file)
index 0000000..4e09acb
--- /dev/null
+++ b/src/test/mir-opt/const_prop/boolean_identities.rs
@@ -0,0 +1,10 @@
+// compile-flags: -O -Zmir-opt-level=3
+
+// EMIT_MIR rustc.test.ConstProp.diff
+pub fn test(x: bool, y: bool) -> bool {
+    (y | true) & (x & false)
+}
+
+fn main() {
+    test(true, false);
+}

diff --git a/src/test/mir-opt/const_prop/boolean_identities/rustc.test.ConstProp.diff b/src/test/mir-opt/const_prop/boolean_identities/rustc.test.ConstProp.diff
new file mode 100644 (file)
index 0000000..b8f0ad4
--- /dev/null
+++ b/src/test/mir-opt/const_prop/boolean_identities/rustc.test.ConstProp.diff
@@ -0,0 +1,53 @@
+- // MIR for `test` before ConstProp
++ // MIR for `test` after ConstProp
+  
+  fn test(_1: bool, _2: bool) -> bool {
+      debug x => _1;                       // in scope 0 at $DIR/boolean_identities.rs:4:13: 4:14
+      debug y => _2;                       // in scope 0 at $DIR/boolean_identities.rs:4:22: 4:23
+      let mut _0: bool;                    // return place in scope 0 at $DIR/boolean_identities.rs:4:34: 4:38
+      let mut _3: bool;                    // in scope 0 at $DIR/boolean_identities.rs:5:5: 5:15
+      let mut _4: bool;                    // in scope 0 at $DIR/boolean_identities.rs:5:6: 5:7
+      let mut _5: bool;                    // in scope 0 at $DIR/boolean_identities.rs:5:18: 5:29
+      let mut _6: bool;                    // in scope 0 at $DIR/boolean_identities.rs:5:19: 5:20
+  
+      bb0: {
+          StorageLive(_3);                 // scope 0 at $DIR/boolean_identities.rs:5:5: 5:15
+          StorageLive(_4);                 // scope 0 at $DIR/boolean_identities.rs:5:6: 5:7
+          _4 = _2;                         // scope 0 at $DIR/boolean_identities.rs:5:6: 5:7
+-         _3 = BitOr(move _4, const true); // scope 0 at $DIR/boolean_identities.rs:5:5: 5:15
++         _3 = const true;                 // scope 0 at $DIR/boolean_identities.rs:5:5: 5:15
+                                           // ty::Const
+                                           // + ty: bool
+                                           // + val: Value(Scalar(0x01))
+                                           // mir::Constant
+-                                          // + span: $DIR/boolean_identities.rs:5:10: 5:14
++                                          // + span: $DIR/boolean_identities.rs:5:5: 5:15
+                                           // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
+          StorageDead(_4);                 // scope 0 at $DIR/boolean_identities.rs:5:14: 5:15
+          StorageLive(_5);                 // scope 0 at $DIR/boolean_identities.rs:5:18: 5:29
+          StorageLive(_6);                 // scope 0 at $DIR/boolean_identities.rs:5:19: 5:20
+          _6 = _1;                         // scope 0 at $DIR/boolean_identities.rs:5:19: 5:20
+-         _5 = BitAnd(move _6, const false); // scope 0 at $DIR/boolean_identities.rs:5:18: 5:29
++         _5 = const false;                // scope 0 at $DIR/boolean_identities.rs:5:18: 5:29
+                                           // ty::Const
+                                           // + ty: bool
+                                           // + val: Value(Scalar(0x00))
+                                           // mir::Constant
+-                                          // + span: $DIR/boolean_identities.rs:5:23: 5:28
++                                          // + span: $DIR/boolean_identities.rs:5:18: 5:29
+                                           // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
+          StorageDead(_6);                 // scope 0 at $DIR/boolean_identities.rs:5:28: 5:29
+-         _0 = BitAnd(move _3, move _5);   // scope 0 at $DIR/boolean_identities.rs:5:5: 5:29
++         _0 = const false;                // scope 0 at $DIR/boolean_identities.rs:5:5: 5:29
++                                          // ty::Const
++                                          // + ty: bool
++                                          // + val: Value(Scalar(0x00))
++                                          // mir::Constant
++                                          // + span: $DIR/boolean_identities.rs:5:5: 5:29
++                                          // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
+          StorageDead(_5);                 // scope 0 at $DIR/boolean_identities.rs:5:28: 5:29
+          StorageDead(_3);                 // scope 0 at $DIR/boolean_identities.rs:5:28: 5:29
+          return;                          // scope 0 at $DIR/boolean_identities.rs:6:2: 6:2
+      }
+  }
+  

diff --git a/src/test/mir-opt/const_prop/mult_by_zero.rs b/src/test/mir-opt/const_prop/mult_by_zero.rs
new file mode 100644 (file)
index 0000000..f40faee
--- /dev/null
+++ b/src/test/mir-opt/const_prop/mult_by_zero.rs
@@ -0,0 +1,10 @@
+// compile-flags: -O -Zmir-opt-level=3
+
+// EMIT_MIR rustc.test.ConstProp.diff
+fn test(x : i32) -> i32 {
+  x * 0
+}
+
+fn main() {
+    test(10);
+}

diff --git a/src/test/mir-opt/const_prop/mult_by_zero/rustc.test.ConstProp.diff b/src/test/mir-opt/const_prop/mult_by_zero/rustc.test.ConstProp.diff
new file mode 100644 (file)
index 0000000..7b36669
--- /dev/null
+++ b/src/test/mir-opt/const_prop/mult_by_zero/rustc.test.ConstProp.diff
@@ -0,0 +1,25 @@
+- // MIR for `test` before ConstProp
++ // MIR for `test` after ConstProp
+  
+  fn test(_1: i32) -> i32 {
+      debug x => _1;                       // in scope 0 at $DIR/mult_by_zero.rs:4:9: 4:10
+      let mut _0: i32;                     // return place in scope 0 at $DIR/mult_by_zero.rs:4:21: 4:24
+      let mut _2: i32;                     // in scope 0 at $DIR/mult_by_zero.rs:5:3: 5:4
+  
+      bb0: {
+          StorageLive(_2);                 // scope 0 at $DIR/mult_by_zero.rs:5:3: 5:4
+          _2 = _1;                         // scope 0 at $DIR/mult_by_zero.rs:5:3: 5:4
+-         _0 = Mul(move _2, const 0_i32);  // scope 0 at $DIR/mult_by_zero.rs:5:3: 5:8
++         _0 = const 0_i32;                // scope 0 at $DIR/mult_by_zero.rs:5:3: 5:8
+                                           // ty::Const
+                                           // + ty: i32
+                                           // + val: Value(Scalar(0x00000000))
+                                           // mir::Constant
+-                                          // + span: $DIR/mult_by_zero.rs:5:7: 5:8
++                                          // + span: $DIR/mult_by_zero.rs:5:3: 5:8
+                                           // + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
+          StorageDead(_2);                 // scope 0 at $DIR/mult_by_zero.rs:5:7: 5:8
+          return;                          // scope 0 at $DIR/mult_by_zero.rs:6:2: 6:2
+      }
+  }
+  

diff --git a/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.rs b/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.rs
index 2a983e426838c683a0b3d3fbabb8ae9ecf8e71e0..795c5154f815551d04c1b0dfa6354c002081a9c1 100644 (file)
--- a/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.rs
+++ b/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.rs
@@ -1,7 +1,7 @@
 // build-fail
 
 // Regression test for #66975
-#![warn(const_err)]
+#![warn(const_err, unconditional_panic)]
 #![feature(never_type)]
 
 struct PrintName<T>(T);
@@ -9,6 +9,7 @@
 impl<T> PrintName<T> {
     const VOID: ! = { let x = 0 * std::mem::size_of::<T>(); [][x] };
     //~^ WARN any use of this value will cause an error
+
 }
 
 fn f<T>() {

diff --git a/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.stderr b/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.stderr
index d78e0da00f5e17723dbd25199aec95b686b6b8b9..33e60dd7c9138c725e43441a8b2dbd677829ed3e 100644 (file)
--- a/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.stderr
+++ b/src/test/ui/consts/const-eval/index-out-of-bounds-never-type.stderr
@@ -9,11 +9,11 @@ LL |     const VOID: ! = { let x = 0 * std::mem::size_of::<T>(); [][x] };
 note: the lint level is defined here
   --> $DIR/index-out-of-bounds-never-type.rs:4:9
    |
-LL | #![warn(const_err)]
+LL | #![warn(const_err, unconditional_panic)]
    |         ^^^^^^^^^
 
 error: erroneous constant encountered
-  --> $DIR/index-out-of-bounds-never-type.rs:15:13
+  --> $DIR/index-out-of-bounds-never-type.rs:16:13
    |
 LL |     let _ = PrintName::<T>::VOID;
    |             ^^^^^^^^^^^^^^^^^^^^