Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[reland][libc] Remove UB specializations of type traits for BigInt #84299

Merged

Conversation

gchatelet
Copy link
Contributor

@gchatelet gchatelet commented Mar 7, 2024

Note: This is a reland of #84035.

The standard specifies that it it UB to specialize the following traits:

  • std::is_integral
  • std::is_unsigned
  • std::make_unsigned
  • std::make_signed

This patch:

  • Removes specializations for BigInt
  • Transforms SFINAE for bit.h functions from template parameter to
    return type (This makes specialization easier).
  • Adds BigInt specialization for bit.h functions.
  • Fixes code depending on previous specializations.

)

The standard specifies that it it UB to specialize the following traits:
 - `std::is_integral`
 - `std::is_unsigned`
 - `std::make_unsigned`
 - `std::make_signed`

This patch:
 - Removes specializations for `BigInt`
 - Transforms SFINAE for `bit.h` functions from template parameter to
   return type (This makes specialization easier).
 - Adds `BigInt` specialization for `bit.h` functions.
 - Fixes code depending on previous specializations.
@llvmbot llvmbot added the libc label Mar 7, 2024
@gchatelet gchatelet changed the title [libc] Remove UB specializations of type traits for BigInt (#84035) [reland][libc] Remove UB specializations of type traits for BigInt (#84035) Mar 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 7, 2024

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

The standard specifies that it it UB to specialize the following traits:

  • std::is_integral
  • std::is_unsigned
  • std::make_unsigned
  • std::make_signed

This patch:

  • Removes specializations for BigInt
  • Transforms SFINAE for bit.h functions from template parameter to
    return type (This makes specialization easier).
  • Adds BigInt specialization for bit.h functions.
  • Fixes code depending on previous specializations.

Patch is 30.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84299.diff

13 Files Affected:

  • (modified) libc/src/__support/CMakeLists.txt (+1)
  • (modified) libc/src/__support/CPP/bit.h (+69-43)
  • (modified) libc/src/__support/UInt.h (+135-57)
  • (modified) libc/src/__support/float_to_string.h (+1-1)
  • (modified) libc/src/__support/integer_to_string.h (+16-3)
  • (modified) libc/test/UnitTest/CMakeLists.txt (+1)
  • (modified) libc/test/UnitTest/LibcTest.cpp (+5-5)
  • (modified) libc/test/UnitTest/LibcTest.h (+1)
  • (modified) libc/test/UnitTest/TestLogger.cpp (+5-3)
  • (modified) libc/test/src/__support/CPP/bit_test.cpp (+34-13)
  • (modified) libc/test/src/__support/FPUtil/fpbits_test.cpp (+2)
  • (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+1)
  • (modified) utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel (+1)
diff --git a/libc/src/__support/CMakeLists.txt b/libc/src/__support/CMakeLists.txt
index 1a4b3e9a2145c0..17c04aa57e6fd6 100644
--- a/libc/src/__support/CMakeLists.txt
+++ b/libc/src/__support/CMakeLists.txt
@@ -95,6 +95,7 @@ add_header_library(
   HDRS
     integer_to_string.h
   DEPENDS
+    .uint
     libc.src.__support.common
     libc.src.__support.CPP.algorithm
     libc.src.__support.CPP.limits
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index 7d11e7d5c497e0..bc2f595845a95f 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -27,13 +27,14 @@ namespace LIBC_NAMESPACE::cpp {
 
 // This implementation of bit_cast requires trivially-constructible To, to avoid
 // UB in the implementation.
-template <
-    typename To, typename From,
-    typename = cpp::enable_if_t<sizeof(To) == sizeof(From) &&
-                                cpp::is_trivially_constructible<To>::value &&
-                                cpp::is_trivially_copyable<To>::value &&
-                                cpp::is_trivially_copyable<From>::value>>
-LIBC_INLINE constexpr To bit_cast(const From &from) {
+template <typename To, typename From>
+LIBC_INLINE constexpr cpp::enable_if_t<
+    (sizeof(To) == sizeof(From)) &&
+        cpp::is_trivially_constructible<To>::value &&
+        cpp::is_trivially_copyable<To>::value &&
+        cpp::is_trivially_copyable<From>::value,
+    To>
+bit_cast(const From &from) {
   MSAN_UNPOISON(&from, sizeof(From));
 #if LIBC_HAS_BUILTIN(__builtin_bit_cast)
   return __builtin_bit_cast(To, from);
@@ -51,8 +52,10 @@ LIBC_INLINE constexpr To bit_cast(const From &from) {
 #endif // LIBC_HAS_BUILTIN(__builtin_bit_cast)
 }
 
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr bool has_single_bit(T value) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>,
+                                                     bool>
+has_single_bit(T value) {
   return (value != 0) && ((value & (value - 1)) == 0);
 }
 
@@ -70,8 +73,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 /// Only unsigned integral types are allowed.
 ///
 /// Returns cpp::numeric_limits<T>::digits on an input of 0.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int countr_zero(T value) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+countr_zero(T value) {
   if (!value)
     return cpp::numeric_limits<T>::digits;
   if (value & 0x1)
@@ -103,8 +107,9 @@ ADD_SPECIALIZATION(countr_zero, unsigned long long, __builtin_ctzll)
 /// Only unsigned integral types are allowed.
 ///
 /// Returns cpp::numeric_limits<T>::digits on an input of 0.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int countl_zero(T value) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+countl_zero(T value) {
   if (!value)
     return cpp::numeric_limits<T>::digits;
   // Bisection method.
@@ -135,8 +140,9 @@ ADD_SPECIALIZATION(countl_zero, unsigned long long, __builtin_clzll)
 /// Only unsigned integral types are allowed.
 ///
 /// Returns cpp::numeric_limits<T>::digits on an input of all ones.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int countl_one(T value) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+countl_one(T value) {
   return cpp::countl_zero<T>(~value);
 }
 
@@ -147,8 +153,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 /// Only unsigned integral types are allowed.
 ///
 /// Returns cpp::numeric_limits<T>::digits on an input of all ones.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int countr_one(T value) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+countr_one(T value) {
   return cpp::countr_zero<T>(~value);
 }
 
@@ -156,8 +163,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 /// Returns 0 otherwise.
 ///
 /// Ex. bit_width(5) == 3.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int bit_width(T value) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+bit_width(T value) {
   return cpp::numeric_limits<T>::digits - cpp::countl_zero(value);
 }
 
@@ -165,8 +173,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 /// nonzero.  Returns 0 otherwise.
 ///
 /// Ex. bit_floor(5) == 4.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr T bit_floor(T value) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+bit_floor(T value) {
   if (!value)
     return 0;
   return T(1) << (cpp::bit_width(value) - 1);
@@ -179,8 +188,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 ///
 /// The return value is undefined if the input is larger than the largest power
 /// of two representable in T.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr T bit_ceil(T value) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+bit_ceil(T value) {
   if (value < 2)
     return 1;
   return T(1) << cpp::bit_width<T>(value - 1u);
@@ -190,28 +200,31 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
 // from https://blog.regehr.org/archives/1063.
 
 // Forward-declare rotr so that rotl can use it.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr T rotr(T value, int rotate);
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+rotr(T value, int rotate);
 
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr T rotl(T value, int rotate) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+rotl(T value, int rotate) {
   constexpr unsigned N = cpp::numeric_limits<T>::digits;
   rotate = rotate % N;
   if (!rotate)
     return value;
   if (rotate < 0)
-    return cpp::rotr(value, -rotate);
+    return cpp::rotr<T>(value, -rotate);
   return (value << rotate) | (value >> (N - rotate));
 }
 
-template <typename T, typename>
-[[nodiscard]] LIBC_INLINE constexpr T rotr(T value, int rotate) {
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+rotr(T value, int rotate) {
   constexpr unsigned N = cpp::numeric_limits<T>::digits;
   rotate = rotate % N;
   if (!rotate)
     return value;
   if (rotate < 0)
-    return cpp::rotl(value, -rotate);
+    return cpp::rotl<T>(value, -rotate);
   return (value >> rotate) | (value << (N - rotate));
 }
 
@@ -226,33 +239,44 @@ LIBC_INLINE constexpr To bit_or_static_cast(const From &from) {
   }
 }
 
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int first_leading_zero(T value) {
+// TODO: remove from 'bit.h' as it is not a standard function.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+first_leading_zero(T value) {
   return value == cpp::numeric_limits<T>::max() ? 0 : countl_one(value) + 1;
 }
 
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int first_leading_one(T value) {
+// TODO: remove from 'bit.h' as it is not a standard function.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+first_leading_one(T value) {
   return first_leading_zero(static_cast<T>(~value));
 }
 
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int first_trailing_zero(T value) {
+// TODO: remove from 'bit.h' as it is not a standard function.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+first_trailing_zero(T value) {
   return value == cpp::numeric_limits<T>::max()
              ? 0
              : countr_zero(static_cast<T>(~value)) + 1;
 }
 
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int first_trailing_one(T value) {
+// TODO: remove from 'bit.h' as it is not a standard function.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+first_trailing_one(T value) {
   return value == cpp::numeric_limits<T>::max() ? 0 : countr_zero(value) + 1;
 }
 
 /// Count number of 1's aka population count or hamming weight.
 ///
 /// Only unsigned integral types are allowed.
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int count_ones(T value) {
+// TODO: rename as 'popcount' to follow the standard
+// https://en.cppreference.com/w/cpp/numeric/popcount
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+count_ones(T value) {
   int count = 0;
   for (int i = 0; i != cpp::numeric_limits<T>::digits; ++i)
     if ((value >> i) & 0x1)
@@ -272,8 +296,10 @@ ADD_SPECIALIZATION(unsigned long long, __builtin_popcountll)
 // TODO: 128b specializations?
 #undef ADD_SPECIALIZATION
 
-template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
-[[nodiscard]] LIBC_INLINE constexpr int count_zeros(T value) {
+// TODO: remove from 'bit.h' as it is not a standard function.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
+count_zeros(T value) {
   return count_ones<T>(static_cast<T>(~value));
 }
 
diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index 5973e6fab1d7d5..b3d8f00b9a01a5 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -43,6 +43,9 @@ struct BigInt {
   static_assert(is_integral_v<WordType> && is_unsigned_v<WordType>,
                 "WordType must be unsigned integer.");
 
+  using word_type = WordType;
+  LIBC_INLINE_VAR static constexpr bool SIGNED = Signed;
+  LIBC_INLINE_VAR static constexpr size_t BITS = Bits;
   LIBC_INLINE_VAR
   static constexpr size_t WORD_SIZE = sizeof(WordType) * CHAR_BIT;
 
@@ -50,6 +53,10 @@ struct BigInt {
                 "Number of bits in BigInt should be a multiple of WORD_SIZE.");
 
   LIBC_INLINE_VAR static constexpr size_t WORD_COUNT = Bits / WORD_SIZE;
+
+  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;
@@ -579,19 +586,33 @@ struct BigInt {
     return *this;
   }
 
-  LIBC_INLINE constexpr uint64_t clz() {
-    uint64_t leading_zeroes = 0;
-    for (size_t i = WORD_COUNT; i > 0; --i) {
-      if (val[i - 1] == 0) {
-        leading_zeroes += WORD_SIZE;
-      } else {
-        leading_zeroes += countl_zero(val[i - 1]);
+  // TODO: remove and use cpp::countl_zero below.
+  [[nodiscard]] LIBC_INLINE constexpr int clz() const {
+    constexpr int word_digits = cpp::numeric_limits<word_type>::digits;
+    int leading_zeroes = 0;
+    for (auto i = val.size(); i > 0;) {
+      --i;
+      const int zeroes = countl_zero(val[i]);
+      leading_zeroes += zeroes;
+      if (zeroes != word_digits)
         break;
-      }
     }
     return leading_zeroes;
   }
 
+  // TODO: remove and use cpp::countr_zero below.
+  [[nodiscard]] LIBC_INLINE constexpr int ctz() const {
+    constexpr int word_digits = cpp::numeric_limits<word_type>::digits;
+    int trailing_zeroes = 0;
+    for (auto word : val) {
+      const int zeroes = countr_zero(word);
+      trailing_zeroes += zeroes;
+      if (zeroes != word_digits)
+        break;
+    }
+    return trailing_zeroes;
+  }
+
   LIBC_INLINE constexpr void shift_left(size_t s) {
     if constexpr (Bits == WORD_SIZE) {
       // Use native types if possible.
@@ -916,66 +937,123 @@ template <> class numeric_limits<Int<128>> {
   LIBC_INLINE_VAR static constexpr int digits = 128;
 };
 
-// Provides is_integral of U/Int<128>, U/Int<192>, U/Int<256>.
-template <size_t Bits, bool Signed, typename T>
-struct is_integral<BigInt<Bits, Signed, T>> : cpp::true_type {};
+// type traits to determine whether a T is a cpp::BigInt.
+template <typename T> struct is_big_int : cpp::false_type {};
 
-// Provides is_unsigned of UInt<128>, UInt<192>, UInt<256>.
 template <size_t Bits, bool Signed, typename T>
-struct is_unsigned<BigInt<Bits, Signed, T>> : cpp::bool_constant<!Signed> {};
-
-template <size_t Bits, bool Signed, typename T>
-struct make_unsigned<BigInt<Bits, Signed, T>>
-    : type_identity<BigInt<Bits, false, T>> {};
-
-template <size_t Bits, bool Signed, typename T>
-struct make_signed<BigInt<Bits, Signed, T>>
-    : type_identity<BigInt<Bits, true, T>> {};
-
-namespace internal {
-template <typename T> struct is_custom_uint : cpp::false_type {};
-
-template <size_t Bits, bool Signed, typename T>
-struct is_custom_uint<BigInt<Bits, Signed, T>> : cpp::true_type {};
-} // namespace internal
-
-// bit_cast to UInt
-// Note: The standard scheme for SFINAE selection is to have exactly one
-// function instanciation valid at a time. This is usually done by having a
-// predicate in one function and the negated predicate in the other one.
-// e.g.
-// template<typename = cpp::enable_if_t< is_custom_uint<To>::value == true> ...
-// template<typename = cpp::enable_if_t< is_custom_uint<To>::value == false> ...
-//
-// Unfortunately this would make the default 'cpp::bit_cast' aware of
-// 'is_custom_uint' (or any other customization). To prevent exposing all
-// customizations in the original function, we create a different function with
-// four 'typename's instead of three - otherwise it would be considered as a
-// redeclaration of the same function leading to "error: template parameter
-// redefines default argument".
-template <typename To, typename From,
-          typename = cpp::enable_if_t<sizeof(To) == sizeof(From) &&
-                                      cpp::is_trivially_copyable<To>::value &&
-                                      cpp::is_trivially_copyable<From>::value>,
-          typename = cpp::enable_if_t<internal::is_custom_uint<To>::value>>
-LIBC_INLINE constexpr To bit_cast(const From &from) {
+struct is_big_int<BigInt<Bits, Signed, T>> : cpp::true_type {};
+
+template <class T>
+LIBC_INLINE_VAR constexpr bool is_big_int_v = is_big_int<T>::value;
+
+// Specialization of cpp::bit_cast ('bit.h') from T to BigInt.
+template <typename To, typename From>
+LIBC_INLINE constexpr cpp::enable_if_t<
+    (sizeof(To) == sizeof(From)) && cpp::is_trivially_copyable<To>::value &&
+        cpp::is_trivially_copyable<From>::value && is_big_int<To>::value,
+    To>
+bit_cast(const From &from) {
   To out;
   using Storage = decltype(out.val);
   out.val = cpp::bit_cast<Storage>(from);
   return out;
 }
 
-// bit_cast from UInt
-template <
-    typename To, size_t Bits,
-    typename = cpp::enable_if_t<sizeof(To) == sizeof(UInt<Bits>) &&
-                                cpp::is_trivially_constructible<To>::value &&
-                                cpp::is_trivially_copyable<To>::value &&
-                                cpp::is_trivially_copyable<UInt<Bits>>::value>>
-LIBC_INLINE constexpr To bit_cast(const UInt<Bits> &from) {
+// Specialization of cpp::bit_cast ('bit.h') from BigInt to T.
+template <typename To, size_t Bits>
+LIBC_INLINE constexpr cpp::enable_if_t<
+    sizeof(To) == sizeof(UInt<Bits>) &&
+        cpp::is_trivially_constructible<To>::value &&
+        cpp::is_trivially_copyable<To>::value &&
+        cpp::is_trivially_copyable<UInt<Bits>>::value,
+    To>
+bit_cast(const UInt<Bits> &from) {
   return cpp::bit_cast<To>(from.val);
 }
 
+// Specialization of cpp::has_single_bit ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, bool>
+has_single_bit(T value) {
+  int bits = 0;
+  for (auto word : value.val) {
+    if (word == 0)
+      continue;
+    bits += count_ones(word);
+    if (bits > 1)
+      return false;
+  }
+  return bits == 1;
+}
+
+// Specialization of cpp::countr_zero ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, int>
+countr_zero(const T &value) {
+  return value.ctz();
+}
+
+// Specialization of cpp::countl_zero ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, int>
+countl_zero(const T &value) {
+  return value.clz();
+}
+
+// Specialization of cpp::countl_one ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, int>
+countl_one(T value) {
+  // TODO : Implement a faster version not involving operator~.
+  return cpp::countl_zero<T>(~value);
+}
+
+// Specialization of cpp::countr_one ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, int>
+countr_one(T value) {
+  // TODO : Implement a faster version not involving operator~.
+  return cpp::countr_zero<T>(~value);
+}
+
+// Specialization of cpp::bit_width ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, int>
+bit_width(T value) {
+  return cpp::numeric_limits<T>::digits - cpp::countl_zero(value);
+}
+
+// Forward-declare rotr so that rotl can use it.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, T>
+rotr(T value, int rotate);
+
+// Specialization of cpp::rotl ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, T>
+rotl(T value, int rotate) {
+  constexpr unsigned N = cpp::numeric_limits<T>::digits;
+  rotate = rotate % N;
+  if (!rotate)
+    return value;
+  if (rotate < 0)
+    return cpp::rotr<T>(value, -rotate);
+  return (value << rotate) | (value >> (N - rotate));
+}
+
+// Specialization of cpp::rotr ('bit.h') for BigInt.
+template <typename T>
+[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, T>
+rotr(T value, int rotate) {
+  constexpr unsigned N = cpp::numeric_limits<T>::digits;
+  rotate = rotate % N;
+  if (!rotate)
+    return value;
+  if (rotate < 0)
+    return cpp::rotl<T>(value, -rotate);
+  return (value >> rotate) | (value << (N - rotate));
+}
+
 } // namespace LIBC_NAMESPACE::cpp
 
 #endif // LLVM_LIBC_SRC___SUPPORT_UINT_H
diff --git a/libc/src/__support/float_to_string.h b/libc/src/__support/float_to_string.h
index 744842ced8d772..27476433a94575 100644
--- a/libc/src/__support/float_to_string.h
+++ b/libc/src/__support/float_to_string.h
@@ -713,7 +713,7 @@ template <> class FloatToString<long double> {
       float_as_fixed.shift_left(SHIFT_AMOUNT);
 
       // If there are still digits above the decimal point, handle those.
-      if (float_as_fixed.clz() < EXTRA_INT_WIDTH) {
+      if (float_as_fixed.clz() < static_cast<int>(EXTRA_INT_WIDTH)) {
         cpp::UInt<EXTRA_INT_WIDTH> above_decimal_point =
             float_as_fixed >> FLOAT_AS_INT_WIDTH;
 
diff --git a/libc/src/__support/integer_to_string.h b/libc/src/__support/integer_to_string.h
index 81ed21ccfca166..a5872dce652036 100644
--- a/libc/src/__support/integer_to_string.h
+++ b/libc/src/__support/integer_to_string.h
@@ -67,6 +67,7 @@
 #include "src/__support/CPP/span.h"
 #include "src/__support/CPP/string_view.h"
 #include "src/__support/CPP/type_traits.h"
+#include "src/__support/UInt.h" // is_big_int
 #include "src/__support/common.h"
 
 namespace LIBC_NAMESPACE {
@@ -149,6 +150,18 @@ template <bool forward> class StringBufferWriterImpl {
 using StringBufferWriter = StringBufferWriterImpl<true>;
 using ...
[truncated]

@gchatelet gchatelet changed the title [reland][libc] Remove UB specializations of type traits for BigInt (#84035) [reland][libc] Remove UB specializations of type traits for BigInt Mar 7, 2024
@gchatelet gchatelet requested a review from legrosbuffle March 7, 2024 10:37
@gchatelet
Copy link
Contributor Author

@legrosbuffle fixup are added as separate commits on top of cherry-picking the defective patch.

@gchatelet gchatelet merged commit 245d669 into llvm:main Mar 7, 2024
6 checks passed
@gchatelet gchatelet deleted the reland_use_return_type_sfinae_for_bit_utils branch March 7, 2024 10:41
gchatelet added a commit to gchatelet/llvm-project that referenced this pull request Mar 7, 2024
…nes` / `mask_trailing_ones`

llvm#84299 broke the arm32 build, this patch fixes it forward.
gchatelet added a commit that referenced this pull request Mar 7, 2024
…nes` / `mask_trailing_ones` (#84325)

#84299 broke the arm32 build, this patch fixes it forward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants