Skip to content

Commit

Permalink
Rollup merge of rust-lang#74507 - lcnr:const-prop-into-op, r=oli-obk
Browse files Browse the repository at this point in the history 
add `visit_operand` to const prop

r? @oli-obk
  • Loading branch information
@Manishearth
Manishearth authored Jul 22, 2020
2 parents 45f2730 + d257bac commit 792d485
Show file tree
Hide file tree
Showing 16 changed files with 261 additions and 89 deletions.
112 changes: 50 additions & 62 deletions src/librustc_mir/transform/const_prop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,34 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
Some(())
}

fn propagate_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) {
match *operand {
Operand::Copy(l) | Operand::Move(l) => {
if let Some(value) = self.get_const(l) {
if self.should_const_prop(value) {
// FIXME(felix91gr): this code only handles `Scalar` cases.
// For now, we're not handling `ScalarPair` cases because
// doing so here would require a lot of code duplication.
// We should hopefully generalize `Operand` handling into a fn,
// and use it to do const-prop here and everywhere else
// where it makes sense.
if let interpret::Operand::Immediate(interpret::Immediate::Scalar(
ScalarMaybeUninit::Scalar(scalar),
)) = *value
{
*operand = self.operand_from_scalar(
scalar,
value.layout.ty,
self.source_info.unwrap().span,
);
}
}
}
}
Operand::Constant(ref mut ct) => self.visit_constant(ct, location),
}
}

fn const_prop(
&mut self,
rvalue: &Rvalue<'tcx>,
Expand Down Expand Up @@ -905,6 +933,16 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
}
}

fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) {
// Only const prop copies and moves on `mir_opt_level=3` as doing so
// currently increases compile time.
if self.tcx.sess.opts.debugging_opts.mir_opt_level < 3 {
self.super_operand(operand, location)
} else {
self.propagate_operand(operand, location)
}
}

fn visit_constant(&mut self, constant: &mut Constant<'tcx>, location: Location) {
trace!("visit_constant: {:?}", constant);
self.super_constant(constant, location);
Expand Down Expand Up @@ -1072,18 +1110,13 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
}
}
}
TerminatorKind::SwitchInt { ref mut discr, switch_ty, .. } => {
if let Some(value) = self.eval_operand(&discr, source_info) {
if self.should_const_prop(value) {
if let ScalarMaybeUninit::Scalar(scalar) =
self.ecx.read_scalar(value).unwrap()
{
*discr = self.operand_from_scalar(scalar, switch_ty, source_info.span);
}
}
}
TerminatorKind::SwitchInt { ref mut discr, .. } => {
// FIXME: This is currently redundant with `visit_operand`, but sadly
// always visiting operands currently causes a perf regression in LLVM codegen, so
// `visit_operand` currently only runs for propagates places for `mir_opt_level=3`.
self.propagate_operand(discr, location)
}
// None of these have Operands to const-propagate
// None of these have Operands to const-propagate.
TerminatorKind::Goto { .. }
| TerminatorKind::Resume
| TerminatorKind::Abort
Expand All @@ -1096,61 +1129,16 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::InlineAsm { .. } => {}
// Every argument in our function calls can be const propagated.
TerminatorKind::Call { ref mut args, .. } => {
let mir_opt_level = self.tcx.sess.opts.debugging_opts.mir_opt_level;
// Constant Propagation into function call arguments is gated
// under mir-opt-level 2, because LLVM codegen gives performance
// regressions with it.
if mir_opt_level >= 2 {
for opr in args {
/*
The following code would appear to be incomplete, because
the function `Operand::place()` returns `None` if the
`Operand` is of the variant `Operand::Constant`. In this
context however, that variant will never appear. This is why:
When constructing the MIR, all function call arguments are
copied into `Locals` of `LocalKind::Temp`. At least, all arguments
that are not unsized (Less than 0.1% are unsized. See #71170
to learn more about those).
This means that, conversely, all `Operands` found as function call
arguments are of the variant `Operand::Copy`. This allows us to
simplify our handling of `Operands` in this case.
*/
if let Some(l) = opr.place() {
if let Some(value) = self.get_const(l) {
if self.should_const_prop(value) {
// FIXME(felix91gr): this code only handles `Scalar` cases.
// For now, we're not handling `ScalarPair` cases because
// doing so here would require a lot of code duplication.
// We should hopefully generalize `Operand` handling into a fn,
// and use it to do const-prop here and everywhere else
// where it makes sense.
if let interpret::Operand::Immediate(
interpret::Immediate::Scalar(ScalarMaybeUninit::Scalar(
scalar,
)),
) = *value
{
*opr = self.operand_from_scalar(
scalar,
value.layout.ty,
source_info.span,
);
}
}
}
}
}
}
}
// Every argument in our function calls have already been propagated in `visit_operand`.
//
// NOTE: because LLVM codegen gives performance regressions with it, so this is gated
// on `mir_opt_level=3`.
TerminatorKind::Call { .. } => {}
}

// We remove all Locals which are restricted in propagation to their containing blocks and
// which were modified in the current block.
// Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`
// Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`.
let mut locals = std::mem::take(&mut self.ecx.machine.written_only_inside_own_block_locals);
for &local in locals.iter() {
Self::remove_const(&mut self.ecx, local);
Expand Down
14 changes: 13 additions & 1 deletion src/test/mir-opt/const_prop/array_index/32bit/rustc.main.ConstProp.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,25 @@
+ // mir::Constant
+ // + span: $DIR/array_index.rs:5:18: 5:33
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
+ assert(const true, "index out of bounds: the len is {} but the index is {}", move _4, _3) -> bb1; // scope 0 at $DIR/array_index.rs:5:18: 5:33
+ assert(const true, "index out of bounds: the len is {} but the index is {}", const 4_usize, const 2_usize) -> bb1; // scope 0 at $DIR/array_index.rs:5:18: 5:33
+ // ty::Const
+ // + ty: bool
+ // + val: Value(Scalar(0x01))
+ // mir::Constant
+ // + span: $DIR/array_index.rs:5:18: 5:33
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
+ // ty::Const
+ // + ty: usize
+ // + val: Value(Scalar(0x00000004))
+ // mir::Constant
+ // + span: $DIR/array_index.rs:5:18: 5:33
+ // + literal: Const { ty: usize, val: Value(Scalar(0x00000004)) }
+ // ty::Const
+ // + ty: usize
+ // + val: Value(Scalar(0x00000002))
+ // mir::Constant
+ // + span: $DIR/array_index.rs:5:18: 5:33
+ // + literal: Const { ty: usize, val: Value(Scalar(0x00000002)) }
}

bb1: {
Expand Down
14 changes: 13 additions & 1 deletion src/test/mir-opt/const_prop/array_index/64bit/rustc.main.ConstProp.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,25 @@
+ // mir::Constant
+ // + span: $DIR/array_index.rs:5:18: 5:33
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
+ assert(const true, "index out of bounds: the len is {} but the index is {}", move _4, _3) -> bb1; // scope 0 at $DIR/array_index.rs:5:18: 5:33
+ assert(const true, "index out of bounds: the len is {} but the index is {}", const 4_usize, const 2_usize) -> bb1; // scope 0 at $DIR/array_index.rs:5:18: 5:33
+ // ty::Const
+ // + ty: bool
+ // + val: Value(Scalar(0x01))
+ // mir::Constant
+ // + span: $DIR/array_index.rs:5:18: 5:33
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
+ // ty::Const
+ // + ty: usize
+ // + val: Value(Scalar(0x0000000000000004))
+ // mir::Constant
+ // + span: $DIR/array_index.rs:5:18: 5:33
+ // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000004)) }
+ // ty::Const
+ // + ty: usize
+ // + val: Value(Scalar(0x0000000000000002))
+ // mir::Constant
+ // + span: $DIR/array_index.rs:5:18: 5:33
+ // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000002)) }
}

bb1: {
Expand Down
32 changes: 26 additions & 6 deletions src/test/mir-opt/const_prop/bad_op_div_by_zero/rustc.main.ConstProp.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,22 @@
- // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // + span: $DIR/bad_op_div_by_zero.rs:5:18: 5:19
// + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
- assert(!move _4, "attempt to divide {} by zero", const 1_i32) -> bb1; // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ _4 = const true; // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
// ty::Const
+ // + ty: bool
+ // + val: Value(Scalar(0x01))
+ // mir::Constant
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
+ assert(!const true, "attempt to divide {} by zero", const 1_i32) -> bb1; // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // ty::Const
+ // + ty: bool
+ // + val: Value(Scalar(0x01))
+ // mir::Constant
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
assert(!move _4, "attempt to divide {} by zero", const 1_i32) -> bb1; // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
// ty::Const
+ // ty::Const
// + ty: i32
// + val: Value(Scalar(0x00000001))
// mir::Constant
Expand Down Expand Up @@ -90,29 +97,42 @@
- _7 = BitAnd(move _5, move _6); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
- assert(!move _7, "attempt to compute `{} / {}` which would overflow", const 1_i32, _3) -> bb2; // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
+ assert(!const false, "attempt to compute `{} / {}` which would overflow", const 1_i32, _3) -> bb2; // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // ty::Const
+ assert(!const false, "attempt to compute `{} / {}` which would overflow", const 1_i32, const 0_i32) -> bb2; // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
// ty::Const
+ // + ty: bool
+ // + val: Value(Scalar(0x00))
+ // mir::Constant
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
// ty::Const
+ // ty::Const
// + ty: i32
// + val: Value(Scalar(0x00000001))
// mir::Constant
// + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:15
// + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) }
+ // ty::Const
+ // + ty: i32
+ // + val: Value(Scalar(0x00000000))
+ // mir::Constant
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
}

bb2: {
_2 = Div(const 1_i32, move _3); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
- _2 = Div(const 1_i32, move _3); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ _2 = Div(const 1_i32, const 0_i32); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
// ty::Const
// + ty: i32
// + val: Value(Scalar(0x00000001))
// mir::Constant
// + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:15
// + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) }
+ // ty::Const
+ // + ty: i32
+ // + val: Value(Scalar(0x00000000))
+ // mir::Constant
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ // + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
StorageDead(_3); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:18: 5:19
_0 = const (); // scope 0 at $DIR/bad_op_div_by_zero.rs:3:11: 6:2
// ty::Const
Expand Down
Loading

0 comments on commit 792d485

Please sign in to comment.