Revert "[libc] Refactor BigInt"
#87612
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: Guillaume Chatelet (gchatelet) ChangesReverts llvm/llvm-project#86137 Some aarch64 compilers seem to consider that Patch is 81.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/87612.diff 13 Files Affected:
diff --git a/libc/fuzzing/CMakeLists.txt b/libc/fuzzing/CMakeLists.txt
index 816691b4bd4403..82487688af1162 100644
--- a/libc/fuzzing/CMakeLists.txt
+++ b/libc/fuzzing/CMakeLists.txt
@@ -1,7 +1,6 @@
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
add_custom_target(libc-fuzzer)
-add_subdirectory(__support)
# TODO(#85680): Re-enable math fuzzing after headers are sorted out
# add_subdirectory(math)
add_subdirectory(stdlib)
diff --git a/libc/fuzzing/__support/CMakeLists.txt b/libc/fuzzing/__support/CMakeLists.txt
deleted file mode 100644
index 278e914e3fbe95..00000000000000
--- a/libc/fuzzing/__support/CMakeLists.txt
+++ /dev/null
@@ -1,7 +0,0 @@
-add_libc_fuzzer(
- uint_fuzz
- SRCS
- uint_fuzz.cpp
- DEPENDS
- libc.src.__support.uint
-)
diff --git a/libc/fuzzing/__support/uint_fuzz.cpp b/libc/fuzzing/__support/uint_fuzz.cpp
deleted file mode 100644
index f48f00d3b4ba11..00000000000000
--- a/libc/fuzzing/__support/uint_fuzz.cpp
+++ /dev/null
@@ -1,70 +0,0 @@
-#include "src/__support/CPP/bit.h"
-#include "src/__support/UInt.h"
-#include "src/string/memory_utils/inline_memcpy.h"
-
-using namespace LIBC_NAMESPACE;
-
-// Helper function when using gdb / lldb to set a breakpoint and inspect values.
-template <typename T> void debug_and_trap(const char *msg, T a, T b) {
- __builtin_trap();
-}
-
-#define DEBUG_AND_TRAP()
-
-#define TEST_BINOP(OP) \
- if ((a OP b) != (static_cast<T>(BigInt(a) OP BigInt(b)))) \
- debug_and_trap(#OP, a, b);
-
-#define TEST_SHIFTOP(OP) \
- if ((a OP b) != (static_cast<T>(BigInt(a) OP b))) \
- debug_and_trap(#OP, a, b);
-
-#define TEST_FUNCTION(FUN) \
- if (FUN(a) != FUN(BigInt(a))) \
- debug_and_trap(#FUN, a, b);
-
-// Test that basic arithmetic operations of BigInt behave like their scalar
-// counterparts.
-template <typename T, typename BigInt> void run_tests(T a, T b) {
- TEST_BINOP(+)
- TEST_BINOP(-)
- TEST_BINOP(*)
- if (b != 0)
- TEST_BINOP(/)
- if (b >= 0 && b < cpp::numeric_limits<T>::digits) {
- TEST_SHIFTOP(<<)
- TEST_SHIFTOP(>>)
- }
- if constexpr (!BigInt::SIGNED) {
- TEST_FUNCTION(cpp::has_single_bit)
- TEST_FUNCTION(cpp::countr_zero)
- TEST_FUNCTION(cpp::countl_zero)
- TEST_FUNCTION(cpp::countl_one)
- TEST_FUNCTION(cpp::countr_one)
- }
-}
-
-// Reads a T from libfuzzer data.
-template <typename T> T read(const uint8_t *data, size_t &remainder) {
- T out = 0;
- constexpr size_t T_SIZE = sizeof(T);
- const size_t copy_size = remainder < T_SIZE ? remainder : T_SIZE;
- inline_memcpy(&out, data, copy_size);
- remainder -= copy_size;
- return out;
-}
-
-template <typename T, typename BigInt>
-void run_tests(const uint8_t *data, size_t size) {
- const auto a = read<T>(data, size);
- const auto b = read<T>(data, size);
- run_tests<T, BigInt>(a, b);
-}
-
-extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
- // unsigned
- run_tests<uint64_t, BigInt<64, false, uint16_t>>(data, size);
- // signed
- run_tests<int64_t, BigInt<64, true, uint16_t>>(data, size);
- return 0;
-}
diff --git a/libc/src/__support/FPUtil/dyadic_float.h b/libc/src/__support/FPUtil/dyadic_float.h
index e0c205f52383ba..73fd7381c3c838 100644
--- a/libc/src/__support/FPUtil/dyadic_float.h
+++ b/libc/src/__support/FPUtil/dyadic_float.h
@@ -58,9 +58,9 @@ template <size_t Bits> struct DyadicFloat {
// significant bit.
LIBC_INLINE constexpr DyadicFloat &normalize() {
if (!mantissa.is_zero()) {
- int shift_length = cpp::countl_zero(mantissa);
+ int shift_length = static_cast<int>(mantissa.clz());
exponent -= shift_length;
- mantissa <<= static_cast<size_t>(shift_length);
+ mantissa.shift_left(static_cast<size_t>(shift_length));
}
return *this;
}
@@ -233,7 +233,7 @@ LIBC_INLINE constexpr DyadicFloat<Bits> quick_add(DyadicFloat<Bits> a,
result.sign = a.sign;
result.exponent = a.exponent;
result.mantissa = a.mantissa;
- if (result.mantissa.add_overflow(b.mantissa)) {
+ if (result.mantissa.add(b.mantissa)) {
// Mantissa addition overflow.
result.shift_right(1);
result.mantissa.val[DyadicFloat<Bits>::MantissaType::WORD_COUNT - 1] |=
diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index c524de38d98654..282efdba1c5f2b 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -14,11 +14,10 @@
#include "src/__support/CPP/limits.h"
#include "src/__support/CPP/optional.h"
#include "src/__support/CPP/type_traits.h"
-#include "src/__support/macros/attributes.h" // LIBC_INLINE
-#include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
-#include "src/__support/macros/properties/compiler.h" // LIBC_COMPILER_IS_CLANG
+#include "src/__support/macros/attributes.h" // LIBC_INLINE
+#include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
#include "src/__support/macros/properties/types.h" // LIBC_TYPES_HAS_INT128, LIBC_TYPES_HAS_INT64
-#include "src/__support/math_extras.h" // add_with_carry, sub_with_borrow
+#include "src/__support/math_extras.h" // SumCarry, DiffBorrow
#include "src/__support/number_pair.h"
#include <stddef.h> // For size_t
@@ -26,321 +25,71 @@
namespace LIBC_NAMESPACE {
-namespace multiword {
-
-// A type trait mapping unsigned integers to their half-width unsigned
-// counterparts.
+namespace internal {
template <typename T> struct half_width;
-template <> struct half_width<uint16_t> : cpp::type_identity<uint8_t> {};
-template <> struct half_width<uint32_t> : cpp::type_identity<uint16_t> {};
-#ifdef LIBC_TYPES_HAS_INT64
+
template <> struct half_width<uint64_t> : cpp::type_identity<uint32_t> {};
+template <> struct half_width<uint32_t> : cpp::type_identity<uint16_t> {};
+template <> struct half_width<uint16_t> : cpp::type_identity<uint8_t> {};
#ifdef LIBC_TYPES_HAS_INT128
template <> struct half_width<__uint128_t> : cpp::type_identity<uint64_t> {};
#endif // LIBC_TYPES_HAS_INT128
-#endif // LIBC_TYPES_HAS_INT64
-template <typename T> using half_width_t = typename half_width<T>::type;
-
-// An array of two elements that can be used in multiword operations.
-template <typename T> struct DoubleWide final : cpp::array<T, 2> {
- using UP = cpp::array<T, 2>;
- using UP::UP;
- LIBC_INLINE constexpr DoubleWide(T lo, T hi) : UP({lo, hi}) {}
-};
-
-// Converts an unsigned value into a DoubleWide<half_width_t<T>>.
-template <typename T> LIBC_INLINE constexpr auto split(T value) {
- static_assert(cpp::is_unsigned_v<T>);
- return cpp::bit_cast<DoubleWide<half_width_t<T>>>(value);
-}
-
-// The low part of a DoubleWide value.
-template <typename T> LIBC_INLINE constexpr T lo(const DoubleWide<T> &value) {
- return value[0];
-}
-// The high part of a DoubleWide value.
-template <typename T> LIBC_INLINE constexpr T hi(const DoubleWide<T> &value) {
- return value[1];
-}
-// The low part of an unsigned value.
-template <typename T> LIBC_INLINE constexpr half_width_t<T> lo(T value) {
- return lo(split(value));
-}
-// The high part of an unsigned value.
-template <typename T> LIBC_INLINE constexpr half_width_t<T> hi(T value) {
- return hi(split(value));
-}
-
-// Returns 'a' times 'b' in a DoubleWide<word>. Cannot overflow by construction.
-template <typename word>
-LIBC_INLINE constexpr DoubleWide<word> mul2(word a, word b) {
- if constexpr (cpp::is_same_v<word, uint8_t>) {
- return split<uint16_t>(uint16_t(a) * uint16_t(b));
- } else if constexpr (cpp::is_same_v<word, uint16_t>) {
- return split<uint32_t>(uint32_t(a) * uint32_t(b));
- }
-#ifdef LIBC_TYPES_HAS_INT64
- else if constexpr (cpp::is_same_v<word, uint32_t>) {
- return split<uint64_t>(uint64_t(a) * uint64_t(b));
- }
-#endif
-#ifdef LIBC_TYPES_HAS_INT128
- else if constexpr (cpp::is_same_v<word, uint64_t>) {
- return split<__uint128_t>(__uint128_t(a) * __uint128_t(b));
- }
-#endif
- else {
- using half_word = half_width_t<word>;
- const auto shiftl = [](word value) -> word {
- return value << cpp::numeric_limits<half_word>::digits;
- };
- const auto shiftr = [](word value) -> word {
- return value >> cpp::numeric_limits<half_word>::digits;
- };
- // Here we do a one digit multiplication where 'a' and 'b' are of type
- // word. We split 'a' and 'b' into half words and perform the classic long
- // multiplication with 'a' and 'b' being two-digit numbers.
-
- // a a_hi a_lo
- // x b => x b_hi b_lo
- // ---- -----------
- // c result
- // We convert 'lo' and 'hi' from 'half_word' to 'word' so multiplication
- // doesn't overflow.
- const word a_lo = lo(a);
- const word b_lo = lo(b);
- const word a_hi = hi(a);
- const word b_hi = hi(b);
- const word step1 = b_lo * a_lo; // no overflow;
- const word step2 = b_lo * a_hi; // no overflow;
- const word step3 = b_hi * a_lo; // no overflow;
- const word step4 = b_hi * a_hi; // no overflow;
- word lo_digit = step1;
- word hi_digit = step4;
- const word no_carry = 0;
- word carry;
- word _; // unused carry variable.
- lo_digit = add_with_carry<word>(lo_digit, shiftl(step2), no_carry, carry);
- hi_digit = add_with_carry<word>(hi_digit, shiftr(step2), carry, _);
- lo_digit = add_with_carry<word>(lo_digit, shiftl(step3), no_carry, carry);
- hi_digit = add_with_carry<word>(hi_digit, shiftr(step3), carry, _);
- return DoubleWide<word>(lo_digit, hi_digit);
- }
-}
-
-// In-place 'dst op= rhs' with operation with carry propagation. Returns carry.
-template <typename Function, typename word, size_t N, size_t M>
-LIBC_INLINE constexpr word inplace_binop(Function op_with_carry,
- cpp::array<word, N> &dst,
- const cpp::array<word, M> &rhs) {
- static_assert(N >= M);
- word carry_out = 0;
- for (size_t i = 0; i < N; ++i) {
- const bool has_rhs_value = i < M;
- const word rhs_value = has_rhs_value ? rhs[i] : 0;
- const word carry_in = carry_out;
- dst[i] = op_with_carry(dst[i], rhs_value, carry_in, carry_out);
- // stop early when rhs is over and no carry is to be propagated.
- if (!has_rhs_value && carry_out == 0)
- break;
- }
- return carry_out;
-}
-// In-place addition. Returns carry.
-template <typename word, size_t N, size_t M>
-LIBC_INLINE constexpr word add_with_carry(cpp::array<word, N> &dst,
- const cpp::array<word, M> &rhs) {
- return inplace_binop(LIBC_NAMESPACE::add_with_carry<word>, dst, rhs);
-}
+template <typename T> using half_width_t = typename half_width<T>::type;
-// In-place subtraction. Returns borrow.
-template <typename word, size_t N, size_t M>
-LIBC_INLINE constexpr word sub_with_borrow(cpp::array<word, N> &dst,
- const cpp::array<word, M> &rhs) {
- return inplace_binop(LIBC_NAMESPACE::sub_with_borrow<word>, dst, rhs);
-}
+template <typename T> constexpr NumberPair<T> full_mul(T a, T b) {
+ NumberPair<T> pa = split(a);
+ NumberPair<T> pb = split(b);
+ NumberPair<T> prod;
-// In-place multiply-add. Returns carry.
-// i.e., 'dst += b * c'
-template <typename word, size_t N>
-LIBC_INLINE constexpr word mul_add_with_carry(cpp::array<word, N> &dst, word b,
- word c) {
- return add_with_carry(dst, mul2(b, c));
-}
+ prod.lo = pa.lo * pb.lo; // exact
+ prod.hi = pa.hi * pb.hi; // exact
+ NumberPair<T> lo_hi = split(pa.lo * pb.hi); // exact
+ NumberPair<T> hi_lo = split(pa.hi * pb.lo); // exact
-// An array of two elements serving as an accumulator during multiword
-// computations.
-template <typename T> struct Accumulator final : cpp::array<T, 2> {
- using UP = cpp::array<T, 2>;
- LIBC_INLINE constexpr Accumulator() : UP({0, 0}) {}
- LIBC_INLINE constexpr T advance(T carry_in) {
- auto result = UP::front();
- UP::front() = UP::back();
- UP::back() = carry_in;
- return result;
- }
- LIBC_INLINE constexpr T sum() const { return UP::front(); }
- LIBC_INLINE constexpr T carry() const { return UP::back(); }
-};
+ constexpr size_t HALF_BIT_WIDTH = sizeof(T) * CHAR_BIT / 2;
-// In-place multiplication by a single word. Returns carry.
-template <typename word, size_t N>
-LIBC_INLINE constexpr word scalar_multiply_with_carry(cpp::array<word, N> &dst,
- word x) {
- Accumulator<word> acc;
- for (auto &val : dst) {
- const word carry = mul_add_with_carry(acc, val, x);
- val = acc.advance(carry);
- }
- return acc.carry();
-}
+ auto r1 = add_with_carry(prod.lo, lo_hi.lo << HALF_BIT_WIDTH, T(0));
+ prod.lo = r1.sum;
+ prod.hi = add_with_carry(prod.hi, lo_hi.hi, r1.carry).sum;
-// Multiplication of 'lhs' by 'rhs' into 'dst'. Returns carry.
-// This function is safe to use for signed numbers.
-// https://stackoverflow.com/a/20793834
-// https://pages.cs.wisc.edu/%7Emarkhill/cs354/Fall2008/beyond354/int.mult.html
-template <typename word, size_t O, size_t M, size_t N>
-LIBC_INLINE constexpr word multiply_with_carry(cpp::array<word, O> &dst,
- const cpp::array<word, M> &lhs,
- const cpp::array<word, N> &rhs) {
- static_assert(O >= M + N);
- Accumulator<word> acc;
- for (size_t i = 0; i < O; ++i) {
- const size_t lower_idx = i < N ? 0 : i - N + 1;
- const size_t upper_idx = i < M ? i : M - 1;
- word carry = 0;
- for (size_t j = lower_idx; j <= upper_idx; ++j)
- carry += mul_add_with_carry(acc, lhs[j], rhs[i - j]);
- dst[i] = acc.advance(carry);
- }
- return acc.carry();
-}
+ auto r2 = add_with_carry(prod.lo, hi_lo.lo << HALF_BIT_WIDTH, T(0));
+ prod.lo = r2.sum;
+ prod.hi = add_with_carry(prod.hi, hi_lo.hi, r2.carry).sum;
-template <typename word, size_t N>
-LIBC_INLINE constexpr void quick_mul_hi(cpp::array<word, N> &dst,
- const cpp::array<word, N> &lhs,
- const cpp::array<word, N> &rhs) {
- Accumulator<word> acc;
- word carry = 0;
- // First round of accumulation for those at N - 1 in the full product.
- for (size_t i = 0; i < N; ++i)
- carry += mul_add_with_carry(acc, lhs[i], rhs[N - 1 - i]);
- for (size_t i = N; i < 2 * N - 1; ++i) {
- acc.advance(carry);
- carry = 0;
- for (size_t j = i - N + 1; j < N; ++j)
- carry += mul_add_with_carry(acc, lhs[j], rhs[i - j]);
- dst[i - N] = acc.sum();
- }
- dst.back() = acc.carry();
+ return prod;
}
-template <typename word, size_t N>
-LIBC_INLINE constexpr bool is_negative(cpp::array<word, N> &array) {
- using signed_word = cpp::make_signed_t<word>;
- return cpp::bit_cast<signed_word>(array.back()) < 0;
+template <>
+LIBC_INLINE constexpr NumberPair<uint32_t> full_mul<uint32_t>(uint32_t a,
+ uint32_t b) {
+ uint64_t prod = uint64_t(a) * uint64_t(b);
+ NumberPair<uint32_t> result;
+ result.lo = uint32_t(prod);
+ result.hi = uint32_t(prod >> 32);
+ return result;
}
-// An enum for the shift function below.
-enum Direction { LEFT, RIGHT };
-
-// A bitwise shift on an array of elements.
-// TODO: Make the result UB when 'offset' is greater or equal to the number of
-// bits in 'array'. This will allow for better code performance.
-template <Direction direction, bool is_signed, typename word, size_t N>
-LIBC_INLINE constexpr cpp::array<word, N> shift(cpp::array<word, N> array,
- size_t offset) {
- static_assert(direction == LEFT || direction == RIGHT);
- constexpr size_t WORD_BITS = cpp::numeric_limits<word>::digits;
- constexpr size_t TOTAL_BITS = N * WORD_BITS;
- if (LIBC_UNLIKELY(offset == 0))
- return array;
- if (LIBC_UNLIKELY(offset >= TOTAL_BITS))
- return {};
#ifdef LIBC_TYPES_HAS_INT128
- if constexpr (TOTAL_BITS == 128) {
- using type = cpp::conditional_t<is_signed, __int128_t, __uint128_t>;
- auto tmp = cpp::bit_cast<type>(array);
- if constexpr (direction == LEFT)
- tmp <<= offset;
- else
- tmp >>= offset;
- return cpp::bit_cast<cpp::array<word, N>>(tmp);
- }
-#endif
- const bool is_neg = is_signed && is_negative(array);
- constexpr auto at = [](size_t index) -> int {
- // reverse iteration when direction == LEFT.
- if constexpr (direction == LEFT)
- return int(N) - int(index) - 1;
- return int(index);
- };
- const auto safe_get_at = [&](size_t index) -> word {
- // return appropriate value when accessing out of bound elements.
- const int i = at(index);
- if (i < 0)
- return 0;
- if (i >= int(N))
- return is_neg ? -1 : 0;
- return array[i];
- };
- const size_t index_offset = offset / WORD_BITS;
- const size_t bit_offset = offset % WORD_BITS;
-#ifdef LIBC_COMPILER_IS_CLANG
- __builtin_assume(index_offset < N);
-#endif
- cpp::array<word, N> out = {};
- for (size_t index = 0; index < N; ++index) {
- const word part1 = safe_get_at(index + index_offset);
- const word part2 = safe_get_at(index + index_offset + 1);
- word &dst = out[at(index)];
- if (bit_offset == 0)
- dst = part1; // no crosstalk between parts.
- else if constexpr (direction == LEFT)
- dst = (part1 << bit_offset) | (part2 >> (WORD_BITS - bit_offset));
- else
- dst = (part1 >> bit_offset) | (part2 << (WORD_BITS - bit_offset));
- }
- return out;
+template <>
+LIBC_INLINE constexpr NumberPair<uint64_t> full_mul<uint64_t>(uint64_t a,
+ uint64_t b) {
+ __uint128_t prod = __uint128_t(a) * __uint128_t(b);
+ NumberPair<uint64_t> result;
+ result.lo = uint64_t(prod);
+ result.hi = uint64_t(prod >> 64);
+ return result;
}
+#endif // LIBC_TYPES_HAS_INT128
-#define DECLARE_COUNTBIT(NAME, INDEX_EXPR) \
- template <typename word, size_t N> \
- LIBC_INLINE constexpr int NAME(const cpp::array<word, N> &val) { \
- int bit_count = 0; \
- for (size_t i = 0; i < N; ++i) { \
- const int word_count = cpp::NAME<word>(val[INDEX_EXPR]); \
- bit_count += word_count; \
- if (word_count != cpp::numeric_limits<word>::digits) \
- break; \
- } \
- return bit_count; \
- }
-
-DECLARE_COUNTBIT(countr_zero, i) // iterating forward
-DECLARE_COUNTBIT(countr_one, i) // iterating forward
-DECLARE_COUNTBIT(countl_zero, N - i - 1) // iterating backward
-DECLARE_COUNTBIT(countl_one, N - i - 1) // iterating backward
-
-} // namespace multiword
+} // namespace internal
template <size_t Bits, bool Signed, typename WordType = uint64_t>
struct BigInt {
-private:
static_assert(cpp::is_integral_v<WordType> && cpp::is_unsigned_v<WordType>,
"WordType must be unsigned integer.");
- struct Division {
- BigInt quotient;
- BigInt remainder;
- };
-
-public:
using word_type = WordType;
- using unsigned_type = BigInt<Bits, false, word_type>;
- using signed_type = BigInt<Bits, true, word_type>;
-
LIBC_INLINE_VAR static constexpr bool SIGNED = Signed;
LIBC_INLINE_VAR static constexpr size_t BITS = Bits;
LIBC_INLINE_VAR
@@ -351,7 +100,10 @@ struct BigInt {
LIBC_INLINE_VAR static constexpr size_t WORD_COUNT = Bits / WORD_SIZE;
- cpp::array<WordType, WORD_COUNT> val{}; // zero initialized.
+ using unsigned_type = BigInt<BITS, false, word_type>;
+ using signed_type = BigInt<BITS, true, word_type>;
+
+ cpp::array<WordType, WORD_COUNT> val{};
LIBC_INLINE constexpr BigInt() = default;
@@ -360,67 +112,76 @@ struct BigInt {
template <size_t OtherBits, bool OtherSigned>
LIBC_INLINE constexpr BigInt(
const BigInt<OtherBi...
[truncated]
|
|
Was that this CI run? Is there a bug to be filed against clang here? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #86137
Some aarch64 compilers seem to consider that
uint128_tis notis_trivially_constructiblewhich preventsbit_cast-ing.