about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFélix Fischer <felix91gr@gmail.com>2020-04-29 18:59:13 -0400
committerFélix Fischer <felix91gr@gmail.com>2020-05-02 00:40:03 -0400
commitd0dea9f5887c8371279b0b07ce0380f060ca1e99 (patch)
tree8430908727076b9c07bc05d20aadd975e76a6330
parente91aebc1a3835b9b420da0c021e211175a724b8d (diff)
downloadrust-d0dea9f5887c8371279b0b07ce0380f060ca1e99.tar.gz
rust-d0dea9f5887c8371279b0b07ce0380f060ca1e99.zip
Added MIR constant propagation of Scalars into function call arguments
- Documented rationale of current solution
- Polished documentation
-rw-r--r--src/librustc_mir/transform/const_prop.rs56
-rw-r--r--src/test/mir-opt/const_prop/scalar_literal_propagation/rustc.main.ConstProp.diff13
-rw-r--r--src/test/mir-opt/simplify-locals-removes-unused-consts/rustc.main.SimplifyLocals.diff22
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 09d8f89676a..a34b9dac428 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 0183ff7716c..596ddcb4353 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(<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: {
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 0742f655730..0bd4ba97b3c 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(<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
@@ -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
@@ -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