Skip to content

Commit 8d1fa47

Browse files
borsbors
committed
Auto merge of #112724 - scottmcm:simpler-unchecked-shifts, r=Mark-Simulacrum
[libs] Simplify `unchecked_{shl,shr}` There's no need for the `const_eval_select` dance here. And while I originally wrote the `.try_into().unwrap_unchecked()` implementation here, it's kinda a mess in MIR -- this new one is substantially simpler, as shown by the old one being above the inlining threshold but the new one being below it in the `mir-opt/inline/unchecked_shifts` tests. We don't need `u32::checked_shl` doing a dance through both `Result` *and* `Option` 🙃
2 parents c911e08 + 3ec4eed commit 8d1fa47

20 files changed

+449
-99
lines changed

Diff for: ‎library/core/src/num/mod.rs

+5-12
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
#![stable(feature = "rust1", since = "1.0.0")]
44

55
use crate::ascii;
6-
use crate::convert::TryInto;
76
use crate::intrinsics;
87
use crate::mem;
98
use crate::ops::{Add, Mul, Sub};
@@ -278,18 +277,12 @@ macro_rules! widening_impl {
278277

279278
macro_rules! conv_rhs_for_unchecked_shift {
280279
($SelfT:ty, $x:expr) => {{
281-
#[inline]
282-
fn conv(x: u32) -> $SelfT {
283-
// FIXME(const-hack) replace with `.try_into().ok().unwrap_unchecked()`.
284-
// SAFETY: Any legal shift amount must be losslessly representable in the self type.
285-
unsafe { x.try_into().ok().unwrap_unchecked() }
286-
}
287-
#[inline]
288-
const fn const_conv(x: u32) -> $SelfT {
289-
x as _
280+
// If the `as` cast will truncate, ensure we still tell the backend
281+
// that the pre-truncation value was also small.
282+
if <$SelfT>::BITS < 32 {
283+
intrinsics::assume($x <= (<$SelfT>::MAX as u32));
290284
}
291-
292-
intrinsics::const_eval_select(($x,), const_conv, conv)
285+
$x as $SelfT
293286
}};
294287
}
295288

Diff for: ‎tests/codegen/unchecked_shifts.rs

+4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// CHECK-LABEL: @unchecked_shl_unsigned_same
99
#[no_mangle]
1010
pub unsafe fn unchecked_shl_unsigned_same(a: u32, b: u32) -> u32 {
11+
// CHECK-NOT: assume
1112
// CHECK-NOT: and i32
1213
// CHECK: shl i32 %a, %b
1314
// CHECK-NOT: and i32
@@ -30,6 +31,7 @@ pub unsafe fn unchecked_shl_unsigned_smaller(a: u16, b: u32) -> u16 {
3031
// CHECK-LABEL: @unchecked_shl_unsigned_bigger
3132
#[no_mangle]
3233
pub unsafe fn unchecked_shl_unsigned_bigger(a: u64, b: u32) -> u64 {
34+
// CHECK-NOT: assume
3335
// CHECK: %[[EXT:.+]] = zext i32 %b to i64
3436
// CHECK: shl i64 %a, %[[EXT]]
3537
a.unchecked_shl(b)
@@ -38,6 +40,7 @@ pub unsafe fn unchecked_shl_unsigned_bigger(a: u64, b: u32) -> u64 {
3840
// CHECK-LABEL: @unchecked_shr_signed_same
3941
#[no_mangle]
4042
pub unsafe fn unchecked_shr_signed_same(a: i32, b: u32) -> i32 {
43+
// CHECK-NOT: assume
4144
// CHECK-NOT: and i32
4245
// CHECK: ashr i32 %a, %b
4346
// CHECK-NOT: and i32
@@ -60,6 +63,7 @@ pub unsafe fn unchecked_shr_signed_smaller(a: i16, b: u32) -> i16 {
6063
// CHECK-LABEL: @unchecked_shr_signed_bigger
6164
#[no_mangle]
6265
pub unsafe fn unchecked_shr_signed_bigger(a: i64, b: u32) -> i64 {
66+
// CHECK-NOT: assume
6367
// CHECK: %[[EXT:.+]] = zext i32 %b to i64
6468
// CHECK: ashr i64 %a, %[[EXT]]
6569
a.unchecked_shr(b)

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.rs

+12
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,15 @@ pub unsafe fn unchecked_shl_unsigned_smaller(a: u16, b: u32) -> u16 {
1616
pub unsafe fn unchecked_shr_signed_smaller(a: i16, b: u32) -> i16 {
1717
a.unchecked_shr(b)
1818
}
19+
20+
// EMIT_MIR unchecked_shifts.unchecked_shl_unsigned_bigger.Inline.diff
21+
// EMIT_MIR unchecked_shifts.unchecked_shl_unsigned_bigger.PreCodegen.after.mir
22+
pub unsafe fn unchecked_shl_unsigned_bigger(a: u64, b: u32) -> u64 {
23+
a.unchecked_shl(b)
24+
}
25+
26+
// EMIT_MIR unchecked_shifts.unchecked_shr_signed_bigger.Inline.diff
27+
// EMIT_MIR unchecked_shifts.unchecked_shr_signed_bigger.PreCodegen.after.mir
28+
pub unsafe fn unchecked_shr_signed_bigger(a: i64, b: u32) -> i64 {
29+
a.unchecked_shr(b)
30+
}

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.Inline.panic-abort.diff

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
- // MIR for `unchecked_shl_unsigned_bigger` before Inline
2+
+ // MIR for `unchecked_shl_unsigned_bigger` after Inline
3+
4+
fn unchecked_shl_unsigned_bigger(_1: u64, _2: u32) -> u64 {
5+
debug a => _1;
6+
debug b => _2;
7+
let mut _0: u64;
8+
let mut _3: u64;
9+
let mut _4: u32;
10+
+ scope 1 (inlined core::num::<impl u64>::unchecked_shl) {
11+
+ debug self => _3;
12+
+ debug rhs => _4;
13+
+ let mut _5: u64;
14+
+ scope 2 {
15+
+ }
16+
+ }
17+
18+
bb0: {
19+
StorageLive(_3);
20+
_3 = _1;
21+
StorageLive(_4);
22+
_4 = _2;
23+
- _0 = core::num::<impl u64>::unchecked_shl(move _3, move _4) -> [return: bb1, unwind unreachable];
24+
+ StorageLive(_5);
25+
+ _5 = _4 as u64 (IntToInt);
26+
+ _0 = unchecked_shl::<u64>(_3, move _5) -> [return: bb1, unwind unreachable];
27+
}
28+
29+
bb1: {
30+
+ StorageDead(_5);
31+
StorageDead(_4);
32+
StorageDead(_3);
33+
return;
34+
}
35+
}
36+

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.Inline.panic-unwind.diff

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
- // MIR for `unchecked_shl_unsigned_bigger` before Inline
2+
+ // MIR for `unchecked_shl_unsigned_bigger` after Inline
3+
4+
fn unchecked_shl_unsigned_bigger(_1: u64, _2: u32) -> u64 {
5+
debug a => _1;
6+
debug b => _2;
7+
let mut _0: u64;
8+
let mut _3: u64;
9+
let mut _4: u32;
10+
+ scope 1 (inlined core::num::<impl u64>::unchecked_shl) {
11+
+ debug self => _3;
12+
+ debug rhs => _4;
13+
+ let mut _5: u64;
14+
+ scope 2 {
15+
+ }
16+
+ }
17+
18+
bb0: {
19+
StorageLive(_3);
20+
_3 = _1;
21+
StorageLive(_4);
22+
_4 = _2;
23+
- _0 = core::num::<impl u64>::unchecked_shl(move _3, move _4) -> bb1;
24+
+ StorageLive(_5);
25+
+ _5 = _4 as u64 (IntToInt);
26+
+ _0 = unchecked_shl::<u64>(_3, move _5) -> [return: bb1, unwind unreachable];
27+
}
28+
29+
bb1: {
30+
+ StorageDead(_5);
31+
StorageDead(_4);
32+
StorageDead(_3);
33+
return;
34+
}
35+
}
36+

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.PreCodegen.after.panic-abort.mir

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// MIR for `unchecked_shl_unsigned_bigger` after PreCodegen
2+
3+
fn unchecked_shl_unsigned_bigger(_1: u64, _2: u32) -> u64 {
4+
debug a => _1;
5+
debug b => _2;
6+
let mut _0: u64;
7+
scope 1 (inlined core::num::<impl u64>::unchecked_shl) {
8+
debug self => _1;
9+
debug rhs => _2;
10+
let mut _3: u64;
11+
scope 2 {
12+
}
13+
}
14+
15+
bb0: {
16+
StorageLive(_3);
17+
_3 = _2 as u64 (IntToInt);
18+
_0 = unchecked_shl::<u64>(_1, move _3) -> [return: bb1, unwind unreachable];
19+
}
20+
21+
bb1: {
22+
StorageDead(_3);
23+
return;
24+
}
25+
}

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_bigger.PreCodegen.after.panic-unwind.mir

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// MIR for `unchecked_shl_unsigned_bigger` after PreCodegen
2+
3+
fn unchecked_shl_unsigned_bigger(_1: u64, _2: u32) -> u64 {
4+
debug a => _1;
5+
debug b => _2;
6+
let mut _0: u64;
7+
scope 1 (inlined core::num::<impl u64>::unchecked_shl) {
8+
debug self => _1;
9+
debug rhs => _2;
10+
let mut _3: u64;
11+
scope 2 {
12+
}
13+
}
14+
15+
bb0: {
16+
StorageLive(_3);
17+
_3 = _2 as u64 (IntToInt);
18+
_0 = unchecked_shl::<u64>(_1, move _3) -> [return: bb1, unwind unreachable];
19+
}
20+
21+
bb1: {
22+
StorageDead(_3);
23+
return;
24+
}
25+
}

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.panic-abort.diff

+21-1
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,36 @@
77
let mut _0: u16;
88
let mut _3: u16;
99
let mut _4: u32;
10+
+ scope 1 (inlined core::num::<impl u16>::unchecked_shl) {
11+
+ debug self => _3;
12+
+ debug rhs => _4;
13+
+ let mut _5: u16;
14+
+ let mut _6: bool;
15+
+ let mut _7: u32;
16+
+ scope 2 {
17+
+ }
18+
+ }
1019

1120
bb0: {
1221
StorageLive(_3);
1322
_3 = _1;
1423
StorageLive(_4);
1524
_4 = _2;
16-
_0 = core::num::<impl u16>::unchecked_shl(move _3, move _4) -> [return: bb1, unwind unreachable];
25+
- _0 = core::num::<impl u16>::unchecked_shl(move _3, move _4) -> [return: bb1, unwind unreachable];
26+
+ StorageLive(_5);
27+
+ StorageLive(_6);
28+
+ StorageLive(_7);
29+
+ _7 = const 65535_u32;
30+
+ _6 = Le(_4, move _7);
31+
+ StorageDead(_7);
32+
+ assume(move _6);
33+
+ StorageDead(_6);
34+
+ _5 = _4 as u16 (IntToInt);
35+
+ _0 = unchecked_shl::<u16>(_3, move _5) -> [return: bb1, unwind unreachable];
1736
}
1837

1938
bb1: {
39+
+ StorageDead(_5);
2040
StorageDead(_4);
2141
StorageDead(_3);
2242
return;

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.panic-unwind.diff

+21-1
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,36 @@
77
let mut _0: u16;
88
let mut _3: u16;
99
let mut _4: u32;
10+
+ scope 1 (inlined core::num::<impl u16>::unchecked_shl) {
11+
+ debug self => _3;
12+
+ debug rhs => _4;
13+
+ let mut _5: u16;
14+
+ let mut _6: bool;
15+
+ let mut _7: u32;
16+
+ scope 2 {
17+
+ }
18+
+ }
1019

1120
bb0: {
1221
StorageLive(_3);
1322
_3 = _1;
1423
StorageLive(_4);
1524
_4 = _2;
16-
_0 = core::num::<impl u16>::unchecked_shl(move _3, move _4) -> bb1;
25+
- _0 = core::num::<impl u16>::unchecked_shl(move _3, move _4) -> bb1;
26+
+ StorageLive(_5);
27+
+ StorageLive(_6);
28+
+ StorageLive(_7);
29+
+ _7 = const 65535_u32;
30+
+ _6 = Le(_4, move _7);
31+
+ StorageDead(_7);
32+
+ assume(move _6);
33+
+ StorageDead(_6);
34+
+ _5 = _4 as u16 (IntToInt);
35+
+ _0 = unchecked_shl::<u16>(_3, move _5) -> [return: bb1, unwind unreachable];
1736
}
1837

1938
bb1: {
39+
+ StorageDead(_5);
2040
StorageDead(_4);
2141
StorageDead(_3);
2242
return;

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.PreCodegen.after.panic-abort.mir

+20-1
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,31 @@ fn unchecked_shl_unsigned_smaller(_1: u16, _2: u32) -> u16 {
44
debug a => _1;
55
debug b => _2;
66
let mut _0: u16;
7+
scope 1 (inlined core::num::<impl u16>::unchecked_shl) {
8+
debug self => _1;
9+
debug rhs => _2;
10+
let mut _3: u32;
11+
let mut _4: bool;
12+
let mut _5: u16;
13+
scope 2 {
14+
}
15+
}
716

817
bb0: {
9-
_0 = core::num::<impl u16>::unchecked_shl(_1, _2) -> [return: bb1, unwind unreachable];
18+
StorageLive(_5);
19+
StorageLive(_4);
20+
StorageLive(_3);
21+
_3 = const 65535_u32;
22+
_4 = Le(_2, move _3);
23+
StorageDead(_3);
24+
assume(move _4);
25+
StorageDead(_4);
26+
_5 = _2 as u16 (IntToInt);
27+
_0 = unchecked_shl::<u16>(_1, move _5) -> [return: bb1, unwind unreachable];
1028
}
1129

1230
bb1: {
31+
StorageDead(_5);
1332
return;
1433
}
1534
}

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.PreCodegen.after.panic-unwind.mir

+20-1
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,31 @@ fn unchecked_shl_unsigned_smaller(_1: u16, _2: u32) -> u16 {
44
debug a => _1;
55
debug b => _2;
66
let mut _0: u16;
7+
scope 1 (inlined core::num::<impl u16>::unchecked_shl) {
8+
debug self => _1;
9+
debug rhs => _2;
10+
let mut _3: u32;
11+
let mut _4: bool;
12+
let mut _5: u16;
13+
scope 2 {
14+
}
15+
}
716

817
bb0: {
9-
_0 = core::num::<impl u16>::unchecked_shl(_1, _2) -> bb1;
18+
StorageLive(_5);
19+
StorageLive(_4);
20+
StorageLive(_3);
21+
_3 = const 65535_u32;
22+
_4 = Le(_2, move _3);
23+
StorageDead(_3);
24+
assume(move _4);
25+
StorageDead(_4);
26+
_5 = _2 as u16 (IntToInt);
27+
_0 = unchecked_shl::<u16>(_1, move _5) -> [return: bb1, unwind unreachable];
1028
}
1129

1230
bb1: {
31+
StorageDead(_5);
1332
return;
1433
}
1534
}

Diff for: ‎tests/mir-opt/inline/unchecked_shifts.unchecked_shr_signed_bigger.Inline.panic-abort.diff

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
- // MIR for `unchecked_shr_signed_bigger` before Inline
2+
+ // MIR for `unchecked_shr_signed_bigger` after Inline
3+
4+
fn unchecked_shr_signed_bigger(_1: i64, _2: u32) -> i64 {
5+
debug a => _1;
6+
debug b => _2;
7+
let mut _0: i64;
8+
let mut _3: i64;
9+
let mut _4: u32;
10+
+ scope 1 (inlined core::num::<impl i64>::unchecked_shr) {
11+
+ debug self => _3;
12+
+ debug rhs => _4;
13+
+ let mut _5: i64;
14+
+ scope 2 {
15+
+ }
16+
+ }
17+
18+
bb0: {
19+
StorageLive(_3);
20+
_3 = _1;
21+
StorageLive(_4);
22+
_4 = _2;
23+
- _0 = core::num::<impl i64>::unchecked_shr(move _3, move _4) -> [return: bb1, unwind unreachable];
24+
+ StorageLive(_5);
25+
+ _5 = _4 as i64 (IntToInt);
26+
+ _0 = unchecked_shr::<i64>(_3, move _5) -> [return: bb1, unwind unreachable];
27+
}
28+
29+
bb1: {
30+
+ StorageDead(_5);
31+
StorageDead(_4);
32+
StorageDead(_3);
33+
return;
34+
}
35+
}
36+

0 commit comments

Comments
 (0)