Skip to content

Commit

Permalink
Auto merge of rust-lang#126852 - scottmcm:more-checked-math-tweaks, r…
Browse files Browse the repository at this point in the history
…=Amanieu

Also get `add nuw` from `uN::checked_add`

When I was doing this for `checked_{sub,shl,shr}`, it was mentioned rust-lang#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
  • Loading branch information
bors committed Jun 25, 2024
2 parents 1bd207e + e4bc79d commit a451b2a
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions core/src/num/uint_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +454,19 @@ macro_rules! uint_impl {
without modifying the original"]
#[inline]
pub const fn checked_add(self, rhs: Self) -> Option<Self> {
let (a, b) = self.overflowing_add(rhs);
if unlikely!(b) { None } else { Some(a) }
// This used to use `overflowing_add`, but that means it ends up being
// a `wrapping_add`, losing some optimization opportunities. Notably,
// phrasing it this way helps `.checked_add(1)` optimize to a check
// against `MAX` and a `add nuw`.
// Per <https://github.com/rust-lang/rust/pull/124114#issuecomment-2066173305>,
// LLVM is happy to re-form the intrinsic later if useful.

if unlikely!(intrinsics::add_with_overflow(self, rhs).1) {
None
} else {
// SAFETY: Just checked it doesn't overflow
Some(unsafe { intrinsics::unchecked_add(self, rhs) })
}
}

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

0 comments on commit a451b2a

Please sign in to comment.