[libc-commits] [libc] [llvm] [libc] Refactor `BigInt` (PR #86137)
Guillaume Chatelet via libc-commits
libc-commits at lists.llvm.org
Mon Mar 25 02:59:25 PDT 2024
================
@@ -33,199 +33,91 @@ mask_trailing_ones() {
template <typename T, size_t count>
LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
mask_leading_ones() {
- constexpr T MASK(mask_trailing_ones<T, CHAR_BIT * sizeof(T) - count>());
- return T(~MASK); // bitwise NOT performs integer promotion.
+ return T(~mask_trailing_ones<T, CHAR_BIT * sizeof(T) - count>());
}
-// Add with carry
-template <typename T> struct SumCarry {
- T sum;
- T carry;
-};
-
-// This version is always valid for constexpr.
-template <typename T>
-LIBC_INLINE constexpr cpp::enable_if_t<
- cpp::is_integral_v<T> && cpp::is_unsigned_v<T>, SumCarry<T>>
-add_with_carry_const(T a, T b, T carry_in) {
- T tmp = a + carry_in;
- T sum = b + tmp;
- T carry_out = (sum < b) + (tmp < a);
- return {sum, carry_out};
-}
-
-template <typename T>
-LIBC_INLINE constexpr cpp::enable_if_t<
- cpp::is_integral_v<T> && cpp::is_unsigned_v<T>, SumCarry<T>>
-add_with_carry(T a, T b, T carry_in) {
- return add_with_carry_const<T>(a, b, carry_in);
-}
-
-#if LIBC_HAS_BUILTIN(__builtin_addc)
-// https://clang.llvm.org/docs/LanguageExtensions.html#multiprecision-arithmetic-builtins
-
-template <>
-LIBC_INLINE constexpr SumCarry<unsigned char>
-add_with_carry<unsigned char>(unsigned char a, unsigned char b,
- unsigned char carry_in) {
- if (__builtin_is_constant_evaluated()) {
- return add_with_carry_const<unsigned char>(a, b, carry_in);
- } else {
- SumCarry<unsigned char> result{0, 0};
- result.sum = __builtin_addcb(a, b, carry_in, &result.carry);
- return result;
- }
-}
-
-template <>
-LIBC_INLINE constexpr SumCarry<unsigned short>
-add_with_carry<unsigned short>(unsigned short a, unsigned short b,
- unsigned short carry_in) {
- if (__builtin_is_constant_evaluated()) {
- return add_with_carry_const<unsigned short>(a, b, carry_in);
- } else {
- SumCarry<unsigned short> result{0, 0};
- result.sum = __builtin_addcs(a, b, carry_in, &result.carry);
- return result;
- }
-}
-
-template <>
-LIBC_INLINE constexpr SumCarry<unsigned int>
-add_with_carry<unsigned int>(unsigned int a, unsigned int b,
- unsigned int carry_in) {
- if (__builtin_is_constant_evaluated()) {
- return add_with_carry_const<unsigned int>(a, b, carry_in);
- } else {
- SumCarry<unsigned int> result{0, 0};
- result.sum = __builtin_addc(a, b, carry_in, &result.carry);
- return result;
- }
-}
-
-template <>
-LIBC_INLINE constexpr SumCarry<unsigned long>
-add_with_carry<unsigned long>(unsigned long a, unsigned long b,
- unsigned long carry_in) {
- if (__builtin_is_constant_evaluated()) {
- return add_with_carry_const<unsigned long>(a, b, carry_in);
- } else {
- SumCarry<unsigned long> result{0, 0};
- result.sum = __builtin_addcl(a, b, carry_in, &result.carry);
- return result;
- }
+// Create a bitmask with the count right-most bits set to 0, and all other bits
+// set to 1. Only unsigned types are allowed.
+template <typename T, size_t count>
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+mask_trailing_zeros() {
+ return mask_leading_ones<T, CHAR_BIT * sizeof(T) - count>();
}
-template <>
-LIBC_INLINE constexpr SumCarry<unsigned long long>
-add_with_carry<unsigned long long>(unsigned long long a, unsigned long long b,
- unsigned long long carry_in) {
- if (__builtin_is_constant_evaluated()) {
- return add_with_carry_const<unsigned long long>(a, b, carry_in);
- } else {
- SumCarry<unsigned long long> result{0, 0};
- result.sum = __builtin_addcll(a, b, carry_in, &result.carry);
- return result;
- }
+// Create a bitmask with the count left-most bits set to 0, and all other bits
+// set to 1. Only unsigned types are allowed.
+template <typename T, size_t count>
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+mask_leading_zeros() {
+ return mask_trailing_ones<T, CHAR_BIT * sizeof(T) - count>();
}
-#endif // LIBC_HAS_BUILTIN(__builtin_addc)
+#define RETURN_IF_BUILTIN(TYPE, BUILTIN) \
+ if constexpr (cpp::is_same_v<T, TYPE> && LIBC_HAS_BUILTIN(BUILTIN)) \
+ return BUILTIN(a, b, res);
-// Subtract with borrow
-template <typename T> struct DiffBorrow {
- T diff;
- T borrow;
-};
-
-// This version is always valid for constexpr.
template <typename T>
-LIBC_INLINE constexpr cpp::enable_if_t<
- cpp::is_integral_v<T> && cpp::is_unsigned_v<T>, DiffBorrow<T>>
-sub_with_borrow_const(T a, T b, T borrow_in) {
- T tmp = a - b;
- T diff = tmp - borrow_in;
- T borrow_out = (diff > tmp) + (tmp > a);
- return {diff, borrow_out};
+[[nodiscard]] LIBC_INLINE constexpr bool add_overflow(T a, T b, T *res) {
----------------
gchatelet wrote:
> Maybe note that supporting res==nullptr is for consistency with the intrinsic ?
Yes I wanted to keep the same signature for consistency with the popular GCC / clang intrinsics.
> Passing nullptr to these intrinsics will cause issues: https://stackoverflow.com/a/1514309
Is this the right link? I fail to see the connection between the stackoverflow page and passing `nullptr` to the intrinsic.
> For these intrinsics, I found that the compiler does quite well to keep track of unused carry, so you should just give them a number pair, and let the compiler optimize, instead of using the unsafe interface with pointer.
All the "overflow" intrinsics are present on the compilers we support ([godbolt](https://godbolt.org/z/9Kqe31xYa)).
The add/sub with carry functions are available on clang but not on GCC ([godbolt](https://godbolt.org/z/6xPTWbTb7)) but the [GCC documentation](https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html) states that they are equivalent to the code I wrote (this is what I used actually).
TBH I think the `NumberPair`/`SumCarry`/`SubBorrow` structs get in the way and make the code less readable. e.g.
https://github.com/llvm/llvm-project/blob/3cb024198f6a028089779c0a3cb4d8e753e87c73/libc/src/__support/UInt.h#L52-L58
vs
https://github.com/llvm/llvm-project/blob/c24e4b12348a5e775e094819f99c6e35a2977076/libc/src/__support/UInt.h#L122-L125
> Also, the pattern that I chose when built-ins are not available are well recognized by both clang and gcc to optimized to a single add with carry / sub with borrow instructions.
I played quite a bit with godbolt but I haven't been able to find a difference in generated code between the two implementations for GCC 12.2 ([godbolt](https://godbolt.org/z/4rhn884Y1)). For clang we have the builtins handy.
Do you have benchmarks I can use to check if this code leads to worse performance?
https://github.com/llvm/llvm-project/pull/86137
More information about the libc-commits
mailing list