Skip to content

Commit ffcb98e

Browse files
borsbors
committed
Auto merge of #126852 - scottmcm:more-checked-math-tweaks, r=<try>
Also get `add nuw` from `uN::checked_add` When I was doing this for `checked_{sub,shl,shr}`, it was mentioned #124114 (comment) that it'd be worth trying for `checked_add` too. It makes a particularly-big difference for `x.checked_add(C)`, as doing this means that LLVM removes the intrinsic and does it as a normal `x <= MAX - C` instead. cc `@DianQK` who had commented about `checked_add` related to rust-lang/hashbrown#509 before cc llvm/llvm-project#80637 for how LLVM is unlikely to do this itself
2 parents d4cc01c + f5db46c commit ffcb98e

4 files changed

+67
-68
lines changed

‎library/core/src/num/uint_macros.rs

+13-2
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,19 @@ macro_rules! uint_impl {
455455
without modifying the original"]
456456
#[inline]
457457
pub const fn checked_add(self, rhs: Self) -> Option<Self> {
458-
let (a, b) = self.overflowing_add(rhs);
459-
if unlikely!(b) { None } else { Some(a) }
458+
// This used to use `overflowing_add`, but that means it ends up being
459+
// a `wrapping_add`, losing some optimization opportunities. Notably,
460+
// phrasing it this way helps `.checked_add(1)` optimize to a check
461+
// against `MAX` and a `add nuw`.
462+
// Per <https://github.com/rust-lang/rust/pull/124114#issuecomment-2066173305>,
463+
// LLVM is happy to re-form the intrinsic later if useful.
464+
465+
if intrinsics::add_with_overflow(self, rhs).1 {
466+
None
467+
} else {
468+
// SAFETY: Just checked it doesn't overflow
469+
Some(unsafe { intrinsics::unchecked_add(self, rhs) })
470+
}
460471
}
461472

462473
/// Strict integer addition. Computes `self + rhs`, panicking

‎tests/codegen/checked_math.rs

+14
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,17 @@ pub fn checked_shr_signed(a: i32, b: u32) -> Option<i32> {
8484
// CHECK: ret { i32, i32 } %[[R1]]
8585
a.checked_shr(b)
8686
}
87+
88+
// CHECK-LABEL: @checked_add_one_unwrap_unsigned
89+
// CHECK-SAME: (i32 noundef %x)
90+
#[no_mangle]
91+
pub fn checked_add_one_unwrap_unsigned(x: u32) -> u32 {
92+
// CHECK: %[[IS_MAX:.+]] = icmp eq i32 %x, -1
93+
// CHECK: br i1 %[[IS_MAX]], label %[[NONE_BB:.+]], label %[[SOME_BB:.+]]
94+
// CHECK: [[SOME_BB]]:
95+
// CHECK: %[[R:.+]] = add nuw i32 %x, 1
96+
// CHECK: ret i32 %[[R]]
97+
// CHECK: [[NONE_BB]]:
98+
// CHECK: call {{.+}}unwrap_failed
99+
x.checked_add(1).unwrap()
100+
}

‎tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-abort.mir

+20-33
Original file line numberDiff line numberDiff line change
@@ -5,41 +5,34 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
55
debug n => _2;
66
let mut _0: u16;
77
scope 1 (inlined <u16 as Step>::forward) {
8-
let mut _8: u16;
8+
let mut _7: u16;
99
scope 2 {
1010
}
1111
scope 3 (inlined <u16 as Step>::forward_checked) {
1212
scope 4 {
1313
scope 6 (inlined core::num::<impl u16>::checked_add) {
14-
let mut _7: bool;
15-
scope 7 {
16-
}
17-
scope 8 (inlined core::num::<impl u16>::overflowing_add) {
18-
let mut _5: (u16, bool);
19-
let _6: bool;
20-
scope 9 {
21-
}
22-
}
14+
let mut _5: (u16, bool);
15+
let mut _6: bool;
2316
}
2417
}
2518
scope 5 (inlined convert::num::ptr_try_from_impls::<impl TryFrom<usize> for u16>::try_from) {
2619
let mut _3: bool;
2720
let mut _4: u16;
2821
}
2922
}
30-
scope 10 (inlined Option::<u16>::is_none) {
31-
scope 11 (inlined Option::<u16>::is_some) {
23+
scope 7 (inlined Option::<u16>::is_none) {
24+
scope 8 (inlined Option::<u16>::is_some) {
3225
}
3326
}
34-
scope 12 (inlined core::num::<impl u16>::wrapping_add) {
27+
scope 9 (inlined core::num::<impl u16>::wrapping_add) {
3528
}
3629
}
3730

3831
bb0: {
3932
StorageLive(_4);
4033
StorageLive(_3);
4134
_3 = Gt(_2, const 65535_usize);
42-
switchInt(move _3) -> [0: bb1, otherwise: bb5];
35+
switchInt(move _3) -> [0: bb1, otherwise: bb4];
4336
}
4437

4538
bb1: {
@@ -49,41 +42,35 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
4942
StorageLive(_5);
5043
_5 = AddWithOverflow(_1, _4);
5144
_6 = (_5.1: bool);
52-
StorageDead(_5);
53-
StorageLive(_7);
54-
_7 = unlikely(move _6) -> [return: bb2, unwind unreachable];
45+
switchInt(move _6) -> [0: bb2, otherwise: bb3];
5546
}
5647

5748
bb2: {
58-
switchInt(move _7) -> [0: bb3, otherwise: bb4];
49+
StorageDead(_5);
50+
StorageDead(_6);
51+
goto -> bb6;
5952
}
6053

6154
bb3: {
62-
StorageDead(_7);
55+
StorageDead(_5);
6356
StorageDead(_6);
64-
goto -> bb7;
57+
goto -> bb5;
6558
}
6659

6760
bb4: {
68-
StorageDead(_7);
69-
StorageDead(_6);
70-
goto -> bb6;
61+
StorageDead(_3);
62+
goto -> bb5;
7163
}
7264

7365
bb5: {
74-
StorageDead(_3);
75-
goto -> bb6;
66+
assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb6, unwind unreachable];
7667
}
7768

7869
bb6: {
79-
assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb7, unwind unreachable];
80-
}
81-
82-
bb7: {
83-
StorageLive(_8);
84-
_8 = _2 as u16 (IntToInt);
85-
_0 = Add(_1, _8);
86-
StorageDead(_8);
70+
StorageLive(_7);
71+
_7 = _2 as u16 (IntToInt);
72+
_0 = Add(_1, _7);
73+
StorageDead(_7);
8774
StorageDead(_4);
8875
return;
8976
}

‎tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-unwind.mir

+20-33
Original file line numberDiff line numberDiff line change
@@ -5,41 +5,34 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
55
debug n => _2;
66
let mut _0: u16;
77
scope 1 (inlined <u16 as Step>::forward) {
8-
let mut _8: u16;
8+
let mut _7: u16;
99
scope 2 {
1010
}
1111
scope 3 (inlined <u16 as Step>::forward_checked) {
1212
scope 4 {
1313
scope 6 (inlined core::num::<impl u16>::checked_add) {
14-
let mut _7: bool;
15-
scope 7 {
16-
}
17-
scope 8 (inlined core::num::<impl u16>::overflowing_add) {
18-
let mut _5: (u16, bool);
19-
let _6: bool;
20-
scope 9 {
21-
}
22-
}
14+
let mut _5: (u16, bool);
15+
let mut _6: bool;
2316
}
2417
}
2518
scope 5 (inlined convert::num::ptr_try_from_impls::<impl TryFrom<usize> for u16>::try_from) {
2619
let mut _3: bool;
2720
let mut _4: u16;
2821
}
2922
}
30-
scope 10 (inlined Option::<u16>::is_none) {
31-
scope 11 (inlined Option::<u16>::is_some) {
23+
scope 7 (inlined Option::<u16>::is_none) {
24+
scope 8 (inlined Option::<u16>::is_some) {
3225
}
3326
}
34-
scope 12 (inlined core::num::<impl u16>::wrapping_add) {
27+
scope 9 (inlined core::num::<impl u16>::wrapping_add) {
3528
}
3629
}
3730

3831
bb0: {
3932
StorageLive(_4);
4033
StorageLive(_3);
4134
_3 = Gt(_2, const 65535_usize);
42-
switchInt(move _3) -> [0: bb1, otherwise: bb5];
35+
switchInt(move _3) -> [0: bb1, otherwise: bb4];
4336
}
4437

4538
bb1: {
@@ -49,41 +42,35 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
4942
StorageLive(_5);
5043
_5 = AddWithOverflow(_1, _4);
5144
_6 = (_5.1: bool);
52-
StorageDead(_5);
53-
StorageLive(_7);
54-
_7 = unlikely(move _6) -> [return: bb2, unwind unreachable];
45+
switchInt(move _6) -> [0: bb2, otherwise: bb3];
5546
}
5647

5748
bb2: {
58-
switchInt(move _7) -> [0: bb3, otherwise: bb4];
49+
StorageDead(_5);
50+
StorageDead(_6);
51+
goto -> bb6;
5952
}
6053

6154
bb3: {
62-
StorageDead(_7);
55+
StorageDead(_5);
6356
StorageDead(_6);
64-
goto -> bb7;
57+
goto -> bb5;
6558
}
6659

6760
bb4: {
68-
StorageDead(_7);
69-
StorageDead(_6);
70-
goto -> bb6;
61+
StorageDead(_3);
62+
goto -> bb5;
7163
}
7264

7365
bb5: {
74-
StorageDead(_3);
75-
goto -> bb6;
66+
assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb6, unwind continue];
7667
}
7768

7869
bb6: {
79-
assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb7, unwind continue];
80-
}
81-
82-
bb7: {
83-
StorageLive(_8);
84-
_8 = _2 as u16 (IntToInt);
85-
_0 = Add(_1, _8);
86-
StorageDead(_8);
70+
StorageLive(_7);
71+
_7 = _2 as u16 (IntToInt);
72+
_0 = Add(_1, _7);
73+
StorageDead(_7);
8774
StorageDead(_4);
8875
return;
8976
}

0 commit comments

Comments
 (0)