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

[libc][NFC] Refactor FPBits and remove LongDoubleBits specialization #78192

Merged
merged 9 commits into from
Jan 16, 2024

Conversation

gchatelet
Copy link
Contributor

@gchatelet gchatelet commented Jan 15, 2024

This patch removes the FPBits specialization for x86 Extended Precision by moving it up to FPRep.
It also introduces two enums (Exp and Sig) to represent the exponent and significant parts of the floating point numbers. These enums are used to construct and observe floating point representations.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 15, 2024

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

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

8 Files Affected:

  • (modified) libc/src/__support/FPUtil/FPBits.h (+284-90)
  • (modified) libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h (+1-1)
  • (removed) libc/src/__support/FPUtil/x86_64/LongDoubleBits.h (-179)
  • (modified) libc/src/__support/FPUtil/x86_64/NextAfterLongDouble.h (+4-4)
  • (modified) libc/test/src/__support/FPUtil/fpbits_test.cpp (+216)
  • (modified) libc/test/utils/FPUtil/x86_long_double_test.cpp (+6-6)
  • (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (-1)
  • (added) utils/bazel/llvm-project-overlay/libc/test/src/__support/FPUtil/BUILD.bazel (+42)
diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 93e32ba7cc9415b..58d63a7d01fb823 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -132,6 +132,70 @@ struct FPRepBase : public internal::FPLayout<fp_type> {
   static_assert((SIG_MASK & EXP_MASK & SIGN_MASK) == 0, "masks disjoint");
   static_assert((SIG_MASK | EXP_MASK | SIGN_MASK) == FP_MASK, "masks cover");
 
+protected:
+  enum class Exp : int32_t {
+    ZERO = 0,
+    // The exponent value for denormal numbers.
+    SUBNORMAL = -EXP_BIAS,
+    // The minimum exponent value for normal numbers.
+    MIN = SUBNORMAL + 1,
+    // The maximum exponent value for normal numbers.
+    MAX = EXP_BIAS,
+    // Special value all ones.
+    INF = MAX + 1,
+    // Aliases
+    BITS_ALL_ZEROES = SUBNORMAL,
+    BITS_ALL_ONES = INF,
+  };
+
+  enum class Sig : StorageType {
+    ZERO = 0,
+    ONE = 1,
+    MSB = StorageType(1) << (SIG_LEN - 1),
+    // Aliases
+    BITS_ALL_ZEROES = ZERO,
+    BITS_ALL_ONES = SIG_MASK,
+  };
+
+  template <typename T>
+  LIBC_INLINE static constexpr auto storage_cast(T value) {
+    return static_cast<StorageType>(value);
+  }
+
+  LIBC_INLINE friend constexpr Sig operator|(const Sig a, const Sig b) {
+    return Sig{storage_cast(storage_cast(a) | storage_cast(b))};
+  }
+  LIBC_INLINE friend constexpr Sig operator^(const Sig a, const Sig b) {
+    return Sig{storage_cast(storage_cast(a) ^ storage_cast(b))};
+  }
+  LIBC_INLINE friend constexpr Sig operator>>(const Sig a, int shift) {
+    return Sig{storage_cast(storage_cast(a) >> shift)};
+  }
+
+  LIBC_INLINE static constexpr StorageType encode(Exp exp) {
+    return storage_cast(static_cast<int32_t>(exp) + EXP_BIAS) << SIG_LEN;
+  }
+
+  LIBC_INLINE static constexpr StorageType encode(Sig value) {
+    return storage_cast(value) & SIG_MASK;
+  }
+
+  LIBC_INLINE static constexpr StorageType encode(Exp exp, Sig sig) {
+    return encode(exp) | encode(sig);
+  }
+
+  LIBC_INLINE static constexpr StorageType encode(bool sign, Exp exp, Sig sig) {
+    if (sign)
+      return SIGN_MASK | encode(exp, sig);
+    return encode(exp, sig);
+  }
+
+  LIBC_INLINE constexpr StorageType exp_bits() const { return bits & EXP_MASK; }
+  LIBC_INLINE constexpr StorageType sig_bits() const { return bits & SIG_MASK; }
+  LIBC_INLINE constexpr StorageType exp_sig_bits() const {
+    return bits & EXP_SIG_MASK;
+  }
+
 private:
   LIBC_INLINE static constexpr StorageType bit_at(int position) {
     return StorageType(1) << position;
@@ -155,20 +219,6 @@ struct FPRepBase : public internal::FPLayout<fp_type> {
   LIBC_INLINE_VAR static constexpr StorageType FRACTION_MASK =
       mask_trailing_ones<StorageType, FRACTION_LEN>();
 
-  // If a number x is a NAN, then it is a quiet NAN if:
-  //   QUIET_NAN_MASK & bits(x) != 0
-  LIBC_INLINE_VAR static constexpr StorageType QUIET_NAN_MASK =
-      fp_type == FPType::X86_Binary80
-          ? bit_at(SIG_LEN - 1) | bit_at(SIG_LEN - 2) // 0b1100...
-          : bit_at(SIG_LEN - 1);                      // 0b1000...
-
-  // Mask to generate a default signaling NAN. Any NAN that is not
-  // a quiet NAN is considered a signaling NAN.
-  LIBC_INLINE_VAR static constexpr StorageType DEFAULT_SIGNALING_NAN =
-      fp_type == FPType::X86_Binary80
-          ? bit_at(SIG_LEN - 1) | bit_at(SIG_LEN - 3) // 0b1010...
-          : bit_at(SIG_LEN - 2);                      // 0b0100...
-
   // The floating point number representation as an unsigned integer.
   StorageType bits = 0;
 
@@ -220,6 +270,9 @@ struct FPRepBase : public internal::FPLayout<fp_type> {
   }
 
   LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
+  LIBC_INLINE constexpr StorageType set_uintval(StorageType value) {
+    return bits = (value & FP_MASK);
+  }
 
   LIBC_INLINE constexpr bool is_zero() const {
     return (bits & EXP_SIG_MASK) == 0;
@@ -241,6 +294,199 @@ template <FPType fp_type> struct FPRep : public FPRepBase<fp_type> {
   using UP::FRACTION_LEN;
   using UP::FRACTION_MASK;
   using UP::MANTISSA_PRECISION;
+
+protected:
+  using typename UP::Exp;
+  using typename UP::Sig;
+  using UP::encode;
+  using UP::exp_bits;
+  using UP::exp_sig_bits;
+  using UP::sig_bits;
+
+public:
+  LIBC_INLINE constexpr bool is_nan() const {
+    return exp_sig_bits() > encode(Exp::BITS_ALL_ONES, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE constexpr bool is_quiet_nan() const {
+    return exp_sig_bits() >= encode(Exp::BITS_ALL_ONES, Sig::MSB);
+  }
+  LIBC_INLINE constexpr bool is_signaling_nan() const {
+    return is_nan() && !is_quiet_nan();
+  }
+  LIBC_INLINE constexpr bool is_inf() const {
+    return exp_sig_bits() == encode(Exp::BITS_ALL_ONES, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE constexpr bool is_zero() const {
+    return exp_sig_bits() == encode(Exp::BITS_ALL_ZEROES, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE constexpr bool is_finite() const {
+    return exp_bits() != encode(Exp::BITS_ALL_ONES);
+  }
+  LIBC_INLINE
+  constexpr bool is_subnormal() const {
+    return exp_bits() == encode(Exp::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE constexpr bool is_normal() const {
+    return is_finite() && !is_subnormal();
+  }
+
+  LIBC_INLINE static constexpr StorageType zero(bool sign = false) {
+    return encode(sign, Exp::BITS_ALL_ZEROES, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE static constexpr StorageType one(bool sign = false) {
+    return encode(sign, Exp::ZERO, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE static constexpr StorageType min_subnormal(bool sign = false) {
+    return encode(sign, Exp::BITS_ALL_ZEROES, Sig::ONE);
+  }
+  LIBC_INLINE static constexpr StorageType max_subnormal(bool sign = false) {
+    return encode(sign, Exp::BITS_ALL_ZEROES, Sig::BITS_ALL_ONES);
+  }
+  LIBC_INLINE static constexpr StorageType min_normal(bool sign = false) {
+    return encode(sign, Exp::MIN, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE static constexpr StorageType max_normal(bool sign = false) {
+    return encode(sign, Exp::MAX, Sig::BITS_ALL_ONES);
+  }
+  LIBC_INLINE static constexpr StorageType inf(bool sign = false) {
+    return encode(sign, Exp::BITS_ALL_ONES, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE static constexpr StorageType build_nan(bool sign = false,
+                                                     StorageType v = 0) {
+    return encode(sign, Exp::BITS_ALL_ONES, (v ? Sig{v} : (Sig::MSB >> 1)));
+  }
+  LIBC_INLINE static constexpr StorageType build_quiet_nan(bool sign = false,
+                                                           StorageType v = 0) {
+    return encode(sign, Exp::BITS_ALL_ONES, Sig::MSB | Sig{v});
+  }
+
+  // The function return mantissa with the implicit bit set iff the current
+  // value is a valid normal number.
+  LIBC_INLINE constexpr StorageType get_explicit_mantissa() {
+    if (is_subnormal())
+      return sig_bits();
+    return (StorageType(1) << UP::SIG_LEN) | sig_bits();
+  }
+};
+
+// Specialization for the X86 Extended Precision type.
+template <>
+struct FPRep<FPType::X86_Binary80> : public FPRepBase<FPType::X86_Binary80> {
+  using UP = FPRepBase<FPType::X86_Binary80>;
+  using typename UP::StorageType;
+  using UP::FRACTION_LEN;
+  using UP::FRACTION_MASK;
+  using UP::MANTISSA_PRECISION;
+
+protected:
+  using typename UP::Exp;
+  using typename UP::Sig;
+  using UP::encode;
+
+public:
+  // The x86 80 bit float represents the leading digit of the mantissa
+  // explicitly. This is the mask for that bit.
+  static constexpr StorageType EXPLICIT_BIT_MASK = StorageType(1)
+                                                   << FRACTION_LEN;
+  // The X80 significand is made of an explicit bit and the fractional part.
+  static_assert((EXPLICIT_BIT_MASK & FRACTION_MASK) == 0,
+                "the explicit bit and the fractional part should not overlap");
+  static_assert((EXPLICIT_BIT_MASK | FRACTION_MASK) == SIG_MASK,
+                "the explicit bit and the fractional part should cover the "
+                "whole significand");
+
+  LIBC_INLINE constexpr bool is_nan() const {
+    // Most encoding forms from the table found in
+    // https://en.wikipedia.org/wiki/Extended_precision#x86_extended_precision_format
+    // are interpreted as NaN.
+    // More precisely :
+    // - Pseudo-Infinity
+    // - Pseudo Not a Number
+    // - Signalling Not a Number
+    // - Floating-point Indefinite
+    // - Quiet Not a Number
+    // - Unnormal
+    // This can be reduced to the following logic:
+    if (exp_bits() == encode(Exp::BITS_ALL_ONES))
+      return !is_inf();
+    if (exp_bits() != encode(Exp::BITS_ALL_ZEROES))
+      return (sig_bits() & encode(Sig::MSB)) == 0;
+    return false;
+  }
+  LIBC_INLINE constexpr bool is_quiet_nan() const {
+    return exp_sig_bits() >=
+           encode(Exp::BITS_ALL_ONES, Sig::MSB | (Sig::MSB >> 1));
+  }
+  LIBC_INLINE constexpr bool is_signaling_nan() const {
+    return is_nan() && !is_quiet_nan();
+  }
+  LIBC_INLINE constexpr bool is_inf() const {
+    return exp_sig_bits() == encode(Exp::BITS_ALL_ONES, Sig::MSB);
+  }
+  LIBC_INLINE constexpr bool is_zero() const {
+    return exp_sig_bits() == encode(Exp::BITS_ALL_ZEROES, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE constexpr bool is_finite() const {
+    return !is_inf() && !is_nan();
+  }
+  LIBC_INLINE
+  constexpr bool is_subnormal() const {
+    return exp_sig_bits() > encode(Exp::BITS_ALL_ZEROES, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE constexpr bool is_normal() const {
+    const auto exp = exp_bits();
+    if (exp == encode(Exp::BITS_ALL_ZEROES) ||
+        exp == encode(Exp::BITS_ALL_ONES))
+      return false;
+    return get_implicit_bit();
+  }
+
+  LIBC_INLINE static constexpr StorageType zero(bool sign = false) {
+    return encode(sign, Exp::BITS_ALL_ZEROES, Sig::BITS_ALL_ZEROES);
+  }
+  LIBC_INLINE static constexpr StorageType one(bool sign = false) {
+    return encode(sign, Exp::ZERO, Sig::MSB);
+  }
+  LIBC_INLINE static constexpr StorageType min_subnormal(bool sign = false) {
+    return encode(sign, Exp::BITS_ALL_ZEROES, Sig::ONE);
+  }
+  LIBC_INLINE static constexpr StorageType max_subnormal(bool sign = false) {
+    return encode(sign, Exp::BITS_ALL_ZEROES, Sig::BITS_ALL_ONES ^ Sig::MSB);
+  }
+  LIBC_INLINE static constexpr StorageType min_normal(bool sign = false) {
+    return encode(sign, Exp::MIN, Sig::MSB);
+  }
+  LIBC_INLINE static constexpr StorageType max_normal(bool sign = false) {
+    return encode(sign, Exp::MAX, Sig::BITS_ALL_ONES);
+  }
+  LIBC_INLINE static constexpr StorageType inf(bool sign = false) {
+    return encode(sign, Exp::BITS_ALL_ONES, Sig::MSB);
+  }
+  LIBC_INLINE static constexpr StorageType build_nan(bool sign = false,
+                                                     StorageType v = 0) {
+    return encode(sign, Exp::BITS_ALL_ONES,
+                  Sig::MSB | (v ? Sig{v} : (Sig::MSB >> 2)));
+  }
+  LIBC_INLINE static constexpr StorageType build_quiet_nan(bool sign = false,
+                                                           StorageType v = 0) {
+    return encode(sign, Exp::BITS_ALL_ONES,
+                  Sig::MSB | (Sig::MSB >> 1) | Sig{v});
+  }
+
+  LIBC_INLINE constexpr StorageType get_explicit_mantissa() const {
+    return sig_bits();
+  }
+
+  // The following functions are specific to this implementation.
+  // TODO: Remove if possible.
+  LIBC_INLINE constexpr bool get_implicit_bit() const {
+    return bits & EXPLICIT_BIT_MASK;
+  }
+
+  LIBC_INLINE constexpr void set_implicit_bit(bool implicitVal) {
+    if (get_implicit_bit() != implicitVal)
+      bits ^= EXPLICIT_BIT_MASK;
+  }
 };
 
 } // namespace internal
@@ -276,47 +522,29 @@ template <typename T> LIBC_INLINE static constexpr FPType get_fp_type() {
     static_assert(cpp::always_false<UnqualT>, "Unsupported type");
 }
 
-// A generic class to represent single precision, double precision, and quad
-// precision IEEE 754 floating point formats.
+// A generic class to represent floating point formats.
 // On most platforms, the 'float' type corresponds to single precision floating
 // point numbers, the 'double' type corresponds to double precision floating
 // point numers, and the 'long double' type corresponds to the quad precision
 // floating numbers. On x86 platforms however, the 'long double' type maps to
-// an x87 floating point format. This format is an IEEE 754 extension format.
-// It is handled as an explicit specialization of this class.
+// an x87 floating point format.
 template <typename T> struct FPBits : public internal::FPRep<get_fp_type<T>()> {
   static_assert(cpp::is_floating_point_v<T>,
                 "FPBits instantiated with invalid type.");
   using UP = internal::FPRep<get_fp_type<T>()>;
-
-private:
-  using UP::EXP_SIG_MASK;
-  using UP::QUIET_NAN_MASK;
-  using UP::SIG_LEN;
-  using UP::SIG_MASK;
-
-public:
+  using Rep = UP;
   using StorageType = typename UP::StorageType;
+
   using UP::bits;
-  using UP::EXP_BIAS;
   using UP::EXP_LEN;
-  using UP::EXP_MASK;
-  using UP::EXP_MASK_SHIFT;
-  using UP::FRACTION_LEN;
-  using UP::FRACTION_MASK;
-  using UP::SIGN_MASK;
-  using UP::TOTAL_LEN;
   using UP::UP;
 
-  using UP::get_biased_exponent;
-  using UP::is_zero;
   // Constants.
   static constexpr int MAX_BIASED_EXPONENT = (1 << EXP_LEN) - 1;
-  static constexpr StorageType MIN_SUBNORMAL = StorageType(1);
-  static constexpr StorageType MAX_SUBNORMAL = FRACTION_MASK;
-  static constexpr StorageType MIN_NORMAL = (StorageType(1) << FRACTION_LEN);
-  static constexpr StorageType MAX_NORMAL =
-      (StorageType(MAX_BIASED_EXPONENT - 1) << SIG_LEN) | SIG_MASK;
+  static constexpr StorageType MIN_NORMAL = UP::min_normal(false);
+  static constexpr StorageType MAX_NORMAL = UP::max_normal(false);
+  static constexpr StorageType MIN_SUBNORMAL = UP::min_subnormal(false);
+  static constexpr StorageType MAX_SUBNORMAL = UP::max_subnormal(false);
 
   // Constructors.
   LIBC_INLINE constexpr FPBits() = default;
@@ -338,88 +566,56 @@ template <typename T> struct FPBits : public internal::FPRep<get_fp_type<T>()> {
 
   LIBC_INLINE constexpr explicit operator T() const { return get_val(); }
 
-  // The function return mantissa with the implicit bit set iff the current
-  // value is a valid normal number.
-  LIBC_INLINE constexpr StorageType get_explicit_mantissa() {
-    return ((get_biased_exponent() > 0 && !is_inf_or_nan())
-                ? (FRACTION_MASK + 1)
-                : 0) |
-           (FRACTION_MASK & bits);
-  }
-
-  LIBC_INLINE constexpr bool is_inf() const {
-    return (bits & EXP_SIG_MASK) == EXP_MASK;
-  }
-
-  LIBC_INLINE constexpr bool is_nan() const {
-    return (bits & EXP_SIG_MASK) > EXP_MASK;
-  }
-
-  LIBC_INLINE constexpr bool is_quiet_nan() const {
-    return (bits & EXP_SIG_MASK) >= (EXP_MASK | QUIET_NAN_MASK);
-  }
-
-  LIBC_INLINE constexpr bool is_inf_or_nan() const {
-    return (bits & EXP_MASK) == EXP_MASK;
-  }
+  LIBC_INLINE constexpr bool is_inf_or_nan() const { return !UP::is_finite(); }
 
   LIBC_INLINE constexpr FPBits abs() const {
-    return FPBits(bits & EXP_SIG_MASK);
+    return FPBits(bits & UP::EXP_SIG_MASK);
   }
 
   // Methods below this are used by tests.
 
   LIBC_INLINE static constexpr T zero(bool sign = false) {
-    StorageType rep = (sign ? SIGN_MASK : StorageType(0)) // sign
-                      | 0                                 // exponent
-                      | 0;                                // mantissa
-    return FPBits(rep).get_val();
+    return FPBits(UP::zero(sign)).get_val();
   }
 
   LIBC_INLINE static constexpr T neg_zero() { return zero(true); }
 
   LIBC_INLINE static constexpr T inf(bool sign = false) {
-    StorageType rep = (sign ? SIGN_MASK : StorageType(0)) // sign
-                      | EXP_MASK                          // exponent
-                      | 0;                                // mantissa
-    return FPBits(rep).get_val();
+    return FPBits(UP::inf(sign)).get_val();
   }
 
   LIBC_INLINE static constexpr T neg_inf() { return inf(true); }
 
   LIBC_INLINE static constexpr T min_normal() {
-    return FPBits(MIN_NORMAL).get_val();
+    return FPBits(UP::min_normal(false)).get_val();
   }
 
   LIBC_INLINE static constexpr T max_normal() {
-    return FPBits(MAX_NORMAL).get_val();
+    return FPBits(UP::max_normal(false)).get_val();
   }
 
   LIBC_INLINE static constexpr T min_denormal() {
-    return FPBits(MIN_SUBNORMAL).get_val();
+    return FPBits(UP::min_subnormal(false)).get_val();
   }
 
   LIBC_INLINE static constexpr T max_denormal() {
-    return FPBits(MAX_SUBNORMAL).get_val();
+    return FPBits(UP::max_subnormal(false)).get_val();
   }
 
   LIBC_INLINE static constexpr T build_nan(StorageType v) {
-    StorageType rep = 0                      // sign
-                      | EXP_MASK             // exponent
-                      | (v & FRACTION_MASK); // mantissa
-    return FPBits(rep).get_val();
+    return FPBits(UP::build_nan(false, v)).get_val();
   }
 
   LIBC_INLINE static constexpr T build_quiet_nan(StorageType v) {
-    return build_nan(QUIET_NAN_MASK | v);
+    return FPBits(UP::build_quiet_nan(false, v)).get_val();
   }
 
   LIBC_INLINE static constexpr FPBits<T>
   create_value(bool sign, StorageType biased_exp, StorageType mantissa) {
-    StorageType rep = (sign ? SIGN_MASK : StorageType(0))           // sign
-                      | ((biased_exp << EXP_MASK_SHIFT) & EXP_MASK) // exponent
-                      | (mantissa & FRACTION_MASK);                 // mantissa
-    return FPBits(rep);
+    static_assert(get_fp_type<T>() != FPType::X86_Binary80,
+                  "This function is not tested for X86 Extended Precision");
+    return FPBits(UP::encode(sign, typename UP::Exp(biased_exp - UP::EXP_BIAS),
+                             typename UP::Sig(mantissa)));
   }
 
   // The function convert integer number and unbiased exponent to proper float
@@ -434,6 +630,8 @@ template <typename T> struct FPBits : public internal::FPRep<get_fp_type<T>()> {
   //   5) Number is unsigned, so the result can be only positive.
   LIBC_INLINE static constexpr FPBits<T> make_value(StorageType number,
                                                     int ep) {
+    static_assert(get_fp_type<T>() != FPType::X86_Binary80,
+                  "This function is not tested for X86 Extended Precision");
     FPBits<T> result;
     // offset: +1 for sign, but -1 for implicit first bit
     int lz = cpp::countl_zero(number) - EXP_LEN;
@@ -454,8 +652,4 @@ template <typename T> struct FPBits : public internal::FPRep<get_fp_type<T>()> {
 } // namespace fputil
 } // namespace LIBC_NAMESPACE
 
-#ifdef LIBC_LONG_DOUBLE_IS_X86_FLOAT80
-#include "x86_64/LongDoubleBits.h"
-#endif
-
 #endif // LLVM_LIBC_SRC___SUPPORT_FPUTIL_FPBITS_H
diff --git a/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h b/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
index 257c02e17d0045c..8815a18cfbc393b 100644
--- a/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
+++ b/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
@@ -131,7 +131,7 @@ LIBC_INLINE long double sqrt(long double x) {
     out.set_implicit_bit(1);
     out.set_mantissa((y & (ONE - 1)));
 
-    return out;
+    return out.get_val();
   }
 }
 #endif // LIBC_LONG_DOUBLE_IS_X86_FLOAT80
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
deleted file mode 100644
index c18abcee77ea50d..000000000000000
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ /dev/null
@@ -1,179 +0,0 @@
-//===-- Bit representation of x86 long double numbers -----------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC___SUPPORT_FPUTIL_X86_64_LONGDOUBLEBITS_H
-#define LLVM_LIBC_SRC___SUPPORT_FPUTIL_X86_64_LONGDOUBLEBITS_H
-
-#include "src/__support/CPP/bit.h"
-#include "src/__support/UInt128.h"
-#include "src/__support/common.h"
-#include "src/__support/macros/attributes.h" // LIBC_INLINE
-#include "src/__support/macros/properties/architectures.h"
-
-#if !defined(LIBC_TARGET_ARCH_IS_X86)
-#error "Invalid include"
-#endif
-
-#include "src...
[truncated]

Copy link

github-actions bot commented Jan 15, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@gchatelet
Copy link
Contributor Author

gchatelet commented Jan 15, 2024

@legrosbuffle The patch is split in individual commits for "easier" review.

return is_finite() && !is_subnormal();
}

LIBC_INLINE static constexpr StorageType zero(bool sign = false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit, maybe in a different patch] What about making sign an enum ? I remember not too long ago that a bool parameter meant "toggle the sign". Maybe enum class Signum { POSITIVE, NEGATIVE } ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes absolutely this is on my TODO list. It has consequences beyond just a few files so I'm keeping this for a later patch.

BITS_ALL_ONES = INF,
};

enum class Sig : StorageType {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sig is not very self-explanatory. Significand is not taken. Or at least add a comment.

Actually Sig does not represent any significand value, but only specific ones.

// A set of specific significand values. Closed under operations or, xor, shift right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went for Significand although it's a bit verbose.

I also clarified that enum is used as a typed integer to prevent mixing and matching between parts of the representation.

I've also turned Exp into BiasedExponent as it's more about the bit pattern than the actual semantics. Hopefully, this is now clearer.

libc/src/__support/FPUtil/FPBits.h Outdated Show resolved Hide resolved
libc/src/__support/FPUtil/FPBits.h Outdated Show resolved Hide resolved
libc/src/__support/FPUtil/FPBits.h Outdated Show resolved Hide resolved
libc/src/__support/FPUtil/FPBits.h Outdated Show resolved Hide resolved
}

LIBC_INLINE static constexpr StorageType encode(Exp exp) {
return storage_cast(static_cast<int32_t>(exp) + EXP_BIAS) << SIG_LEN;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add some asserts there ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have assert in libc, only static_asserts. The behavior is to truncate the value if it exceeds its maximum value. I've made it clearer in the documentation of the enums.

libc/src/__support/FPUtil/FPBits.h Show resolved Hide resolved
libc/src/__support/FPUtil/FPBits.h Outdated Show resolved Hide resolved
@gchatelet gchatelet merged commit fdbf255 into llvm:main Jan 16, 2024
4 checks passed
@gchatelet gchatelet deleted the refactor_fpbits branch January 16, 2024 14:41
@nickdesaulniers
Copy link
Member

ah, sorry, I had the arm32 build bot offline this past week trying to resolve the last of the -Werror issues there (finally fixed by f1f1875). Though it seems that this change breaks the build for 32b arm preventing me from re-enabling the build bot. (I've pinged you on chat most of the error logs, can repost them here if needed). PTAL

// This can be reduced to the following logic:
if (exp_bits() == encode(BiasedExponent::BITS_ALL_ONES))
return !is_inf();
if (exp_bits() != encode(BiasedExponent::BITS_ALL_ZEROES))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 32b arm build is failing here:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:474:23: error: no matching function for call to 'encode'
    if (exp_bits() != encode(BiasedExponent::BITS_ALL_ZEROES))
                      ^~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:234:44: note: candidate function not viable: no known conversion from '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::BiasedExponent' to '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand' for 1st argument
  LIBC_INLINE static constexpr StorageType encode(Significand value) {
                                           ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:238:44: note: candidate function not viable: requires 2 arguments, but 1 was provided
  LIBC_INLINE static constexpr StorageType encode(BiasedExponent exp,
                                           ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:243:44: note: candidate function not viable: requires 3 arguments, but 1 was provided
  LIBC_INLINE static constexpr StorageType encode(bool sign, BiasedExponent exp,
                                           ^


template <typename T>
LIBC_INLINE static constexpr auto storage_cast(T value) {
return static_cast<StorageType>(value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure here:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:214:12: error: no matching conversion for static_cast from '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::BiasedExponent' to '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::StorageType' (aka 'BigInt<128U, false>')
    return static_cast<StorageType>(value);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:231:13: note: in instantiation of function template specialization '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::storage_cast<__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::BiasedExponent>' requested here
    return (storage_cast(exp) << SIG_LEN) & EXP_MASK;
            ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:472:23: note: in instantiation of member function '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::encode' requested here
    if (exp_bits() == encode(BiasedExponent::BITS_ALL_ONES))
                      ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:44:25: note: candidate constructor not viable: no known conversion from '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::BiasedExponent' to 'const BigInt<128U, false>' for 1st argument
  LIBC_INLINE constexpr BigInt(const BigInt<Bits, Signed> &other) = default;
                        ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:100:34: note: candidate constructor not viable: no known conversion from '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::BiasedExponent' to 'const cpp::array<uint64_t, WORDCOUNT>' (aka 'const array<unsigned long long, WORDCOUNT>') for 1st argument
  LIBC_INLINE constexpr explicit BigInt(
                                 ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:47:25: note: candidate template ignored: could not match 'BigInt<OtherBits, OtherSigned>' against '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::BiasedExponent'
  LIBC_INLINE constexpr BigInt(const BigInt<OtherBits, OtherSigned> &other) {
                        ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:67:25: note: candidate template ignored: could not match 'uint64_t[N]' (aka 'unsigned long long[N]') against '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::BiasedExponent'
  LIBC_INLINE constexpr BigInt(const uint64_t (&nums)[N]) {
                        ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:81:25: note: candidate template ignored: substitution failure [with T = __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::BiasedExponent]: implicit instantiation of undefined template '__llvm_libc_18_0_0_git::cpp::enable_if<false>'
  LIBC_INLINE constexpr BigInt(T v) {
                        ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:42:25: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
  LIBC_INLINE constexpr BigInt() = default;
                        ^

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copying all of the errors I'm seeing.

// as they are in the range [BITS_ALL_ZEROES, BITS_ALL_ONES].
// Note that the semantics of the Significand are implementation dependent.
// Values greater than BITS_ALL_ONES are truncated.
enum class Significand : StorageType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure here:

In file included from /llvm/ndesaulniers/llvm-project/libc/src/fenv/fegetround.cpp:10:
In file included from /llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FEnvImpl.h:33:
In file included from /llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/arm/FEnvImpl.h:12:
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:203:28: error: non-integral type '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::StorageType' (aka 'BigInt<128U, false>') is an invalid underlying type
  enum class Significand : StorageType {
                           ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:436:45: note: in instantiation of template class '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>' requested here
struct FPRep<FPType::X86_Binary80> : public FPRepBase<FPType::X86_Binary80> {
                                            ^

enum class Significand : StorageType {
ZERO = 0,
LSB = 1,
MSB = bit_at(SIG_LEN - 1),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure here:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:206:11: error: no viable conversion from '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::StorageType' (aka 'BigInt<128U, false>') to 'int'
    MSB = bit_at(SIG_LEN - 1),
          ^~~~~~~~~~~~~~~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:475:35: note: in instantiation of enumeration '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand' requested here
      return (sig_bits() & encode(Significand::MSB)) == 0;
                                  ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:106:56: note: explicit conversion function is not a candidate
  template <typename T> LIBC_INLINE constexpr explicit operator T() const {
                                                       ^

MSB = bit_at(SIG_LEN - 1),
// Aliases
BITS_ALL_ZEROES = ZERO,
BITS_ALL_ONES = SIG_MASK,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:209:21: error: no viable conversion from 'const __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::StorageType' (aka 'const BigInt<128U, false>') to 'int'
    BITS_ALL_ONES = SIG_MASK,
                    ^~~~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:106:56: note: explicit conversion function is not a candidate
  template <typename T> LIBC_INLINE constexpr explicit operator T() const {
                                                       ^

}
LIBC_INLINE friend constexpr Significand operator>>(const Significand a,
int shift) {
return Significand{storage_cast(storage_cast(a) >> shift)};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:227:37: error: no matching function for call to 'storage_cast'
    return Significand{storage_cast(storage_cast(a) >> shift)};
                                    ^~~~~~~~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:480:74: note: in instantiation of member function '__llvm_libc_18_0_0_git::fputil::operator>>' requested here
                                    Significand::MSB | (Significand::MSB >> 1));
                                                                         ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:213:37: note: candidate template ignored: substitution failure [with T = __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand]
  LIBC_INLINE static constexpr auto storage_cast(T value) {
                                    ^


LIBC_INLINE friend constexpr Significand operator|(const Significand a,
const Significand b) {
return Significand{storage_cast(storage_cast(a) | storage_cast(b))};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure here:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:219:37: error: no matching function for call to 'storage_cast'
    return Significand{storage_cast(storage_cast(a) | storage_cast(b))};
                                    ^~~~~~~~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:480:54: note: in instantiation of member function '__llvm_libc_18_0_0_git::fputil::operator|' requested here
                                    Significand::MSB | (Significand::MSB >> 1));
                                                     ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:213:37: note: candidate template ignored: substitution failure [with T = __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand]
  LIBC_INLINE static constexpr auto storage_cast(T value) {
                                    ^


LIBC_INLINE static constexpr StorageType encode(BiasedExponent exp,
Significand sig) {
return encode(exp) | encode(sig);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure here:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:240:12: error: no matching function for call to 'encode'
    return encode(exp) | encode(sig);
           ^~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:479:30: note: in instantiation of member function '__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::encode' requested here
    return exp_sig_bits() >= encode(BiasedExponent::BITS_ALL_ONES,
                             ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:238:44: note: candidate function not viable: requires 2 arguments, but 1 was provided
  LIBC_INLINE static constexpr StorageType encode(BiasedExponent exp,
                                           ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:243:44: note: candidate function not viable: requires 3 arguments, but 1 was provided
  LIBC_INLINE static constexpr StorageType encode(bool sign, BiasedExponent exp,
                                           ^

Comment on lines +503 to +504
if (exp == encode(BiasedExponent::BITS_ALL_ZEROES) ||
exp == encode(BiasedExponent::BITS_ALL_ONES))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failures here:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:503:16: error: no matching function for call to 'encode'
    if (exp == encode(BiasedExponent::BITS_ALL_ZEROES) ||
               ^~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:238:44: note: candidate function not viable: requires 2 arguments, but 1 was provided
  LIBC_INLINE static constexpr StorageType encode(BiasedExponent exp,
                                           ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:243:44: note: candidate function not viable: requires 3 arguments, but 1 was provided
  LIBC_INLINE static constexpr StorageType encode(bool sign, BiasedExponent exp,
                                           ^

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:504:16: error: no matching function for call to 'encode'
        exp == encode(BiasedExponent::BITS_ALL_ONES))
               ^~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:238:44: note: candidate function not viable: requires 2 arguments, but 1 was provided
  LIBC_INLINE static constexpr StorageType encode(BiasedExponent exp,
                                           ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:243:44: note: candidate function not viable: requires 3 arguments, but 1 was provided
  LIBC_INLINE static constexpr StorageType encode(bool sign, BiasedExponent exp,
                                           ^

}
LIBC_INLINE friend constexpr Significand operator^(const Significand a,
const Significand b) {
return Significand{storage_cast(storage_cast(a) ^ storage_cast(b))};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure:

/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:223:37: error: no matching function for call to 'storage_cast'
    return Significand{storage_cast(storage_cast(a) ^ storage_cast(b))};
                                    ^~~~~~~~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:520:46: note: in instantiation of member function '__llvm_libc_18_0_0_git::fputil::operator^' requested here
                  Significand::BITS_ALL_ONES ^ Significand::MSB);
                                             ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:213:37: note: candidate template ignored: substitution failure [with T = __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand]
  LIBC_INLINE static constexpr auto storage_cast(T value) {
                                    ^

// The following functions are specific to FPRep<FPType::X86_Binary80>.
// TODO: Remove if possible.
LIBC_INLINE constexpr bool get_implicit_bit() const {
return bits & EXPLICIT_BIT_MASK;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure here:

In file included from /llvm/ndesaulniers/llvm-project/libc/src/fenv/fegetround.cpp:10:
In file included from /llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FEnvImpl.h:33:
In file included from /llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/arm/FEnvImpl.h:12:
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:550:12: error: no viable conversion from returned value of type 'BigInt<128U, false>' to function return type 'bool'
    return bits & EXPLICIT_BIT_MASK;
           ^~~~~~~~~~~~~~~~~~~~~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:106:56: note: explicit conversion function is not a candidate
  template <typename T> LIBC_INLINE constexpr explicit operator T() const {
                                                       ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:135:34: note: explicit conversion function is not a candidate
  LIBC_INLINE constexpr explicit operator bool() const { return !is_zero(); }
                                 ^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding a static_cast<bool>() to the expression helps. It looks like BigInt's operator bool is marked explicit, so it can only be used for explicit casts and not implicit casts IIUC).

StorageType v = 0) {
return encode(sign, BiasedExponent::BITS_ALL_ONES,
Significand::MSB |
(v ? Significand{v} : (Significand::MSB >> 2)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32b arm build bot failure here:

In file included from /llvm/ndesaulniers/llvm-project/libc/src/fenv/fegetround.cpp:10:
In file included from /llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FEnvImpl.h:33:
In file included from /llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/arm/FEnvImpl.h:12:
In file included from /llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:14:
In file included from /llvm/ndesaulniers/llvm-project/libc/src/__support/UInt128.h:12:
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:107:12: error: no matching member function for call to 'to'
    return to<T>();
           ^~~~~
/llvm/ndesaulniers/llvm-project/libc/src/__support/FPUtil/FPBits.h:535:40: note: in instantiation of function template specialization '__llvm_libc_18_0_0_git::cpp::BigInt<128, false>::operator __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand<__llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand>' requested here
                      (v ? Significand{v} : (Significand::MSB >> 2)));
                                       ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:113:3: note: candidate template ignored: substitution failure [with T = __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand]: implicit instantiation of undefined template '__llvm_libc_18_0_0_git::cpp::enable_if<false, __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand>'
  to() const {
  ^
/llvm/ndesaulniers/llvm-project/libc/src/__support/UInt.h:119:3: note: candidate template ignored: substitution failure [with T = __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand]: implicit instantiation of undefined template '__llvm_libc_18_0_0_git::cpp::enable_if<false, __llvm_libc_18_0_0_git::fputil::FPRepBase<__llvm_libc_18_0_0_git::fputil::FPType::X86_Binary80>::Significand>'
  to() const {
  ^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems resolved if we remove the cpp::is_integral_v<T> check from BigInt::to (added in 1ee6a1e). Probably because Significand is sizeof(4) but probably isn't is_integral_v.

nickdesaulniers added a commit to nickdesaulniers/llvm-project that referenced this pull request Jan 16, 2024
…ization (llvm#78192)"

This reverts commit fdbf255.

Causes build breakage on 32b arm (see reports:
llvm#78192).

These are reproducible for the 32b arm baremetal target on x86 hosts as well.
@nickdesaulniers
Copy link
Member

sorry, I wasn't able to figure out how to fix-forward these failures quickly. Suggest revert in #78329 so that we can get the 32b arm build bot back online

nickdesaulniers added a commit that referenced this pull request Jan 16, 2024
…ization (#78192)" (#78329)

This reverts commit fdbf255.

Causes build breakage on 32b arm (see reports:
#78192).

These are reproducible for the 32b arm baremetal target on x86 hosts as
well.
@nickdesaulniers
Copy link
Member

Backed out in 337b771. I'll stand up the 32b arm build bot to help catch these kinds of things, ASAP. Let me know if you need help testing on the 32b build bot (or the baremetal 32b arm config should work on an x86 host for reproducing the errors). I suspect we may need to improve the BigInt related cast operators added in 1ee6a1e.

@gchatelet
Copy link
Contributor Author

Thank you so much for the detailed investigation @nickdesaulniers !
I can log on the build bot and start fixing the patch.

gchatelet added a commit that referenced this pull request Jan 17, 2024
…ization (#78447)

- [reland] #78192
- Make the implementation work when `__uint128_t` is not available on
the plaftorm.
gchatelet added a commit to gchatelet/llvm-project that referenced this pull request Jan 17, 2024
…ization (llvm#78447)

- [reland] llvm#78192
- Make the implementation work when `__uint128_t` is not available on
the plaftorm.
gchatelet added a commit that referenced this pull request Jan 17, 2024
…ization (#78465)

- [reland] #78192 
- [reland] #78447 
- Turn `as` static function into a `to_storage_type` member function.
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
…ization (llvm#78447)

- [reland] llvm#78192
- Make the implementation work when `__uint128_t` is not available on
the plaftorm.
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
…ization (llvm#78465)

- [reland] llvm#78192 
- [reland] llvm#78447 
- Turn `as` static function into a `to_storage_type` member function.
gnoliyil pushed a commit to gnoliyil/fuchsia that referenced this pull request Jan 27, 2024
…Bits and remove LongDoubleBits specialization (#78192)" (#78329)

This reverts commit fdbf255c96cb18bc1fb08fa7264446bcbbd5fbd0.

Causes build breakage on 32b arm (see reports:
llvm/llvm-project#78192).

These are reproducible for the 32b arm baremetal target on x86 hosts as
well.

GitOrigin-RevId: 1d83f653229de230bfb426470b36b4b6951266c2
Original-Revision: 7b791dfc058d9d911125b1700ddf499af33ac136
Roller-URL: https://ci.chromium.org/b/8758699903148777393
CQ-Do-Not-Cancel-Tryjobs: true
Change-Id: I57642257170918cc2fe7f557c2c98cc4f8530001
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/974927
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…lvm#78192)

This patch removes the `FPBits` specialization for x86 Extended Precision by moving it up to `FPRep`.
It also introduces enums (`Exponent`, `BiasedExponent` and `Significand`) to represent the exponent and significant parts of the floating point numbers. These enums are used to construct and observe floating point representations.

Additionally, we remove `LongDoubleBits.h` that is now unnecessary.
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…ization (llvm#78192)" (llvm#78329)

This reverts commit fdbf255.

Causes build breakage on 32b arm (see reports:
llvm#78192).

These are reproducible for the 32b arm baremetal target on x86 hosts as
well.
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.

None yet

4 participants