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
This commit is contained in:
Manish Goregaokar 2020-07-24 10:01:32 -07:00 committed by GitHub
commit e59effed30
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 187 additions and 24 deletions

View file

@ -28,9 +28,9 @@ use rustc_trait_selection::traits;
use crate::const_eval::error_to_const_error; use crate::const_eval::error_to_const_error;
use crate::interpret::{ use crate::interpret::{
self, compile_time_machine, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx, LocalState, self, compile_time_machine, truncate, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx,
LocalValue, MemPlace, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Pointer, LocalState, LocalValue, MemPlace, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy,
ScalarMaybeUninit, StackPopCleanup, Pointer, ScalarMaybeUninit, StackPopCleanup,
}; };
use crate::transform::{MirPass, MirSource}; use crate::transform::{MirPass, MirSource};
@ -527,11 +527,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
right: &Operand<'tcx>, right: &Operand<'tcx>,
source_info: SourceInfo, source_info: SourceInfo,
) -> Option<()> { ) -> Option<()> {
let r = let r = self.use_ecx(|this| this.ecx.read_immediate(this.ecx.eval_operand(right, None)?));
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)?)); 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. // Check for exceeding shifts *even if* we cannot evaluate the LHS.
if op == BinOp::Shr || op == BinOp::Shl { 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 // 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! // of the result, which for checked binops is not the same!
let left_ty = left.ty(&self.local_decls, self.tcx); let left_ty = left.ty(&self.local_decls, self.tcx);
@ -564,8 +564,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
} }
} }
let l = l?; if let (Some(l), Some(r)) = (l, r) {
// The remaining operators are handled through `overflowing_binary_op`. // The remaining operators are handled through `overflowing_binary_op`.
if self.use_ecx(|this| { if self.use_ecx(|this| {
let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?; let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
@ -578,7 +577,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
AssertKind::Overflow(op, l.to_const_int(), r.to_const_int()), AssertKind::Overflow(op, l.to_const_int(), r.to_const_int()),
)?; )?;
} }
}
Some(()) Some(())
} }
@ -659,9 +658,74 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
return None; 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| { self.use_ecx(|this| {
trace!("calling eval_rvalue_into_place(rvalue = {:?}, 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)?; 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(()) Ok(())
}) })
} }

View file

@ -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);
}

View file

@ -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
}
}

View file

@ -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);
}

View file

@ -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
}
}

View file

@ -1,7 +1,7 @@
// build-fail // build-fail
// Regression test for #66975 // Regression test for #66975
#![warn(const_err)] #![warn(const_err, unconditional_panic)]
#![feature(never_type)] #![feature(never_type)]
struct PrintName<T>(T); struct PrintName<T>(T);
@ -9,6 +9,7 @@ struct PrintName<T>(T);
impl<T> PrintName<T> { impl<T> PrintName<T> {
const VOID: ! = { let x = 0 * std::mem::size_of::<T>(); [][x] }; const VOID: ! = { let x = 0 * std::mem::size_of::<T>(); [][x] };
//~^ WARN any use of this value will cause an error //~^ WARN any use of this value will cause an error
} }
fn f<T>() { fn f<T>() {

View file

@ -9,11 +9,11 @@ LL | const VOID: ! = { let x = 0 * std::mem::size_of::<T>(); [][x] };
note: the lint level is defined here note: the lint level is defined here
--> $DIR/index-out-of-bounds-never-type.rs:4:9 --> $DIR/index-out-of-bounds-never-type.rs:4:9
| |
LL | #![warn(const_err)] LL | #![warn(const_err, unconditional_panic)]
| ^^^^^^^^^ | ^^^^^^^^^
error: erroneous constant encountered 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; LL | let _ = PrintName::<T>::VOID;
| ^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^