mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-22 23:04:33 +00:00
Rollup merge of #125038 - ivan-shrimp:checked_sub, r=joboet
Invert comparison in `uN::checked_sub` After #124114, LLVM no longer combines the comparison and subtraction in `uN::checked_sub` when either operand is a constant (demo: https://rust.godbolt.org/z/MaeoYbsP1). The difference is more pronounced when the expression is slightly more complex (https://rust.godbolt.org/z/4rPavsYdc). This is due to the use of `>=` here:ee97564e3a/library/core/src/num/uint_macros.rs (L581-L593)
For constant `C`, LLVM eagerly converts `a >= C` into `a > C - 1`, but the backend can only combine `a < C` with `a - C`, not `C - 1 < a` and `a - C`:e586556e37/llvm/lib/CodeGen/CodeGenPrepare.cpp (L1697-L1742)
This PR[^1] simply inverts the `>=` into `<` to restore the LLVM magic, and somewhat align this with the implementation of `uN::overflowing_sub` from #103299. When the result is stored as an `Option` (rather than being branched/cmoved on), the discriminant is `self >= rhs`. This PR doesn't affect the codegen (and relevant tests) of that since LLVM will negate `self < rhs` to `self >= rhs` when necessary. [^1]: Note to `self`: My very first contribution to publicly-used code. Hopefully like what I should learn to always be, tiny and humble.
This commit is contained in:
commit
4f7d9d4ad8
@ -584,11 +584,11 @@ macro_rules! uint_impl {
|
|||||||
// Thus, rather than using `overflowing_sub` that produces a wrapping
|
// Thus, rather than using `overflowing_sub` that produces a wrapping
|
||||||
// subtraction, check it ourself so we can use an unchecked one.
|
// subtraction, check it ourself so we can use an unchecked one.
|
||||||
|
|
||||||
if self >= rhs {
|
if self < rhs {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
// SAFETY: just checked this can't overflow
|
// SAFETY: just checked this can't overflow
|
||||||
Some(unsafe { intrinsics::unchecked_sub(self, rhs) })
|
Some(unsafe { intrinsics::unchecked_sub(self, rhs) })
|
||||||
} else {
|
|
||||||
None
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user