Skip to content

Commit

Permalink
Rollup merge of rust-lang#71697 - felix91gr:new_prop_into_fn_call, r=…
Browse files Browse the repository at this point in the history 
…wesleywiser

Added MIR constant propagation of Scalars into function call arguments

Now for the function call arguments!

Caveats:
1. It's only being enabled at `mir-opt-2` or higher, because currently codegen gives performance regressions with this optimization.
2. Only propagates Scalars. Tuples and references (references are `Indirect`, right??) are not being propagated into as of this PR.
3. Maybe more tests would be nice?
4. I need (shamefully) to ask @wesleywiser to write in his words (or explain to me, and then I can write it down) why we want to ignore propagation into `ScalarPairs` and `Indirect` arguments.

r? @wesleywiser
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored May 4, 2020
2 parents e6389ef + d0dea9f commit c476b28
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 10 deletions.
56 changes: 51 additions & 5 deletions src/librustc_mir/transform/const_prop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp {
| NonMutatingUse(NonMutatingUseContext::Inspect)
| NonMutatingUse(NonMutatingUseContext::Projection)
| NonUse(_) => {}
// FIXME(felix91gr): explain the reasoning behind this
MutatingUse(MutatingUseContext::Projection) => {
if self.local_kinds[local] != LocalKind::Temp {
self.can_const_prop[local] = ConstPropMode::NoPropagation;
Expand Down Expand Up @@ -969,13 +970,58 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
| TerminatorKind::GeneratorDrop
| TerminatorKind::FalseEdges { .. }
| TerminatorKind::FalseUnwind { .. } => {}
//FIXME(wesleywiser) Call does have Operands that could be const-propagated
TerminatorKind::Call { .. } => {}
// 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().and_then(|p| p.as_local()) {
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(
interpret::ScalarMaybeUndef::Scalar(scalar),
),
) = *value
{
*opr = self.operand_from_scalar(
scalar,
value.layout.ty,
source_info.span,
);
}
}
}
}
}
}
}
}
// We remove all Locals which are restricted in propagation to their containing blocks.
// We wouldn't need to clone, but the borrow checker can't see that we're not aliasing
// the locals_of_current_block field, so we need to clone it first.
// let ecx = &mut self.ecx;
for local in self.locals_of_current_block.iter() {
Self::remove_const(&mut self.ecx, local);
}
Expand Down
13 changes: 10 additions & 3 deletions src/test/mir-opt/const_prop/scalar_literal_propagation/rustc.main.ConstProp.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,27 @@
StorageLive(_2); // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
StorageLive(_3); // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
- _3 = _1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
- _2 = const consume(move _3) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
+ _3 = const 1u32; // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
+ // ty::Const
// ty::Const
+ // + ty: u32
+ // + val: Value(Scalar(0x00000001))
+ // mir::Constant
+ // + span: $DIR/scalar_literal_propagation.rs:4:13: 4:14
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000001)) }
_2 = const consume(move _3) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
// ty::Const
+ _2 = const consume(const 1u32) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
+ // ty::Const
// + ty: fn(u32) {consume}
// + val: Value(Scalar(<ZST>))
// mir::Constant
// + span: $DIR/scalar_literal_propagation.rs:4:5: 4:12
// + literal: Const { ty: fn(u32) {consume}, val: Value(Scalar(<ZST>)) }
+ // ty::Const
+ // + ty: u32
+ // + val: Value(Scalar(0x00000001))
+ // mir::Constant
+ // + span: $DIR/scalar_literal_propagation.rs:4:5: 4:15
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000001)) }
}

bb1: {
Expand Down
22 changes: 20 additions & 2 deletions src/test/mir-opt/simplify-locals-removes-unused-consts/rustc.main.SimplifyLocals.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
- StorageDead(_2); // scope 0 at $DIR/simplify-locals-removes-unused-consts.rs:13:27: 13:28
- StorageDead(_1); // scope 0 at $DIR/simplify-locals-removes-unused-consts.rs:13:28: 13:29
- StorageLive(_4); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:5: 14:22
- StorageLive(_5); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21
- StorageLive(_6); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:14: 14:16
- _6 = const (); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:14: 14:16
- // ty::Const
Expand All @@ -66,6 +67,13 @@
- // mir::Constant
- // + span: $DIR/simplify-locals-removes-unused-consts.rs:14:18: 14:20
- // + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
- _5 = const ((), ()); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21
- // ty::Const
- // + ty: ((), ())
- // + val: Value(Scalar(<ZST>))
- // mir::Constant
- // + span: $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21
- // + literal: Const { ty: ((), ()), val: Value(Scalar(<ZST>)) }
- StorageDead(_7); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:20: 14:21
- StorageDead(_6); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:20: 14:21
- _4 = const use_zst(const ((), ())) -> bb1; // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:5: 14:22
Expand All @@ -79,13 +87,15 @@
// + ty: ((), ())
// + val: Value(Scalar(<ZST>))
// mir::Constant
// + span: $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21
// + span: $DIR/simplify-locals-removes-unused-consts.rs:14:5: 14:22
// + literal: Const { ty: ((), ()), val: Value(Scalar(<ZST>)) }
}

bb1: {
- StorageDead(_5); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:21: 14:22
- StorageDead(_4); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:22: 14:23
- StorageLive(_8); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:5: 16:35
- StorageLive(_9); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:34
- StorageLive(_10); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:30
- StorageLive(_11); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:28
- _11 = const Temp { x: 40u8 }; // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:28
Expand All @@ -105,6 +115,13 @@
- // mir::Constant
- // + span: $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:30
- // + literal: Const { ty: u8, val: Value(Scalar(0x28)) }
- _9 = const 42u8; // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:34
- // ty::Const
- // + ty: u8
- // + val: Value(Scalar(0x2a))
- // mir::Constant
- // + span: $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:34
- // + literal: Const { ty: u8, val: Value(Scalar(0x2a)) }
- StorageDead(_10); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:33: 16:34
- _8 = const use_u8(const 42u8) -> bb2; // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:5: 16:35
- // ty::Const
Expand All @@ -117,11 +134,12 @@
// + ty: u8
// + val: Value(Scalar(0x2a))
// mir::Constant
// + span: $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:34
// + span: $DIR/simplify-locals-removes-unused-consts.rs:16:5: 16:35
// + literal: Const { ty: u8, val: Value(Scalar(0x2a)) }
}

bb2: {
- StorageDead(_9); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:34: 16:35
- StorageDead(_11); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:35: 16:36
- StorageDead(_8); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:35: 16:36
+ StorageDead(_2); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:35: 16:36
Expand Down

0 comments on commit c476b28

Please sign in to comment.