Skip to content

Commit e59effe

Browse files
ManishearthManishearth
authored
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
2 parents 3226d72 + 711a680 commit e59effe

File tree

7 files changed

+187
-24
lines changed

7 files changed

+187
-24
lines changed

‎src/librustc_mir/transform/const_prop.rs

+85-21
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ use rustc_trait_selection::traits;
2828

2929
use crate::const_eval::error_to_const_error;
3030
use crate::interpret::{
31-
self, compile_time_machine, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx, LocalState,
32-
LocalValue, MemPlace, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Pointer,
33-
ScalarMaybeUninit, StackPopCleanup,
31+
self, compile_time_machine, truncate, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx,
32+
LocalState, LocalValue, MemPlace, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy,
33+
Pointer, ScalarMaybeUninit, StackPopCleanup,
3434
};
3535
use crate::transform::{MirPass, MirSource};
3636

@@ -527,11 +527,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
527527
right: &Operand<'tcx>,
528528
source_info: SourceInfo,
529529
) -> Option<()> {
530-
let r =
531-
self.use_ecx(|this| this.ecx.read_immediate(this.ecx.eval_operand(right, None)?))?;
530+
let r = self.use_ecx(|this| this.ecx.read_immediate(this.ecx.eval_operand(right, None)?));
532531
let l = self.use_ecx(|this| this.ecx.read_immediate(this.ecx.eval_operand(left, None)?));
533532
// Check for exceeding shifts *even if* we cannot evaluate the LHS.
534533
if op == BinOp::Shr || op == BinOp::Shl {
534+
let r = r?;
535535
// We need the type of the LHS. We cannot use `place_layout` as that is the type
536536
// of the result, which for checked binops is not the same!
537537
let left_ty = left.ty(&self.local_decls, self.tcx);
@@ -564,21 +564,20 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
564564
}
565565
}
566566

567-
let l = l?;
568-
569-
// The remaining operators are handled through `overflowing_binary_op`.
570-
if self.use_ecx(|this| {
571-
let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
572-
Ok(overflow)
573-
})? {
574-
self.report_assert_as_lint(
575-
lint::builtin::ARITHMETIC_OVERFLOW,
576-
source_info,
577-
"this arithmetic operation will overflow",
578-
AssertKind::Overflow(op, l.to_const_int(), r.to_const_int()),
579-
)?;
567+
if let (Some(l), Some(r)) = (l, r) {
568+
// The remaining operators are handled through `overflowing_binary_op`.
569+
if self.use_ecx(|this| {
570+
let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
571+
Ok(overflow)
572+
})? {
573+
self.report_assert_as_lint(
574+
lint::builtin::ARITHMETIC_OVERFLOW,
575+
source_info,
576+
"this arithmetic operation will overflow",
577+
AssertKind::Overflow(op, l.to_const_int(), r.to_const_int()),
578+
)?;
579+
}
580580
}
581-
582581
Some(())
583582
}
584583

@@ -659,9 +658,74 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
659658
return None;
660659
}
661660

661+
if self.tcx.sess.opts.debugging_opts.mir_opt_level >= 3 {
662+
self.eval_rvalue_with_identities(rvalue, place)
663+
} else {
664+
self.use_ecx(|this| this.ecx.eval_rvalue_into_place(rvalue, place))
665+
}
666+
}
667+
668+
// Attempt to use albegraic identities to eliminate constant expressions
669+
fn eval_rvalue_with_identities(
670+
&mut self,
671+
rvalue: &Rvalue<'tcx>,
672+
place: Place<'tcx>,
673+
) -> Option<()> {
662674
self.use_ecx(|this| {
663-
trace!("calling eval_rvalue_into_place(rvalue = {:?}, place = {:?})", rvalue, place);
664-
this.ecx.eval_rvalue_into_place(rvalue, place)?;
675+
match rvalue {
676+
Rvalue::BinaryOp(op, left, right) | Rvalue::CheckedBinaryOp(op, left, right) => {
677+
let l = this.ecx.eval_operand(left, None);
678+
let r = this.ecx.eval_operand(right, None);
679+
680+
let const_arg = match (l, r) {
681+
(Ok(x), Err(_)) | (Err(_), Ok(x)) => this.ecx.read_immediate(x)?,
682+
(Err(e), Err(_)) => return Err(e),
683+
(Ok(_), Ok(_)) => {
684+
this.ecx.eval_rvalue_into_place(rvalue, place)?;
685+
return Ok(());
686+
}
687+
};
688+
689+
let arg_value =
690+
this.ecx.force_bits(const_arg.to_scalar()?, const_arg.layout.size)?;
691+
let dest = this.ecx.eval_place(place)?;
692+
693+
match op {
694+
BinOp::BitAnd => {
695+
if arg_value == 0 {
696+
this.ecx.write_immediate(*const_arg, dest)?;
697+
}
698+
}
699+
BinOp::BitOr => {
700+
if arg_value == truncate(u128::MAX, const_arg.layout.size)
701+
|| (const_arg.layout.ty.is_bool() && arg_value == 1)
702+
{
703+
this.ecx.write_immediate(*const_arg, dest)?;
704+
}
705+
}
706+
BinOp::Mul => {
707+
if const_arg.layout.ty.is_integral() && arg_value == 0 {
708+
if let Rvalue::CheckedBinaryOp(_, _, _) = rvalue {
709+
let val = Immediate::ScalarPair(
710+
const_arg.to_scalar()?.into(),
711+
Scalar::from_bool(false).into(),
712+
);
713+
this.ecx.write_immediate(val, dest)?;
714+
} else {
715+
this.ecx.write_immediate(*const_arg, dest)?;
716+
}
717+
}
718+
}
719+
_ => {
720+
this.ecx.eval_rvalue_into_place(rvalue, place)?;
721+
}
722+
}
723+
}
724+
_ => {
725+
this.ecx.eval_rvalue_into_place(rvalue, place)?;
726+
}
727+
}
728+
665729
Ok(())
666730
})
667731
}

‎src/test/mir-opt/const_prop/boolean_identities.rs

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// compile-flags: -O -Zmir-opt-level=3
2+
3+
// EMIT_MIR rustc.test.ConstProp.diff
4+
pub fn test(x: bool, y: bool) -> bool {
5+
(y | true) & (x & false)
6+
}
7+
8+
fn main() {
9+
test(true, false);
10+
}

‎src/test/mir-opt/const_prop/boolean_identities/rustc.test.ConstProp.diff

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
- // MIR for `test` before ConstProp
2+
+ // MIR for `test` after ConstProp
3+
4+
fn test(_1: bool, _2: bool) -> bool {
5+
debug x => _1; // in scope 0 at $DIR/boolean_identities.rs:4:13: 4:14
6+
debug y => _2; // in scope 0 at $DIR/boolean_identities.rs:4:22: 4:23
7+
let mut _0: bool; // return place in scope 0 at $DIR/boolean_identities.rs:4:34: 4:38
8+
let mut _3: bool; // in scope 0 at $DIR/boolean_identities.rs:5:5: 5:15
9+
let mut _4: bool; // in scope 0 at $DIR/boolean_identities.rs:5:6: 5:7
10+
let mut _5: bool; // in scope 0 at $DIR/boolean_identities.rs:5:18: 5:29
11+
let mut _6: bool; // in scope 0 at $DIR/boolean_identities.rs:5:19: 5:20
12+
13+
bb0: {
14+
StorageLive(_3); // scope 0 at $DIR/boolean_identities.rs:5:5: 5:15
15+
StorageLive(_4); // scope 0 at $DIR/boolean_identities.rs:5:6: 5:7
16+
_4 = _2; // scope 0 at $DIR/boolean_identities.rs:5:6: 5:7
17+
- _3 = BitOr(move _4, const true); // scope 0 at $DIR/boolean_identities.rs:5:5: 5:15
18+
+ _3 = const true; // scope 0 at $DIR/boolean_identities.rs:5:5: 5:15
19+
// ty::Const
20+
// + ty: bool
21+
// + val: Value(Scalar(0x01))
22+
// mir::Constant
23+
- // + span: $DIR/boolean_identities.rs:5:10: 5:14
24+
+ // + span: $DIR/boolean_identities.rs:5:5: 5:15
25+
// + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
26+
StorageDead(_4); // scope 0 at $DIR/boolean_identities.rs:5:14: 5:15
27+
StorageLive(_5); // scope 0 at $DIR/boolean_identities.rs:5:18: 5:29
28+
StorageLive(_6); // scope 0 at $DIR/boolean_identities.rs:5:19: 5:20
29+
_6 = _1; // scope 0 at $DIR/boolean_identities.rs:5:19: 5:20
30+
- _5 = BitAnd(move _6, const false); // scope 0 at $DIR/boolean_identities.rs:5:18: 5:29
31+
+ _5 = const false; // scope 0 at $DIR/boolean_identities.rs:5:18: 5:29
32+
// ty::Const
33+
// + ty: bool
34+
// + val: Value(Scalar(0x00))
35+
// mir::Constant
36+
- // + span: $DIR/boolean_identities.rs:5:23: 5:28
37+
+ // + span: $DIR/boolean_identities.rs:5:18: 5:29
38+
// + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
39+
StorageDead(_6); // scope 0 at $DIR/boolean_identities.rs:5:28: 5:29
40+
- _0 = BitAnd(move _3, move _5); // scope 0 at $DIR/boolean_identities.rs:5:5: 5:29
41+
+ _0 = const false; // scope 0 at $DIR/boolean_identities.rs:5:5: 5:29
42+
+ // ty::Const
43+
+ // + ty: bool
44+
+ // + val: Value(Scalar(0x00))
45+
+ // mir::Constant
46+
+ // + span: $DIR/boolean_identities.rs:5:5: 5:29
47+
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
48+
StorageDead(_5); // scope 0 at $DIR/boolean_identities.rs:5:28: 5:29
49+
StorageDead(_3); // scope 0 at $DIR/boolean_identities.rs:5:28: 5:29
50+
return; // scope 0 at $DIR/boolean_identities.rs:6:2: 6:2
51+
}
52+
}
53+

‎src/test/mir-opt/const_prop/mult_by_zero.rs

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// compile-flags: -O -Zmir-opt-level=3
2+
3+
// EMIT_MIR rustc.test.ConstProp.diff
4+
fn test(x : i32) -> i32 {
5+
x * 0
6+
}
7+
8+
fn main() {
9+
test(10);
10+
}

‎src/test/mir-opt/const_prop/mult_by_zero/rustc.test.ConstProp.diff

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
- // MIR for `test` before ConstProp
2+
+ // MIR for `test` after ConstProp
3+
4+
fn test(_1: i32) -> i32 {
5+
debug x => _1; // in scope 0 at $DIR/mult_by_zero.rs:4:9: 4:10
6+
let mut _0: i32; // return place in scope 0 at $DIR/mult_by_zero.rs:4:21: 4:24
7+
let mut _2: i32; // in scope 0 at $DIR/mult_by_zero.rs:5:3: 5:4
8+
9+
bb0: {
10+
StorageLive(_2); // scope 0 at $DIR/mult_by_zero.rs:5:3: 5:4
11+
_2 = _1; // scope 0 at $DIR/mult_by_zero.rs:5:3: 5:4
12+
- _0 = Mul(move _2, const 0_i32); // scope 0 at $DIR/mult_by_zero.rs:5:3: 5:8
13+
+ _0 = const 0_i32; // scope 0 at $DIR/mult_by_zero.rs:5:3: 5:8
14+
// ty::Const
15+
// + ty: i32
16+
// + val: Value(Scalar(0x00000000))
17+
// mir::Constant
18+
- // + span: $DIR/mult_by_zero.rs:5:7: 5:8
19+
+ // + span: $DIR/mult_by_zero.rs:5:3: 5:8
20+
// + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
21+
StorageDead(_2); // scope 0 at $DIR/mult_by_zero.rs:5:7: 5:8
22+
return; // scope 0 at $DIR/mult_by_zero.rs:6:2: 6:2
23+
}
24+
}
25+

‎src/test/ui/consts/const-eval/index-out-of-bounds-never-type.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
// build-fail
22

33
// Regression test for #66975
4-
#![warn(const_err)]
4+
#![warn(const_err, unconditional_panic)]
55
#![feature(never_type)]
66

77
struct PrintName<T>(T);
88

99
impl<T> PrintName<T> {
1010
const VOID: ! = { let x = 0 * std::mem::size_of::<T>(); [][x] };
1111
//~^ WARN any use of this value will cause an error
12+
1213
}
1314

1415
fn f<T>() {

‎src/test/ui/consts/const-eval/index-out-of-bounds-never-type.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ LL | const VOID: ! = { let x = 0 * std::mem::size_of::<T>(); [][x] };
99
note: the lint level is defined here
1010
--> $DIR/index-out-of-bounds-never-type.rs:4:9
1111
|
12-
LL | #![warn(const_err)]
12+
LL | #![warn(const_err, unconditional_panic)]
1313
| ^^^^^^^^^
1414

1515
error: erroneous constant encountered
16-
--> $DIR/index-out-of-bounds-never-type.rs:15:13
16+
--> $DIR/index-out-of-bounds-never-type.rs:16:13
1717
|
1818
LL | let _ = PrintName::<T>::VOID;
1919
| ^^^^^^^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)