Rollup merge of #125038 - ivan-shrimp:checked_sub, r=joboet

fmease · web-flow · commit 4f7d9d4ad815 · 2024-05-15T14:21:38.000+02:00
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: https://github.com/rust-lang/rust/blob/ee97564e3a9f9ac8c65103abb37c6aa48d95bfa2/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`: https://github.com/llvm/llvm-project/blob/e586556e375fc5c4f7e76b5c299cb981f2016108/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.
diff --git a/library/core/src/num/uint_macros.rs b/library/core/src/num/uint_macros.rs
@@ -584,11 +584,11 @@ macro_rules! uint_impl {
             // Thus, rather than using `overflowing_sub` that produces a wrapping
             // 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
                 Some(unsafe { intrinsics::unchecked_sub(self, rhs) })
-            } else {
-                None
             }
         }
 

Original file line number	Diff line number	Diff line change
`@@ -584,11 +584,11 @@ macro_rules! uint_impl {`
`584`	`584`	// Thus, rather than using `overflowing_sub` that produces a wrapping
`585`	`585`	`// subtraction, check it ourself so we can use an unchecked one.`
`586`	`586`
`587`		`- if self >= rhs {`
	`587`	`+ if self < rhs {`
	`588`	`+ None`
	`589`	`+ } else {`
`588`	`590`	`// SAFETY: just checked this can't overflow`
`589`	`591`	`Some(unsafe { intrinsics::unchecked_sub(self, rhs) })`
`590`		`- } else {`
`591`		`- None`
`592`	`592`	`}`
`593`	`593`	`}`
`594`	`594`