Skip to content

Commit 9b29907

Browse files
committed
Maybe fix the regression?
1 parent c4b8c57 commit 9b29907

16 files changed

+130
-172
lines changed

compiler/rustc_mir_transform/src/simplify_pow_of_two.rs

Lines changed: 19 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
44
use crate::MirPass;
55
use rustc_const_eval::interpret::{ConstValue, Scalar};
6-
use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
76
use rustc_middle::mir::patch::MirPatch;
87
use rustc_middle::mir::*;
98
use rustc_middle::ty::{self, Ty, TyCtxt, UintTy};
@@ -33,18 +32,14 @@ impl<'tcx> MirPass<'tcx> for SimplifyPowOfTwo {
3332
&& let Some(def_id) = func.const_fn_def().map(|def| def.0)
3433
&& let def_path = tcx.def_path(def_id)
3534
&& tcx.crate_name(def_path.krate) == sym::core
36-
// FIXME(Centri3): I feel like we should do this differently...
37-
&& let [
38-
DisambiguatedDefPathData { data: DefPathData::TypeNs(sym::num), disambiguator: 0 },
39-
DisambiguatedDefPathData { data: DefPathData::Impl, .. },
40-
DisambiguatedDefPathData { data: DefPathData::ValueNs(sym::pow), .. },
41-
] = &*def_path.data
4235
&& let [recv, exp] = args.as_slice()
4336
&& let Some(recv_const) = recv.constant()
4437
&& let ConstantKind::Val(
4538
ConstValue::Scalar(Scalar::Int(recv_int)),
4639
recv_ty,
4740
) = recv_const.literal
41+
&& recv_ty.is_integral()
42+
&& tcx.item_name(def_id) == sym::pow
4843
&& let Ok(recv_val) = match recv_ty.kind() {
4944
ty::Int(_) => {
5045
let result = recv_int.try_to_int(recv_int.size()).unwrap_or(-1).max(0);
@@ -63,8 +58,8 @@ impl<'tcx> MirPass<'tcx> for SimplifyPowOfTwo {
6358
// `0` would be `1.pow()`, which we shouldn't try to optimize as it's
6459
// already entirely optimized away
6560
&& power_used != 0.0
66-
// Same here
67-
&& recv_val != 0
61+
// `-inf` would be `0.pow()`
62+
&& power_used.is_finite()
6863
{
6964
let power_used = power_used as u32;
7065
let loc = Location { block: i, statement_index: bb.statements.len() };
@@ -110,7 +105,8 @@ impl<'tcx> MirPass<'tcx> for SimplifyPowOfTwo {
110105
);
111106
let shl_result = patch.new_temp(Ty::new_bool(tcx), span);
112107

113-
// Whether the shl will overflow, if so we return 0
108+
// Whether the shl will overflow, if so we return 0. We can do this rather
109+
// than doing a shr because only one bit is set on any power of two
114110
patch.add_assign(
115111
loc,
116112
shl_result.into(),
@@ -130,12 +126,12 @@ impl<'tcx> MirPass<'tcx> for SimplifyPowOfTwo {
130126
),
131127
);
132128

133-
let should_be_zero_bool = patch.new_temp(Ty::new_bool(tcx), span);
134-
let should_be_zero = patch.new_temp(recv_ty, span);
129+
let fine_bool = patch.new_temp(Ty::new_bool(tcx), span);
130+
let fine = patch.new_temp(recv_ty, span);
135131

136132
patch.add_assign(
137133
loc,
138-
should_be_zero_bool.into(),
134+
fine_bool.into(),
139135
Rvalue::BinaryOp(
140136
BinOp::BitOr,
141137
Box::new((
@@ -147,94 +143,47 @@ impl<'tcx> MirPass<'tcx> for SimplifyPowOfTwo {
147143

148144
patch.add_assign(
149145
loc,
150-
should_be_zero.into(),
151-
Rvalue::Cast(
152-
CastKind::IntToInt,
153-
Operand::Copy(should_be_zero_bool.into()),
154-
recv_ty,
155-
),
146+
fine.into(),
147+
Rvalue::Cast(CastKind::IntToInt, Operand::Copy(fine_bool.into()), recv_ty),
156148
);
157149

158-
let shl_exp_ty = patch.new_temp(exp_ty, span);
159150
let shl = patch.new_temp(recv_ty, span);
160151

161152
patch.add_assign(
162153
loc,
163-
shl_exp_ty.into(),
154+
shl.into(),
164155
Rvalue::BinaryOp(
165156
BinOp::Shl,
166157
Box::new((
167158
Operand::Constant(Box::new(Constant {
168159
span,
169160
user_ty: None,
170-
literal: ConstantKind::Val(ConstValue::from_u32(1), exp_ty),
161+
literal: ConstantKind::Val(
162+
ConstValue::Scalar(Scalar::from_uint(1u128, recv_int.size())),
163+
recv_ty,
164+
),
171165
})),
172166
Operand::Copy(num_shl.into()),
173167
)),
174168
),
175169
);
176170

177-
patch.add_assign(
178-
loc,
179-
shl.into(),
180-
Rvalue::Cast(CastKind::IntToInt, Operand::Copy(shl_exp_ty.into()), recv_ty),
181-
);
182-
183171
patch.add_assign(
184172
loc,
185173
*destination,
186174
Rvalue::BinaryOp(
187175
BinOp::MulUnchecked,
188-
Box::new((Operand::Copy(shl.into()), Operand::Copy(should_be_zero.into()))),
176+
Box::new((Operand::Copy(shl.into()), Operand::Copy(fine.into()))),
189177
),
190178
);
191179

192-
// shl doesn't set the overflow flag on x86_64 or even in Rust, so shr to
193-
// see if it overflowed. If it equals 1, it did not, but we also need to
194-
// check `shl_result` to ensure that if this is a multiple of the type's
195-
// size it won't wrap back over to 1
196-
//
197180
// FIXME(Centri3): Do we use `debug_assertions` or `overflow_checks` here?
198181
if tcx.sess.opts.debug_assertions {
199-
let shr = patch.new_temp(recv_ty, span);
200-
let shl_eq_shr = patch.new_temp(Ty::new_bool(tcx), span);
201-
let overflowed = patch.new_temp(Ty::new_bool(tcx), span);
202-
203-
patch.add_assign(
204-
loc,
205-
shr.into(),
206-
Rvalue::BinaryOp(
207-
BinOp::Shr,
208-
Box::new((Operand::Copy(shl.into()), Operand::Copy(num_shl.into()))),
209-
),
210-
);
211-
212-
patch.add_assign(
213-
loc,
214-
shl_eq_shr.into(),
215-
Rvalue::BinaryOp(
216-
BinOp::Eq,
217-
Box::new((Operand::Copy(shl.into()), Operand::Copy(shr.into()))),
218-
),
219-
);
220-
221-
patch.add_assign(
222-
loc,
223-
overflowed.into(),
224-
Rvalue::BinaryOp(
225-
BinOp::BitOr,
226-
Box::new((
227-
Operand::Copy(shl_eq_shr.into()),
228-
Operand::Copy(shl_result.into()),
229-
)),
230-
),
231-
);
232-
233182
patch.patch_terminator(
234183
i,
235184
TerminatorKind::Assert {
236-
cond: Operand::Copy(overflowed.into()),
237-
expected: false,
185+
cond: Operand::Copy(fine_bool.into()),
186+
expected: true,
238187
msg: Box::new(AssertMessage::Overflow(
239188
// For consistency with the previous error message, though
240189
// it's technically incorrect
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// compile-flags: -Copt-level=3 -Cdebug-assertions=true
2+
3+
// CHECK-LABEL: @slow_2_u(
4+
#[no_mangle]
5+
fn slow_2_u(a: u32) -> u32 {
6+
// CHECK: %_3 = icmp ult i32 %a, 32
7+
// CHECK-NEXT: br i1 %_3, label %bb1, label %panic, !prof !{{[0-9]+}}
8+
// CHECK-EMPTY:
9+
// CHECK-NEXT: bb1:
10+
// CHECK-NEXT: %_01 = shl nuw i32 1, %a
11+
// CHECK-NEXT: ret i32 %_0
12+
// CHECK-EMPTY:
13+
// CHECK-NEXT: panic:
14+
2u32.pow(a)
15+
}
16+
17+
fn main() {
18+
slow_2_u(2);
19+
}

tests/codegen/simplify-pow-of-two.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// compile-flags: -Copt-level=3
2+
3+
// CHECK-LABEL: @slow_2_u(
4+
#[no_mangle]
5+
fn slow_2_u(a: u32) -> u32 {
6+
// CHECK: %_3 = icmp ult i32 %a, 32
7+
// CHECK-NEXT: %_5 = zext i1 %_3 to i32
8+
// CHECK-NEXT: %0 = and i32 %a, 31
9+
// CHECK-NEXT: %_01 = shl nuw i32 %_5, %0
10+
// CHECK-NEXT: ret i32 %_01
11+
2u32.pow(a)
12+
}
13+
14+
fn main() {
15+
slow_2_u(2);
16+
}

tests/mir-opt/simplify_pow_of_two_no_overflow_checks.slow_256_i.SimplifyPowOfTwo.diff

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,18 @@
99
+ let mut _4: bool;
1010
+ let mut _5: bool;
1111
+ let mut _6: i32;
12-
+ let mut _7: u32;
13-
+ let mut _8: i32;
12+
+ let mut _7: i32;
1413

1514
bb0: {
1615
StorageLive(_2);
1716
_2 = _1;
18-
- _0 = core::num::<impl i32>::pow(const 256_i32, move _2) -> [return: bb1, unwind continue];
17+
- _0 = core::num::<impl i32>::pow(const 256_i32, move _2) -> [return: bb1, unwind unreachable];
1918
+ _3 = CheckedMul(move _2, const 8_u32);
2019
+ _4 = Lt((_3.0: u32), const 32_u32);
2120
+ _5 = BitOr((_3.1: bool), _4);
2221
+ _6 = _5 as i32 (IntToInt);
23-
+ _7 = Shl(const 1_u32, (_3.0: u32));
24-
+ _8 = _7 as i32 (IntToInt);
25-
+ _0 = MulUnchecked(_8, _6);
22+
+ _7 = Shl(const 1_i32, (_3.0: u32));
23+
+ _0 = MulUnchecked(_7, _6);
2624
+ goto -> bb1;
2725
}
2826

tests/mir-opt/simplify_pow_of_two_no_overflow_checks.slow_256_u.SimplifyPowOfTwo.diff

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,17 @@
1010
+ let mut _5: bool;
1111
+ let mut _6: u32;
1212
+ let mut _7: u32;
13-
+ let mut _8: u32;
1413

1514
bb0: {
1615
StorageLive(_2);
1716
_2 = _1;
18-
- _0 = core::num::<impl u32>::pow(const 256_u32, move _2) -> [return: bb1, unwind continue];
17+
- _0 = core::num::<impl u32>::pow(const 256_u32, move _2) -> [return: bb1, unwind unreachable];
1918
+ _3 = CheckedMul(move _2, const 8_u32);
2019
+ _4 = Lt((_3.0: u32), const 32_u32);
2120
+ _5 = BitOr((_3.1: bool), _4);
2221
+ _6 = _5 as u32 (IntToInt);
2322
+ _7 = Shl(const 1_u32, (_3.0: u32));
24-
+ _8 = _7 as u32 (IntToInt);
25-
+ _0 = MulUnchecked(_8, _6);
23+
+ _0 = MulUnchecked(_7, _6);
2624
+ goto -> bb1;
2725
}
2826

tests/mir-opt/simplify_pow_of_two_no_overflow_checks.slow_2_i.SimplifyPowOfTwo.diff

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,18 @@
99
+ let mut _4: bool;
1010
+ let mut _5: bool;
1111
+ let mut _6: i32;
12-
+ let mut _7: u32;
13-
+ let mut _8: i32;
12+
+ let mut _7: i32;
1413

1514
bb0: {
1615
StorageLive(_2);
1716
_2 = _1;
18-
- _0 = core::num::<impl i32>::pow(const 2_i32, move _2) -> [return: bb1, unwind continue];
17+
- _0 = core::num::<impl i32>::pow(const 2_i32, move _2) -> [return: bb1, unwind unreachable];
1918
+ _3 = CheckedMul(move _2, const 1_u32);
2019
+ _4 = Lt((_3.0: u32), const 32_u32);
2120
+ _5 = BitOr((_3.1: bool), _4);
2221
+ _6 = _5 as i32 (IntToInt);
23-
+ _7 = Shl(const 1_u32, (_3.0: u32));
24-
+ _8 = _7 as i32 (IntToInt);
25-
+ _0 = MulUnchecked(_8, _6);
22+
+ _7 = Shl(const 1_i32, (_3.0: u32));
23+
+ _0 = MulUnchecked(_7, _6);
2624
+ goto -> bb1;
2725
}
2826

tests/mir-opt/simplify_pow_of_two_no_overflow_checks.slow_2_u.SimplifyPowOfTwo.diff

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,17 @@
1010
+ let mut _5: bool;
1111
+ let mut _6: u32;
1212
+ let mut _7: u32;
13-
+ let mut _8: u32;
1413

1514
bb0: {
1615
StorageLive(_2);
1716
_2 = _1;
18-
- _0 = core::num::<impl u32>::pow(const 2_u32, move _2) -> [return: bb1, unwind continue];
17+
- _0 = core::num::<impl u32>::pow(const 2_u32, move _2) -> [return: bb1, unwind unreachable];
1918
+ _3 = CheckedMul(move _2, const 1_u32);
2019
+ _4 = Lt((_3.0: u32), const 32_u32);
2120
+ _5 = BitOr((_3.1: bool), _4);
2221
+ _6 = _5 as u32 (IntToInt);
2322
+ _7 = Shl(const 1_u32, (_3.0: u32));
24-
+ _8 = _7 as u32 (IntToInt);
25-
+ _0 = MulUnchecked(_8, _6);
23+
+ _0 = MulUnchecked(_7, _6);
2624
+ goto -> bb1;
2725
}
2826

tests/mir-opt/simplify_pow_of_two_no_overflow_checks.slow_4_i.SimplifyPowOfTwo.diff

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,18 @@
99
+ let mut _4: bool;
1010
+ let mut _5: bool;
1111
+ let mut _6: i32;
12-
+ let mut _7: u32;
13-
+ let mut _8: i32;
12+
+ let mut _7: i32;
1413

1514
bb0: {
1615
StorageLive(_2);
1716
_2 = _1;
18-
- _0 = core::num::<impl i32>::pow(const 4_i32, move _2) -> [return: bb1, unwind continue];
17+
- _0 = core::num::<impl i32>::pow(const 4_i32, move _2) -> [return: bb1, unwind unreachable];
1918
+ _3 = CheckedMul(move _2, const 2_u32);
2019
+ _4 = Lt((_3.0: u32), const 32_u32);
2120
+ _5 = BitOr((_3.1: bool), _4);
2221
+ _6 = _5 as i32 (IntToInt);
23-
+ _7 = Shl(const 1_u32, (_3.0: u32));
24-
+ _8 = _7 as i32 (IntToInt);
25-
+ _0 = MulUnchecked(_8, _6);
22+
+ _7 = Shl(const 1_i32, (_3.0: u32));
23+
+ _0 = MulUnchecked(_7, _6);
2624
+ goto -> bb1;
2725
}
2826

tests/mir-opt/simplify_pow_of_two_no_overflow_checks.slow_4_u.SimplifyPowOfTwo.diff

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,17 @@
1010
+ let mut _5: bool;
1111
+ let mut _6: u32;
1212
+ let mut _7: u32;
13-
+ let mut _8: u32;
1413

1514
bb0: {
1615
StorageLive(_2);
1716
_2 = _1;
18-
- _0 = core::num::<impl u32>::pow(const 4_u32, move _2) -> [return: bb1, unwind continue];
17+
- _0 = core::num::<impl u32>::pow(const 4_u32, move _2) -> [return: bb1, unwind unreachable];
1918
+ _3 = CheckedMul(move _2, const 2_u32);
2019
+ _4 = Lt((_3.0: u32), const 32_u32);
2120
+ _5 = BitOr((_3.1: bool), _4);
2221
+ _6 = _5 as u32 (IntToInt);
2322
+ _7 = Shl(const 1_u32, (_3.0: u32));
24-
+ _8 = _7 as u32 (IntToInt);
25-
+ _0 = MulUnchecked(_8, _6);
23+
+ _0 = MulUnchecked(_7, _6);
2624
+ goto -> bb1;
2725
}
2826

tests/mir-opt/simplify_pow_of_two_overflow_checks.slow_256_i.SimplifyPowOfTwo.diff

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,27 +9,19 @@
99
+ let mut _4: bool;
1010
+ let mut _5: bool;
1111
+ let mut _6: i32;
12-
+ let mut _7: u32;
13-
+ let mut _8: i32;
14-
+ let mut _9: i32;
15-
+ let mut _10: bool;
16-
+ let mut _11: bool;
12+
+ let mut _7: i32;
1713

1814
bb0: {
1915
StorageLive(_2);
2016
_2 = _1;
21-
- _0 = core::num::<impl i32>::pow(const 256_i32, move _2) -> [return: bb1, unwind continue];
17+
- _0 = core::num::<impl i32>::pow(const 256_i32, move _2) -> [return: bb1, unwind unreachable];
2218
+ _3 = CheckedMul(move _2, const 8_u32);
2319
+ _4 = Lt((_3.0: u32), const 32_u32);
2420
+ _5 = BitOr((_3.1: bool), _4);
2521
+ _6 = _5 as i32 (IntToInt);
26-
+ _7 = Shl(const 1_u32, (_3.0: u32));
27-
+ _8 = _7 as i32 (IntToInt);
28-
+ _0 = MulUnchecked(_8, _6);
29-
+ _9 = Shr(_8, (_3.0: u32));
30-
+ _10 = Eq(_8, _9);
31-
+ _11 = BitOr(_10, _4);
32-
+ assert(!_11, "attempt to compute `{} * {}`, which would overflow", const 1_u32, (_3.0: u32)) -> [success: bb1, unwind continue];
22+
+ _7 = Shl(const 1_i32, (_3.0: u32));
23+
+ _0 = MulUnchecked(_7, _6);
24+
+ assert(_5, "attempt to compute `{} * {}`, which would overflow", const 1_u32, (_3.0: u32)) -> [success: bb1, unwind unreachable];
3325
}
3426

3527
bb1: {

0 commit comments

Comments
 (0)