From d0dea9f5887c8371279b0b07ce0380f060ca1e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Fischer?= Date: Wed, 29 Apr 2020 18:59:13 -0400 Subject: [PATCH] Added MIR constant propagation of Scalars into function call arguments - Documented rationale of current solution - Polished documentation --- src/librustc_mir/transform/const_prop.rs | 56 +++++++++++++++++-- .../rustc.main.ConstProp.diff | 13 ++++- .../rustc.main.SimplifyLocals.diff | 22 +++++++- 3 files changed, 81 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 09d8f89676a66..a34b9dac42808 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -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; @@ -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); } diff --git a/src/test/mir-opt/const_prop/scalar_literal_propagation/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/scalar_literal_propagation/rustc.main.ConstProp.diff index 0183ff7716cbb..596ddcb43533b 100644 --- a/src/test/mir-opt/const_prop/scalar_literal_propagation/rustc.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/scalar_literal_propagation/rustc.main.ConstProp.diff @@ -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()) // mir::Constant // + span: $DIR/scalar_literal_propagation.rs:4:5: 4:12 // + literal: Const { ty: fn(u32) {consume}, val: Value(Scalar()) } ++ // 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: { diff --git a/src/test/mir-opt/simplify-locals-removes-unused-consts/rustc.main.SimplifyLocals.diff b/src/test/mir-opt/simplify-locals-removes-unused-consts/rustc.main.SimplifyLocals.diff index 0742f655730c5..0bd4ba97b3ca0 100644 --- a/src/test/mir-opt/simplify-locals-removes-unused-consts/rustc.main.SimplifyLocals.diff +++ b/src/test/mir-opt/simplify-locals-removes-unused-consts/rustc.main.SimplifyLocals.diff @@ -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 @@ -66,6 +67,13 @@ - // mir::Constant - // + span: $DIR/simplify-locals-removes-unused-consts.rs:14:18: 14:20 - // + literal: Const { ty: (), val: Value(Scalar()) } +- _5 = const ((), ()); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21 +- // ty::Const +- // + ty: ((), ()) +- // + val: Value(Scalar()) +- // mir::Constant +- // + span: $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21 +- // + literal: Const { ty: ((), ()), val: Value(Scalar()) } - 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 @@ -79,13 +87,15 @@ // + ty: ((), ()) // + val: Value(Scalar()) // 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()) } } 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 @@ -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 @@ -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