Skip to content

Commit d8cf749

Browse files
committed
Auto merge of rust-lang#74507 - lcnr:const-prop-into-op, r=oli-obk
add `visit_operand` to const prop r? @oli-obk
2 parents fe07ece + 61a9ab8 commit d8cf749

File tree

16 files changed

+261
-89
lines changed

16 files changed

+261
-89
lines changed

src/librustc_mir/transform/const_prop.rs

+50-62
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,34 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
581581
Some(())
582582
}
583583

584+
fn propagate_operand(&mut self, operand: &mut Operand<'tcx>) {
585+
match *operand {
586+
Operand::Copy(l) | Operand::Move(l) => {
587+
if let Some(value) = self.get_const(l) {
588+
if self.should_const_prop(value) {
589+
// FIXME(felix91gr): this code only handles `Scalar` cases.
590+
// For now, we're not handling `ScalarPair` cases because
591+
// doing so here would require a lot of code duplication.
592+
// We should hopefully generalize `Operand` handling into a fn,
593+
// and use it to do const-prop here and everywhere else
594+
// where it makes sense.
595+
if let interpret::Operand::Immediate(interpret::Immediate::Scalar(
596+
ScalarMaybeUninit::Scalar(scalar),
597+
)) = *value
598+
{
599+
*operand = self.operand_from_scalar(
600+
scalar,
601+
value.layout.ty,
602+
self.source_info.unwrap().span,
603+
);
604+
}
605+
}
606+
}
607+
}
608+
Operand::Constant(_) => (),
609+
}
610+
}
611+
584612
fn const_prop(
585613
&mut self,
586614
rvalue: &Rvalue<'tcx>,
@@ -969,6 +997,16 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
969997
}
970998
}
971999

1000+
fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) {
1001+
self.super_operand(operand, location);
1002+
1003+
// Only const prop copies and moves on `mir_opt_level=3` as doing so
1004+
// currently increases compile time.
1005+
if self.tcx.sess.opts.debugging_opts.mir_opt_level >= 3 {
1006+
self.propagate_operand(operand)
1007+
}
1008+
}
1009+
9721010
fn visit_constant(&mut self, constant: &mut Constant<'tcx>, location: Location) {
9731011
trace!("visit_constant: {:?}", constant);
9741012
self.super_constant(constant, location);
@@ -1136,18 +1174,13 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
11361174
}
11371175
}
11381176
}
1139-
TerminatorKind::SwitchInt { ref mut discr, switch_ty, .. } => {
1140-
if let Some(value) = self.eval_operand(&discr, source_info) {
1141-
if self.should_const_prop(value) {
1142-
if let ScalarMaybeUninit::Scalar(scalar) =
1143-
self.ecx.read_scalar(value).unwrap()
1144-
{
1145-
*discr = self.operand_from_scalar(scalar, switch_ty, source_info.span);
1146-
}
1147-
}
1148-
}
1177+
TerminatorKind::SwitchInt { ref mut discr, .. } => {
1178+
// FIXME: This is currently redundant with `visit_operand`, but sadly
1179+
// always visiting operands currently causes a perf regression in LLVM codegen, so
1180+
// `visit_operand` currently only runs for propagates places for `mir_opt_level=3`.
1181+
self.propagate_operand(discr)
11491182
}
1150-
// None of these have Operands to const-propagate
1183+
// None of these have Operands to const-propagate.
11511184
TerminatorKind::Goto { .. }
11521185
| TerminatorKind::Resume
11531186
| TerminatorKind::Abort
@@ -1160,61 +1193,16 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
11601193
| TerminatorKind::FalseEdge { .. }
11611194
| TerminatorKind::FalseUnwind { .. }
11621195
| TerminatorKind::InlineAsm { .. } => {}
1163-
// Every argument in our function calls can be const propagated.
1164-
TerminatorKind::Call { ref mut args, .. } => {
1165-
let mir_opt_level = self.tcx.sess.opts.debugging_opts.mir_opt_level;
1166-
// Constant Propagation into function call arguments is gated
1167-
// under mir-opt-level 2, because LLVM codegen gives performance
1168-
// regressions with it.
1169-
if mir_opt_level >= 2 {
1170-
for opr in args {
1171-
/*
1172-
The following code would appear to be incomplete, because
1173-
the function `Operand::place()` returns `None` if the
1174-
`Operand` is of the variant `Operand::Constant`. In this
1175-
context however, that variant will never appear. This is why:
1176-
1177-
When constructing the MIR, all function call arguments are
1178-
copied into `Locals` of `LocalKind::Temp`. At least, all arguments
1179-
that are not unsized (Less than 0.1% are unsized. See #71170
1180-
to learn more about those).
1181-
1182-
This means that, conversely, all `Operands` found as function call
1183-
arguments are of the variant `Operand::Copy`. This allows us to
1184-
simplify our handling of `Operands` in this case.
1185-
*/
1186-
if let Some(l) = opr.place() {
1187-
if let Some(value) = self.get_const(l) {
1188-
if self.should_const_prop(value) {
1189-
// FIXME(felix91gr): this code only handles `Scalar` cases.
1190-
// For now, we're not handling `ScalarPair` cases because
1191-
// doing so here would require a lot of code duplication.
1192-
// We should hopefully generalize `Operand` handling into a fn,
1193-
// and use it to do const-prop here and everywhere else
1194-
// where it makes sense.
1195-
if let interpret::Operand::Immediate(
1196-
interpret::Immediate::Scalar(ScalarMaybeUninit::Scalar(
1197-
scalar,
1198-
)),
1199-
) = *value
1200-
{
1201-
*opr = self.operand_from_scalar(
1202-
scalar,
1203-
value.layout.ty,
1204-
source_info.span,
1205-
);
1206-
}
1207-
}
1208-
}
1209-
}
1210-
}
1211-
}
1212-
}
1196+
// Every argument in our function calls have already been propagated in `visit_operand`.
1197+
//
1198+
// NOTE: because LLVM codegen gives performance regressions with it, so this is gated
1199+
// on `mir_opt_level=3`.
1200+
TerminatorKind::Call { .. } => {}
12131201
}
12141202

12151203
// We remove all Locals which are restricted in propagation to their containing blocks and
12161204
// which were modified in the current block.
1217-
// Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`
1205+
// Take it out of the ecx so we can get a mutable reference to the ecx for `remove_const`.
12181206
let mut locals = std::mem::take(&mut self.ecx.machine.written_only_inside_own_block_locals);
12191207
for &local in locals.iter() {
12201208
Self::remove_const(&mut self.ecx, local);

src/test/mir-opt/const_prop/array_index/32bit/rustc.main.ConstProp.diff

+13-1
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,25 @@
6464
+ // mir::Constant
6565
+ // + span: $DIR/array_index.rs:5:18: 5:33
6666
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
67-
+ 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
67+
+ 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
6868
+ // ty::Const
6969
+ // + ty: bool
7070
+ // + val: Value(Scalar(0x01))
7171
+ // mir::Constant
7272
+ // + span: $DIR/array_index.rs:5:18: 5:33
7373
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
74+
+ // ty::Const
75+
+ // + ty: usize
76+
+ // + val: Value(Scalar(0x00000004))
77+
+ // mir::Constant
78+
+ // + span: $DIR/array_index.rs:5:18: 5:33
79+
+ // + literal: Const { ty: usize, val: Value(Scalar(0x00000004)) }
80+
+ // ty::Const
81+
+ // + ty: usize
82+
+ // + val: Value(Scalar(0x00000002))
83+
+ // mir::Constant
84+
+ // + span: $DIR/array_index.rs:5:18: 5:33
85+
+ // + literal: Const { ty: usize, val: Value(Scalar(0x00000002)) }
7486
}
7587

7688
bb1: {

src/test/mir-opt/const_prop/array_index/64bit/rustc.main.ConstProp.diff

+13-1
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,25 @@
6464
+ // mir::Constant
6565
+ // + span: $DIR/array_index.rs:5:18: 5:33
6666
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
67-
+ 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
67+
+ 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
6868
+ // ty::Const
6969
+ // + ty: bool
7070
+ // + val: Value(Scalar(0x01))
7171
+ // mir::Constant
7272
+ // + span: $DIR/array_index.rs:5:18: 5:33
7373
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
74+
+ // ty::Const
75+
+ // + ty: usize
76+
+ // + val: Value(Scalar(0x0000000000000004))
77+
+ // mir::Constant
78+
+ // + span: $DIR/array_index.rs:5:18: 5:33
79+
+ // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000004)) }
80+
+ // ty::Const
81+
+ // + ty: usize
82+
+ // + val: Value(Scalar(0x0000000000000002))
83+
+ // mir::Constant
84+
+ // + span: $DIR/array_index.rs:5:18: 5:33
85+
+ // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000002)) }
7486
}
7587

7688
bb1: {

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

+26-6
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,22 @@
3838
- // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
3939
+ // + span: $DIR/bad_op_div_by_zero.rs:5:18: 5:19
4040
// + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
41+
- 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
4142
+ _4 = const true; // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
43+
// ty::Const
44+
+ // + ty: bool
45+
+ // + val: Value(Scalar(0x01))
46+
+ // mir::Constant
47+
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
48+
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
49+
+ 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
4250
+ // ty::Const
4351
+ // + ty: bool
4452
+ // + val: Value(Scalar(0x01))
4553
+ // mir::Constant
4654
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
4755
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
48-
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
49-
// ty::Const
56+
+ // ty::Const
5057
// + ty: i32
5158
// + val: Value(Scalar(0x00000001))
5259
// mir::Constant
@@ -90,29 +97,42 @@
9097
- _7 = BitAnd(move _5, move _6); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
9198
- 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
9299
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
93-
+ 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
94-
+ // ty::Const
100+
+ 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
101+
// ty::Const
95102
+ // + ty: bool
96103
+ // + val: Value(Scalar(0x00))
97104
+ // mir::Constant
98105
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
99106
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
100-
// ty::Const
107+
+ // ty::Const
101108
// + ty: i32
102109
// + val: Value(Scalar(0x00000001))
103110
// mir::Constant
104111
// + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:15
105112
// + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) }
113+
+ // ty::Const
114+
+ // + ty: i32
115+
+ // + val: Value(Scalar(0x00000000))
116+
+ // mir::Constant
117+
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
118+
+ // + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
106119
}
107120

108121
bb2: {
109-
_2 = Div(const 1_i32, move _3); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
122+
- _2 = Div(const 1_i32, move _3); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
123+
+ _2 = Div(const 1_i32, const 0_i32); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
110124
// ty::Const
111125
// + ty: i32
112126
// + val: Value(Scalar(0x00000001))
113127
// mir::Constant
114128
// + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:15
115129
// + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) }
130+
+ // ty::Const
131+
+ // + ty: i32
132+
+ // + val: Value(Scalar(0x00000000))
133+
+ // mir::Constant
134+
+ // + span: $DIR/bad_op_div_by_zero.rs:5:14: 5:19
135+
+ // + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
116136
StorageDead(_3); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:18: 5:19
117137
_0 = const (); // scope 0 at $DIR/bad_op_div_by_zero.rs:3:11: 6:2
118138
// ty::Const

0 commit comments

Comments
 (0)