[libc] [llvm] Revert "[reland][libc][NFC] Refactor FPBits and remove LongDoubleBits specialization" (PR #78457)

via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 17 07:51:22 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

<details>
<summary>Changes</summary>

Reverts llvm/llvm-project#<!-- -->78447
This broke the gcc buildbot.

---

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


9 Files Affected:

- (modified) libc/src/__support/FPUtil/FPBits.h (+95-392) 
- (modified) libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h (+1-1) 
- (added) libc/src/__support/FPUtil/x86_64/LongDoubleBits.h (+179) 
- (modified) libc/src/__support/FPUtil/x86_64/NextAfterLongDouble.h (+4-4) 
- (modified) libc/src/__support/UInt.h (+1-1) 
- (modified) libc/test/src/__support/FPUtil/fpbits_test.cpp (-213) 
- (modified) libc/test/utils/FPUtil/x86_long_double_test.cpp (+6-6) 
- (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+1) 
- (removed) utils/bazel/llvm-project-overlay/libc/test/src/__support/FPUtil/BUILD.bazel (-42) 


``````````diff
diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index f67e62add2e28f..93e32ba7cc9415 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -31,40 +31,6 @@ enum class FPType {
   X86_Binary80,
 };
 
-// The classes hierarchy is as follows:
-//
-//             ┌───────────────────┐
-//             │ FPLayout<FPType>  │
-//             └─────────▲─────────┘
-//                       │
-//             ┌─────────┴─────────┐
-//             │ FPRepBase<FPType> │
-//             └─────────▲─────────┘
-//                       │
-//          ┌────────────┴─────────────┐
-//          │                          │
-// ┌────────┴──────┐     ┌─────────────┴──────────────┐
-// │ FPRep<FPType> │     │ FPRep<FPType::X86_Binary80 │
-// └────────▲──────┘     └─────────────▲──────────────┘
-//          │                          │
-//          └────────────┬─────────────┘
-//                       │
-//                 ┌─────┴─────┐
-//                 │ FPBits<T> │
-//                 └───────────┘
-//
-// - 'FPLayout' defines only a few constants, namely the 'StorageType' and the
-// length of the sign, the exponent and significand parts.
-// - 'FPRepBase' builds more constants on top of those from 'FPLayout' like
-// exponent bias, shifts and masks. It also defines tools to assemble or test
-// these parts.
-// - 'FPRep' defines functions to interact with the floating point
-// representation. The default implementation is the one for 'IEEE754', a
-// specialization is provided for X86 Extended Precision that has a different
-// encoding.
-// - 'FPBits' is templated on the platform floating point types. Contrary to
-// 'FPRep' that is platform agnostic 'FPBits' is architecture dependent.
-
 namespace internal {
 
 // Defines the layout (sign, exponent, significand) of a floating point type in
@@ -166,127 +132,11 @@ 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:
+private:
   LIBC_INLINE static constexpr StorageType bit_at(int position) {
     return StorageType(1) << position;
   }
 
-  // A stongly typed integer that prevents mixing and matching integers with
-  // different semantics.
-  template <typename T> struct TypedInt {
-    using value_type = T;
-    LIBC_INLINE constexpr explicit TypedInt(T value) : value(value) {}
-    LIBC_INLINE constexpr TypedInt(const TypedInt &value) = default;
-
-    LIBC_INLINE constexpr explicit operator T() const { return value; }
-
-  private:
-    T value;
-  };
-
-  // Allows explicit casting to a different type.
-  template <typename To, typename T>
-  LIBC_INLINE static constexpr To as(TypedInt<T> typed_int) {
-    return To(static_cast<T>(typed_int));
-  }
-
-  // An opaque type to store a floating point exponent.
-  // We define special values but it is valid to create arbitrary values as long
-  // as they are in the range [MIN, MAX].
-  struct Exponent : public TypedInt<int32_t> {
-    using UP = TypedInt<int32_t>;
-    using UP::UP;
-    LIBC_INLINE
-    static constexpr auto MIN() { return Exponent(1 - EXP_BIAS); }
-    LIBC_INLINE static constexpr auto ZERO() { return Exponent(0); }
-    LIBC_INLINE static constexpr auto MAX() { return Exponent(EXP_BIAS); }
-  };
-
-  // An opaque type to store a floating point biased exponent.
-  // We define special values but it is valid to create arbitrary values as long
-  // as they are in the range [BITS_ALL_ZEROES, BITS_ALL_ONES].
-  // Values greater than BITS_ALL_ONES are truncated.
-  struct BiasedExponent : public TypedInt<uint32_t> {
-    using UP = TypedInt<uint32_t>;
-    using UP::UP;
-
-    LIBC_INLINE constexpr BiasedExponent(Exponent exp)
-        : UP(static_cast<int32_t>(exp) + EXP_BIAS) {}
-    // The exponent value for denormal numbers.
-    LIBC_INLINE static constexpr auto BITS_ALL_ZEROES() {
-      return BiasedExponent(uint32_t(0));
-    }
-    // The exponent value for infinity.
-    LIBC_INLINE static constexpr auto BITS_ALL_ONES() {
-      return BiasedExponent(uint32_t(2 * EXP_BIAS + 1));
-    }
-  };
-
-  // An opaque type to store a floating point significand.
-  // We define special values but it is valid to create arbitrary values as long
-  // 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.
-  struct Significand : public TypedInt<StorageType> {
-    using UP = TypedInt<StorageType>;
-    using UP::UP;
-
-    LIBC_INLINE friend constexpr Significand operator|(const Significand a,
-                                                       const Significand b) {
-      return Significand(StorageType(as<StorageType>(a) | as<StorageType>(b)));
-    }
-    LIBC_INLINE friend constexpr Significand operator^(const Significand a,
-                                                       const Significand b) {
-      return Significand(StorageType(as<StorageType>(a) ^ as<StorageType>(b)));
-    }
-    LIBC_INLINE friend constexpr Significand operator>>(const Significand a,
-                                                        int shift) {
-      return Significand(StorageType(as<StorageType>(a) >> shift));
-    }
-
-    LIBC_INLINE static constexpr auto ZERO() {
-      return Significand(StorageType(0));
-    }
-    LIBC_INLINE static constexpr auto LSB() {
-      return Significand(StorageType(1));
-    }
-    LIBC_INLINE static constexpr auto MSB() {
-      return Significand(StorageType(bit_at(SIG_LEN - 1)));
-    }
-    // Aliases
-    LIBC_INLINE static constexpr auto BITS_ALL_ZEROES() { return ZERO(); }
-    LIBC_INLINE static constexpr auto BITS_ALL_ONES() {
-      return Significand(SIG_MASK);
-    }
-  };
-
-  LIBC_INLINE static constexpr StorageType encode(BiasedExponent exp) {
-    return (as<StorageType>(exp) << SIG_LEN) & EXP_MASK;
-  }
-
-  LIBC_INLINE static constexpr StorageType encode(Significand value) {
-    return as<StorageType>(value) & SIG_MASK;
-  }
-
-  LIBC_INLINE static constexpr StorageType encode(BiasedExponent exp,
-                                                  Significand sig) {
-    return encode(exp) | encode(sig);
-  }
-
-  LIBC_INLINE static constexpr StorageType encode(bool sign, BiasedExponent exp,
-                                                  Significand 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:
   // Merge bits from 'a' and 'b' values according to 'mask'.
   // Use 'a' bits when corresponding 'mask' bits are zeroes and 'b' bits when
   // corresponding bits are ones.
@@ -305,6 +155,20 @@ 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;
 
@@ -356,9 +220,6 @@ struct FPRepBase : public internal::FPLayout<fp_type> {
   }
 
   LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
-  LIBC_INLINE constexpr void set_uintval(StorageType value) {
-    bits = (value & FP_MASK);
-  }
 
   LIBC_INLINE constexpr bool is_zero() const {
     return (bits & EXP_SIG_MASK) == 0;
@@ -380,214 +241,6 @@ 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::BiasedExponent;
-  using typename UP::Exponent;
-  using typename UP::Significand;
-  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(BiasedExponent::BITS_ALL_ONES(), Significand::ZERO());
-  }
-  LIBC_INLINE constexpr bool is_quiet_nan() const {
-    return exp_sig_bits() >=
-           encode(BiasedExponent::BITS_ALL_ONES(), Significand::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(BiasedExponent::BITS_ALL_ONES(), Significand::ZERO());
-  }
-  LIBC_INLINE constexpr bool is_zero() const {
-    return exp_sig_bits() ==
-           encode(BiasedExponent::BITS_ALL_ZEROES(), Significand::ZERO());
-  }
-  LIBC_INLINE constexpr bool is_finite() const {
-    return exp_bits() != encode(BiasedExponent::BITS_ALL_ONES());
-  }
-  LIBC_INLINE
-  constexpr bool is_subnormal() const {
-    return exp_bits() == encode(BiasedExponent::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, BiasedExponent::BITS_ALL_ZEROES(), Significand::ZERO());
-  }
-  LIBC_INLINE static constexpr StorageType one(bool sign = false) {
-    return encode(sign, Exponent::ZERO(), Significand::ZERO());
-  }
-  LIBC_INLINE static constexpr StorageType min_subnormal(bool sign = false) {
-    return encode(sign, BiasedExponent::BITS_ALL_ZEROES(), Significand::LSB());
-  }
-  LIBC_INLINE static constexpr StorageType max_subnormal(bool sign = false) {
-    return encode(sign, BiasedExponent::BITS_ALL_ZEROES(),
-                  Significand::BITS_ALL_ONES());
-  }
-  LIBC_INLINE static constexpr StorageType min_normal(bool sign = false) {
-    return encode(sign, Exponent::MIN(), Significand::ZERO());
-  }
-  LIBC_INLINE static constexpr StorageType max_normal(bool sign = false) {
-    return encode(sign, Exponent::MAX(), Significand::BITS_ALL_ONES());
-  }
-  LIBC_INLINE static constexpr StorageType inf(bool sign = false) {
-    return encode(sign, BiasedExponent::BITS_ALL_ONES(), Significand::ZERO());
-  }
-  LIBC_INLINE static constexpr StorageType build_nan(bool sign = false,
-                                                     StorageType v = 0) {
-    return encode(sign, BiasedExponent::BITS_ALL_ONES(),
-                  (v ? Significand(v) : (Significand::MSB() >> 1)));
-  }
-  LIBC_INLINE static constexpr StorageType build_quiet_nan(bool sign = false,
-                                                           StorageType v = 0) {
-    return encode(sign, BiasedExponent::BITS_ALL_ONES(),
-                  Significand::MSB() | Significand(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::BiasedExponent;
-  using typename UP::Significand;
-  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(BiasedExponent::BITS_ALL_ONES()))
-      return !is_inf();
-    if (exp_bits() != encode(BiasedExponent::BITS_ALL_ZEROES()))
-      return (sig_bits() & encode(Significand::MSB())) == 0;
-    return false;
-  }
-  LIBC_INLINE constexpr bool is_quiet_nan() const {
-    return exp_sig_bits() >=
-           encode(BiasedExponent::BITS_ALL_ONES(),
-                  Significand::MSB() | (Significand::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(BiasedExponent::BITS_ALL_ONES(), Significand::MSB());
-  }
-  LIBC_INLINE constexpr bool is_zero() const {
-    return exp_sig_bits() ==
-           encode(BiasedExponent::BITS_ALL_ZEROES(), Significand::ZERO());
-  }
-  LIBC_INLINE constexpr bool is_finite() const {
-    return !is_inf() && !is_nan();
-  }
-  LIBC_INLINE
-  constexpr bool is_subnormal() const {
-    return exp_sig_bits() >
-           encode(BiasedExponent::BITS_ALL_ZEROES(), Significand::ZERO());
-  }
-  LIBC_INLINE constexpr bool is_normal() const {
-    const auto exp = exp_bits();
-    if (exp == encode(BiasedExponent::BITS_ALL_ZEROES()) ||
-        exp == encode(BiasedExponent::BITS_ALL_ONES()))
-      return false;
-    return get_implicit_bit();
-  }
-
-  LIBC_INLINE static constexpr StorageType zero(bool sign = false) {
-    return encode(sign, BiasedExponent::BITS_ALL_ZEROES(), Significand::ZERO());
-  }
-  LIBC_INLINE static constexpr StorageType one(bool sign = false) {
-    return encode(sign, Exponent::ZERO(), Significand::MSB());
-  }
-  LIBC_INLINE static constexpr StorageType min_subnormal(bool sign = false) {
-    return encode(sign, BiasedExponent::BITS_ALL_ZEROES(), Significand::LSB());
-  }
-  LIBC_INLINE static constexpr StorageType max_subnormal(bool sign = false) {
-    return encode(sign, BiasedExponent::BITS_ALL_ZEROES(),
-                  Significand::BITS_ALL_ONES() ^ Significand::MSB());
-  }
-  LIBC_INLINE static constexpr StorageType min_normal(bool sign = false) {
-    return encode(sign, Exponent::MIN(), Significand::MSB());
-  }
-  LIBC_INLINE static constexpr StorageType max_normal(bool sign = false) {
-    return encode(sign, Exponent::MAX(), Significand::BITS_ALL_ONES());
-  }
-  LIBC_INLINE static constexpr StorageType inf(bool sign = false) {
-    return encode(sign, BiasedExponent::BITS_ALL_ONES(), Significand::MSB());
-  }
-  LIBC_INLINE static constexpr StorageType build_nan(bool sign = false,
-                                                     StorageType v = 0) {
-    return encode(sign, BiasedExponent::BITS_ALL_ONES(),
-                  Significand::MSB() |
-                      (v ? Significand(v) : (Significand::MSB() >> 2)));
-  }
-  LIBC_INLINE static constexpr StorageType build_quiet_nan(bool sign = false,
-                                                           StorageType v = 0) {
-    return encode(sign, BiasedExponent::BITS_ALL_ONES(),
-                  Significand::MSB() | (Significand::MSB() >> 1) |
-                      Significand(v));
-  }
-
-  LIBC_INLINE constexpr StorageType get_explicit_mantissa() const {
-    return sig_bits();
-  }
-
-  // The following functions are specific to FPRep<FPType::X86_Binary80>.
-  // TODO: Remove if possible.
-  LIBC_INLINE constexpr bool get_implicit_bit() const {
-    return static_cast<bool>(bits & EXPLICIT_BIT_MASK);
-  }
-
-  LIBC_INLINE constexpr void set_implicit_bit(bool implicitVal) {
-    if (get_implicit_bit() != implicitVal)
-      bits ^= EXPLICIT_BIT_MASK;
-  }
 };
 
 } // namespace internal
@@ -623,29 +276,47 @@ template <typename T> LIBC_INLINE static constexpr FPType get_fp_type() {
     static_assert(cpp::always_false<UnqualT>, "Unsupported type");
 }
 
-// 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.
+// A generic class to represent single precision, double precision, and quad
+// precision IEEE 754 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.
 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>()>;
-  using Rep = UP;
-  using StorageType = typename UP::StorageType;
 
+private:
+  using UP::EXP_SIG_MASK;
+  using UP::QUIET_NAN_MASK;
+  using UP::SIG_LEN;
+  using UP::SIG_MASK;
+
+public:
+  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_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);
+  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;
 
   // Constructors.
   LIBC_INLINE constexpr FPBits() = default;
@@ -667,58 +338,88 @@ template <typename T> struct FPBits : public internal::FPRep<get_fp_type<T>()> {
 
   LIBC_INLINE constexpr explicit operator T() const { return get_val(); }
 
-  LIBC_INLINE constexpr bool is_inf_or_nan() const { return !UP::is_finite(); }
+  // 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...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/78457


More information about the llvm-commits mailing list