Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit d0dea9f

Browse files
felix91grfelix91gr
committedMay 2, 2020
Added MIR constant propagation of Scalars into function call arguments
- Documented rationale of current solution - Polished documentation
1 parent e91aebc commit d0dea9f

File tree

3 files changed

+81
-10
lines changed

3 files changed

+81
-10
lines changed
 

‎src/librustc_mir/transform/const_prop.rs

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp {
787787
| NonMutatingUse(NonMutatingUseContext::Inspect)
788788
| NonMutatingUse(NonMutatingUseContext::Projection)
789789
| NonUse(_) => {}
790+
// FIXME(felix91gr): explain the reasoning behind this
790791
MutatingUse(MutatingUseContext::Projection) => {
791792
if self.local_kinds[local] != LocalKind::Temp {
792793
self.can_const_prop[local] = ConstPropMode::NoPropagation;
@@ -969,13 +970,58 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
969970
| TerminatorKind::GeneratorDrop
970971
| TerminatorKind::FalseEdges { .. }
971972
| TerminatorKind::FalseUnwind { .. } => {}
972-
//FIXME(wesleywiser) Call does have Operands that could be const-propagated
973-
TerminatorKind::Call { .. } => {}
973+
// Every argument in our function calls can be const propagated.
974+
TerminatorKind::Call { ref mut args, .. } => {
975+
let mir_opt_level = self.tcx.sess.opts.debugging_opts.mir_opt_level;
976+
// Constant Propagation into function call arguments is gated
977+
// under mir-opt-level 2, because LLVM codegen gives performance
978+
// regressions with it.
979+
if mir_opt_level >= 2 {
980+
for opr in args {
981+
/*
982+
The following code would appear to be incomplete, because
983+
the function `Operand::place()` returns `None` if the
984+
`Operand` is of the variant `Operand::Constant`. In this
985+
context however, that variant will never appear. This is why:
986+
987+
When constructing the MIR, all function call arguments are
988+
copied into `Locals` of `LocalKind::Temp`. At least, all arguments
989+
that are not unsized (Less than 0.1% are unsized. See #71170
990+
to learn more about those).
991+
992+
This means that, conversely, all `Operands` found as function call
993+
arguments are of the variant `Operand::Copy`. This allows us to
994+
simplify our handling of `Operands` in this case.
995+
*/
996+
if let Some(l) = opr.place().and_then(|p| p.as_local()) {
997+
if let Some(value) = self.get_const(l) {
998+
if self.should_const_prop(value) {
999+
// FIXME(felix91gr): this code only handles `Scalar` cases.
1000+
// For now, we're not handling `ScalarPair` cases because
1001+
// doing so here would require a lot of code duplication.
1002+
// We should hopefully generalize `Operand` handling into a fn,
1003+
// and use it to do const-prop here and everywhere else
1004+
// where it makes sense.
1005+
if let interpret::Operand::Immediate(
1006+
interpret::Immediate::Scalar(
1007+
interpret::ScalarMaybeUndef::Scalar(scalar),
1008+
),
1009+
) = *value
1010+
{
1011+
*opr = self.operand_from_scalar(
1012+
scalar,
1013+
value.layout.ty,
1014+
source_info.span,
1015+
);
1016+
}
1017+
}
1018+
}
1019+
}
1020+
}
1021+
}
1022+
}
9741023
}
9751024
// We remove all Locals which are restricted in propagation to their containing blocks.
976-
// We wouldn't need to clone, but the borrow checker can't see that we're not aliasing
977-
// the locals_of_current_block field, so we need to clone it first.
978-
// let ecx = &mut self.ecx;
9791025
for local in self.locals_of_current_block.iter() {
9801026
Self::remove_const(&mut self.ecx, local);
9811027
}

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,27 @@
2222
StorageLive(_2); // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
2323
StorageLive(_3); // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
2424
- _3 = _1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
25+
- _2 = const consume(move _3) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
2526
+ _3 = const 1u32; // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
26-
+ // ty::Const
27+
// ty::Const
2728
+ // + ty: u32
2829
+ // + val: Value(Scalar(0x00000001))
2930
+ // mir::Constant
3031
+ // + span: $DIR/scalar_literal_propagation.rs:4:13: 4:14
3132
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000001)) }
32-
_2 = const consume(move _3) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
33-
// ty::Const
33+
+ _2 = const consume(const 1u32) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
34+
+ // ty::Const
3435
// + ty: fn(u32) {consume}
3536
// + val: Value(Scalar(<ZST>))
3637
// mir::Constant
3738
// + span: $DIR/scalar_literal_propagation.rs:4:5: 4:12
3839
// + literal: Const { ty: fn(u32) {consume}, val: Value(Scalar(<ZST>)) }
40+
+ // ty::Const
41+
+ // + ty: u32
42+
+ // + val: Value(Scalar(0x00000001))
43+
+ // mir::Constant
44+
+ // + span: $DIR/scalar_literal_propagation.rs:4:5: 4:15
45+
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000001)) }
3946
}
4047

4148
bb1: {

‎src/test/mir-opt/simplify-locals-removes-unused-consts/rustc.main.SimplifyLocals.diff

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
- StorageDead(_2); // scope 0 at $DIR/simplify-locals-removes-unused-consts.rs:13:27: 13:28
5151
- StorageDead(_1); // scope 0 at $DIR/simplify-locals-removes-unused-consts.rs:13:28: 13:29
5252
- StorageLive(_4); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:5: 14:22
53+
- StorageLive(_5); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21
5354
- StorageLive(_6); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:14: 14:16
5455
- _6 = const (); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:14: 14:16
5556
- // ty::Const
@@ -66,6 +67,13 @@
6667
- // mir::Constant
6768
- // + span: $DIR/simplify-locals-removes-unused-consts.rs:14:18: 14:20
6869
- // + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
70+
- _5 = const ((), ()); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21
71+
- // ty::Const
72+
- // + ty: ((), ())
73+
- // + val: Value(Scalar(<ZST>))
74+
- // mir::Constant
75+
- // + span: $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21
76+
- // + literal: Const { ty: ((), ()), val: Value(Scalar(<ZST>)) }
6977
- StorageDead(_7); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:20: 14:21
7078
- StorageDead(_6); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:20: 14:21
7179
- _4 = const use_zst(const ((), ())) -> bb1; // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:5: 14:22
@@ -79,13 +87,15 @@
7987
// + ty: ((), ())
8088
// + val: Value(Scalar(<ZST>))
8189
// mir::Constant
82-
// + span: $DIR/simplify-locals-removes-unused-consts.rs:14:13: 14:21
90+
// + span: $DIR/simplify-locals-removes-unused-consts.rs:14:5: 14:22
8391
// + literal: Const { ty: ((), ()), val: Value(Scalar(<ZST>)) }
8492
}
8593

8694
bb1: {
95+
- StorageDead(_5); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:21: 14:22
8796
- StorageDead(_4); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:14:22: 14:23
8897
- StorageLive(_8); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:5: 16:35
98+
- StorageLive(_9); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:34
8999
- StorageLive(_10); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:30
90100
- StorageLive(_11); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:28
91101
- _11 = const Temp { x: 40u8 }; // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:28
@@ -105,6 +115,13 @@
105115
- // mir::Constant
106116
- // + span: $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:30
107117
- // + literal: Const { ty: u8, val: Value(Scalar(0x28)) }
118+
- _9 = const 42u8; // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:34
119+
- // ty::Const
120+
- // + ty: u8
121+
- // + val: Value(Scalar(0x2a))
122+
- // mir::Constant
123+
- // + span: $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:34
124+
- // + literal: Const { ty: u8, val: Value(Scalar(0x2a)) }
108125
- StorageDead(_10); // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:33: 16:34
109126
- _8 = const use_u8(const 42u8) -> bb2; // scope 1 at $DIR/simplify-locals-removes-unused-consts.rs:16:5: 16:35
110127
- // ty::Const
@@ -117,11 +134,12 @@
117134
// + ty: u8
118135
// + val: Value(Scalar(0x2a))
119136
// mir::Constant
120-
// + span: $DIR/simplify-locals-removes-unused-consts.rs:16:12: 16:34
137+
// + span: $DIR/simplify-locals-removes-unused-consts.rs:16:5: 16:35
121138
// + literal: Const { ty: u8, val: Value(Scalar(0x2a)) }
122139
}
123140

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

0 commit comments

Comments
 (0)
Please sign in to comment.