[libc-commits] [libc] [libc][NFC] Unify `FPBits` implementations (PR #76033)

Guillaume Chatelet via libc-commits libc-commits at lists.llvm.org
Thu Dec 21 04:12:32 PST 2023


https://github.com/gchatelet updated https://github.com/llvm/llvm-project/pull/76033

>From f802c4010c62702a4afa28597ac90b697afc681f Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:03:59 +0000
Subject: [PATCH 01/10] [libc][NFC] Unify `FPBits` implementations

`FPBits` is currently implemented as a general case and a specialization for `long double` when `long double` is x86 80-bit extended precision. This patch is a first of a series to provide a single implementation based on `FPType` instead of the C++ float type (which implementation is architecture dependent).
---
 libc/src/__support/FPUtil/FPBits.h            | 86 ++++++++++++-------
 .../__support/FPUtil/x86_64/LongDoubleBits.h  | 44 ++++------
 2 files changed, 74 insertions(+), 56 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 37e2820eab8553..bc79930797b691 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -20,41 +20,36 @@
 namespace LIBC_NAMESPACE {
 namespace fputil {
 
-// 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 : private FloatProperties<T> {
-  static_assert(cpp::is_floating_point_v<T>,
-                "FPBits instantiated with invalid type.");
-  using typename FloatProperties<T>::StorageType;
-  using FloatProperties<T>::TOTAL_LEN;
-
-private:
-  using FloatProperties<T>::EXP_SIG_MASK;
+namespace internal {
 
-public:
-  using FloatProperties<T>::EXP_MASK;
-  using FloatProperties<T>::EXP_BIAS;
-  using FloatProperties<T>::EXP_LEN;
-  using FloatProperties<T>::FRACTION_MASK;
-  using FloatProperties<T>::FRACTION_LEN;
+// This is a temporary class to unify common methods and properties between
+// FPBits and FPBits<long double>.
+template <FPType fp_type> struct FPBitsCommon : private FPProperties<fp_type> {
+  using UP = FPProperties<fp_type>;
+  using typename UP::StorageType;
+  using UP::TOTAL_LEN;
 
-private:
-  using FloatProperties<T>::QUIET_NAN_MASK;
+protected:
+  using UP::EXP_SIG_MASK;
+  using UP::QUIET_NAN_MASK;
 
 public:
-  using FloatProperties<T>::SIGN_MASK;
+  using UP::EXP_BIAS;
+  using UP::EXP_LEN;
+  using UP::EXP_MASK;
+  using UP::FP_MASK;
+  using UP::FRACTION_LEN;
+  using UP::FRACTION_MASK;
+  using UP::SIGN_MASK;
 
   // Reinterpreting bits as an integer value and interpreting the bits of an
   // integer value as a floating point value is used in tests. So, a convenient
   // type is provided for such reinterpretations.
   StorageType bits;
 
+  LIBC_INLINE constexpr FPBitsCommon() : bits(0) {}
+  LIBC_INLINE explicit constexpr FPBitsCommon(StorageType bits) : bits(bits) {}
+
   LIBC_INLINE constexpr void set_mantissa(StorageType mantVal) {
     mantVal &= FRACTION_MASK;
     bits &= ~FRACTION_MASK;
@@ -64,6 +59,38 @@ template <typename T> struct FPBits : private FloatProperties<T> {
   LIBC_INLINE constexpr StorageType get_mantissa() const {
     return bits & FRACTION_MASK;
   }
+};
+
+} // namespace internal
+
+// 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::FPBitsCommon<get_fp_type<T>()> {
+  static_assert(cpp::is_floating_point_v<T>,
+                "FPBits instantiated with invalid type.");
+  using UP = internal::FPBitsCommon<get_fp_type<T>()>;
+  using StorageType = typename UP::StorageType;
+  using UP::bits;
+
+private:
+  using UP::EXP_SIG_MASK;
+  using UP::QUIET_NAN_MASK;
+
+public:
+  using UP::EXP_BIAS;
+  using UP::EXP_LEN;
+  using UP::EXP_MASK;
+  using UP::FRACTION_LEN;
+  using UP::FRACTION_MASK;
+  using UP::SIGN_MASK;
+  using UP::TOTAL_LEN;
 
   LIBC_INLINE constexpr void set_biased_exponent(StorageType expVal) {
     expVal = (expVal << FRACTION_LEN) & EXP_MASK;
@@ -94,9 +121,6 @@ template <typename T> struct FPBits : private FloatProperties<T> {
     return (bits & SIGN_MASK) != 0;
   }
 
-  static_assert(sizeof(T) == sizeof(StorageType),
-                "Data type and integral representation have different sizes.");
-
   static constexpr int MAX_BIASED_EXPONENT = (1 << EXP_LEN) - 1;
   static constexpr StorageType MIN_SUBNORMAL = StorageType(1);
   static constexpr StorageType MAX_SUBNORMAL = FRACTION_MASK;
@@ -108,13 +132,13 @@ template <typename T> struct FPBits : private FloatProperties<T> {
   // type match.
   template <typename XType, cpp::enable_if_t<cpp::is_same_v<T, XType>, int> = 0>
   LIBC_INLINE constexpr explicit FPBits(XType x)
-      : bits(cpp::bit_cast<StorageType>(x)) {}
+      : UP(cpp::bit_cast<StorageType>(x)) {}
 
   template <typename XType,
             cpp::enable_if_t<cpp::is_same_v<XType, StorageType>, int> = 0>
-  LIBC_INLINE constexpr explicit FPBits(XType x) : bits(x) {}
+  LIBC_INLINE constexpr explicit FPBits(XType x) : UP(x) {}
 
-  LIBC_INLINE constexpr FPBits() : bits(0) {}
+  LIBC_INLINE constexpr FPBits() : UP() {}
 
   LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(bits); }
 
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index b2b016adb661a7..9c67ef6126f6be 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -26,20 +26,26 @@
 namespace LIBC_NAMESPACE {
 namespace fputil {
 
-template <> struct FPBits<long double> : private FloatProperties<long double> {
-  using typename FloatProperties<long double>::StorageType;
-  using FloatProperties<long double>::TOTAL_LEN;
-  using FloatProperties<long double>::EXP_MASK;
-  using FloatProperties<long double>::EXP_BIAS;
-  using FloatProperties<long double>::EXP_LEN;
-  using FloatProperties<long double>::FRACTION_MASK;
-  using FloatProperties<long double>::FRACTION_LEN;
+template <>
+struct FPBits<long double>
+    : public internal::FPBitsCommon<FPType::X86_Binary80> {
+  using UP = internal::FPBitsCommon<FPType::X86_Binary80>;
+  using StorageType = typename UP::StorageType;
+  using UP::bits;
 
 private:
-  using FloatProperties<long double>::QUIET_NAN_MASK;
+  using UP::EXP_SIG_MASK;
+  using UP::QUIET_NAN_MASK;
 
 public:
-  using FloatProperties<long double>::SIGN_MASK;
+  using UP::EXP_BIAS;
+  using UP::EXP_LEN;
+  using UP::EXP_MASK;
+  using UP::FP_MASK;
+  using UP::FRACTION_LEN;
+  using UP::FRACTION_MASK;
+  using UP::SIGN_MASK;
+  using UP::TOTAL_LEN;
 
   static constexpr int MAX_BIASED_EXPONENT = 0x7FFF;
   static constexpr StorageType MIN_SUBNORMAL = StorageType(1);
@@ -51,18 +57,6 @@ template <> struct FPBits<long double> : private FloatProperties<long double> {
       (StorageType(MAX_BIASED_EXPONENT - 1) << (FRACTION_LEN + 1)) |
       (StorageType(1) << FRACTION_LEN) | MAX_SUBNORMAL;
 
-  StorageType bits;
-
-  LIBC_INLINE constexpr void set_mantissa(StorageType mantVal) {
-    mantVal &= FRACTION_MASK;
-    bits &= ~FRACTION_MASK;
-    bits |= mantVal;
-  }
-
-  LIBC_INLINE constexpr StorageType get_mantissa() const {
-    return bits & FRACTION_MASK;
-  }
-
   LIBC_INLINE constexpr StorageType get_explicit_mantissa() const {
     // The x86 80 bit float represents the leading digit of the mantissa
     // explicitly. This is the mask for that bit.
@@ -99,12 +93,12 @@ template <> struct FPBits<long double> : private FloatProperties<long double> {
     return bool((bits & SIGN_MASK) >> (TOTAL_LEN - 1));
   }
 
-  LIBC_INLINE constexpr FPBits() : bits(0) {}
+  LIBC_INLINE constexpr FPBits() : UP() {}
 
   template <typename XType,
             cpp::enable_if_t<cpp::is_same_v<long double, XType>, int> = 0>
   LIBC_INLINE constexpr explicit FPBits(XType x)
-      : bits(cpp::bit_cast<StorageType>(x)) {
+      : UP(cpp::bit_cast<StorageType>(x)) {
     // bits starts uninitialized, and setting it to a long double only
     // overwrites the first 80 bits. This clears those upper bits.
     bits = bits & ((StorageType(1) << 80) - 1);
@@ -112,7 +106,7 @@ template <> struct FPBits<long double> : private FloatProperties<long double> {
 
   template <typename XType,
             cpp::enable_if_t<cpp::is_same_v<XType, StorageType>, int> = 0>
-  LIBC_INLINE constexpr explicit FPBits(XType x) : bits(x) {}
+  LIBC_INLINE constexpr explicit FPBits(XType x) : UP(x) {}
 
   LIBC_INLINE constexpr operator long double() {
     return cpp::bit_cast<long double>(bits);

>From 04983437ea45aa5819d464d37986681a3f5e4022 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:14:15 +0000
Subject: [PATCH 02/10] Unify (get|set)_sign

---
 libc/src/__support/FPUtil/FPBits.h                | 5 ++---
 libc/src/__support/FPUtil/x86_64/LongDoubleBits.h | 7 +++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index bc79930797b691..9e088f91e96544 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -112,9 +112,8 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
   }
 
   LIBC_INLINE constexpr void set_sign(bool signVal) {
-    bits |= SIGN_MASK;
-    if (!signVal)
-      bits -= SIGN_MASK;
+    if (get_sign() != signVal)
+      bits ^= SIGN_MASK;
   }
 
   LIBC_INLINE constexpr bool get_sign() const {
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index 9c67ef6126f6be..daeda7f04357f8 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -84,13 +84,12 @@ struct FPBits<long double>
   }
 
   LIBC_INLINE constexpr void set_sign(bool signVal) {
-    bits &= ~SIGN_MASK;
-    StorageType sign1 = StorageType(signVal) << (TOTAL_LEN - 1);
-    bits |= sign1;
+    if (get_sign() != signVal)
+      bits ^= SIGN_MASK;
   }
 
   LIBC_INLINE constexpr bool get_sign() const {
-    return bool((bits & SIGN_MASK) >> (TOTAL_LEN - 1));
+    return (bits & SIGN_MASK) != 0;
   }
 
   LIBC_INLINE constexpr FPBits() : UP() {}

>From 5d616669693723e60f3c5b3a217af40e4a7b4309 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:15:23 +0000
Subject: [PATCH 03/10] Move (get|set)_sign to common implementation

---
 libc/src/__support/FPUtil/FPBits.h             | 18 +++++++++---------
 .../__support/FPUtil/x86_64/LongDoubleBits.h   |  9 ---------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 9e088f91e96544..fb778c8adf9a3d 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -59,6 +59,15 @@ template <FPType fp_type> struct FPBitsCommon : private FPProperties<fp_type> {
   LIBC_INLINE constexpr StorageType get_mantissa() const {
     return bits & FRACTION_MASK;
   }
+
+  LIBC_INLINE constexpr void set_sign(bool signVal) {
+    if (get_sign() != signVal)
+      bits ^= SIGN_MASK;
+  }
+
+  LIBC_INLINE constexpr bool get_sign() const {
+    return (bits & SIGN_MASK) != 0;
+  }
 };
 
 } // namespace internal
@@ -111,15 +120,6 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
            (FRACTION_MASK & bits);
   }
 
-  LIBC_INLINE constexpr void set_sign(bool signVal) {
-    if (get_sign() != signVal)
-      bits ^= SIGN_MASK;
-  }
-
-  LIBC_INLINE constexpr bool get_sign() const {
-    return (bits & SIGN_MASK) != 0;
-  }
-
   static constexpr int MAX_BIASED_EXPONENT = (1 << EXP_LEN) - 1;
   static constexpr StorageType MIN_SUBNORMAL = StorageType(1);
   static constexpr StorageType MAX_SUBNORMAL = FRACTION_MASK;
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index daeda7f04357f8..2c1162d4f2e151 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -83,15 +83,6 @@ struct FPBits<long double>
     return bool((bits & (StorageType(1) << FRACTION_LEN)) >> FRACTION_LEN);
   }
 
-  LIBC_INLINE constexpr void set_sign(bool signVal) {
-    if (get_sign() != signVal)
-      bits ^= SIGN_MASK;
-  }
-
-  LIBC_INLINE constexpr bool get_sign() const {
-    return (bits & SIGN_MASK) != 0;
-  }
-
   LIBC_INLINE constexpr FPBits() : UP() {}
 
   template <typename XType,

>From a19ec2d594e7160059a812347f458a12881c8d46 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:17:55 +0000
Subject: [PATCH 04/10] Move uintval() to common implementation

---
 libc/src/__support/FPUtil/FPBits.h                | 8 ++++----
 libc/src/__support/FPUtil/x86_64/LongDoubleBits.h | 5 -----
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index fb778c8adf9a3d..b4d3d0581726d5 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -68,6 +68,8 @@ template <FPType fp_type> struct FPBitsCommon : private FPProperties<fp_type> {
   LIBC_INLINE constexpr bool get_sign() const {
     return (bits & SIGN_MASK) != 0;
   }
+
+  LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
 };
 
 } // namespace internal
@@ -139,15 +141,13 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
 
   LIBC_INLINE constexpr FPBits() : UP() {}
 
-  LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(bits); }
-
   LIBC_INLINE constexpr void set_val(T value) {
     bits = cpp::bit_cast<StorageType>(value);
   }
 
-  LIBC_INLINE constexpr explicit operator T() const { return get_val(); }
+  LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(bits); }
 
-  LIBC_INLINE constexpr StorageType uintval() const { return bits; }
+  LIBC_INLINE constexpr explicit operator T() const { return get_val(); }
 
   LIBC_INLINE constexpr int get_exponent() const {
     return int(get_biased_exponent()) - EXP_BIAS;
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index 2c1162d4f2e151..8e6c86ff53a753 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -102,11 +102,6 @@ struct FPBits<long double>
     return cpp::bit_cast<long double>(bits);
   }
 
-  LIBC_INLINE constexpr StorageType uintval() {
-    // We zero the padding bits as they can contain garbage.
-    return bits & FP_MASK;
-  }
-
   LIBC_INLINE constexpr long double get_val() const {
     return cpp::bit_cast<long double>(bits);
   }

>From 8f32f4a03072efb0b80d9a2b4389099d31fd6eea Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:25:10 +0000
Subject: [PATCH 05/10] Unify (get|set)_biased_exponent

---
 libc/src/__support/FPUtil/FPBits.h                | 11 +++++++----
 libc/src/__support/FPUtil/FloatProperties.h       |  2 +-
 libc/src/__support/FPUtil/x86_64/LongDoubleBits.h | 10 ++++++----
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index b4d3d0581726d5..541bac1b0a17d3 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -37,6 +37,7 @@ template <FPType fp_type> struct FPBitsCommon : private FPProperties<fp_type> {
   using UP::EXP_BIAS;
   using UP::EXP_LEN;
   using UP::EXP_MASK;
+  using UP::EXP_MASK_SHIFT;
   using UP::FP_MASK;
   using UP::FRACTION_LEN;
   using UP::FRACTION_MASK;
@@ -98,19 +99,21 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
   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;
 
-  LIBC_INLINE constexpr void set_biased_exponent(StorageType expVal) {
-    expVal = (expVal << FRACTION_LEN) & EXP_MASK;
+  LIBC_INLINE constexpr void set_biased_exponent(StorageType biased) {
+    // clear exponent bits
     bits &= ~EXP_MASK;
-    bits |= expVal;
+    // set exponent bits
+    bits |= (biased << EXP_MASK_SHIFT) & EXP_MASK;
   }
 
   LIBC_INLINE constexpr uint16_t get_biased_exponent() const {
-    return uint16_t((bits & EXP_MASK) >> FRACTION_LEN);
+    return uint16_t((bits & EXP_MASK) >> EXP_MASK_SHIFT);
   }
 
   // The function return mantissa with the implicit bit set iff the current
diff --git a/libc/src/__support/FPUtil/FloatProperties.h b/libc/src/__support/FPUtil/FloatProperties.h
index bcf1f7cfabd34b..6bf75b7167d32d 100644
--- a/libc/src/__support/FPUtil/FloatProperties.h
+++ b/libc/src/__support/FPUtil/FloatProperties.h
@@ -111,7 +111,7 @@ struct FPProperties : public internal::FPBaseProperties<fp_type> {
       (1U << (EXP_LEN - 1U)) - 1U;
   static_assert(EXP_BIAS > 0);
 
-private:
+protected:
   // The shift amount to get the *significand* part to the least significant
   // bit. Always `0` but kept for consistency.
   LIBC_INLINE_VAR static constexpr int SIG_MASK_SHIFT = 0;
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index 8e6c86ff53a753..88e3df8b66f103 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -41,6 +41,7 @@ struct FPBits<long double>
   using UP::EXP_BIAS;
   using UP::EXP_LEN;
   using UP::EXP_MASK;
+  using UP::EXP_MASK_SHIFT;
   using UP::FP_MASK;
   using UP::FRACTION_LEN;
   using UP::FRACTION_MASK;
@@ -64,14 +65,15 @@ struct FPBits<long double>
     return bits & (FRACTION_MASK | EXPLICIT_BIT_MASK);
   }
 
-  LIBC_INLINE constexpr void set_biased_exponent(StorageType expVal) {
-    expVal = (expVal << (TOTAL_LEN - 1 - EXP_LEN)) & EXP_MASK;
+  LIBC_INLINE constexpr void set_biased_exponent(StorageType biased) {
+    // clear exponent bits
     bits &= ~EXP_MASK;
-    bits |= expVal;
+    // set exponent bits
+    bits |= (biased << EXP_MASK_SHIFT) & EXP_MASK;
   }
 
   LIBC_INLINE constexpr uint16_t get_biased_exponent() const {
-    return uint16_t((bits & EXP_MASK) >> (TOTAL_LEN - 1 - EXP_LEN));
+    return uint16_t((bits & EXP_MASK) >> EXP_MASK_SHIFT);
   }
 
   LIBC_INLINE constexpr void set_implicit_bit(bool implicitVal) {

>From 5dbb7ee4d993728ad95501e31a5f9c273e5c2bc1 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:27:02 +0000
Subject: [PATCH 06/10] Move (get|set)_biased_exponent to common implementation

---
 libc/src/__support/FPUtil/FPBits.h            | 22 ++++++++++---------
 .../__support/FPUtil/x86_64/LongDoubleBits.h  | 11 ----------
 2 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 541bac1b0a17d3..a30d8dbd40a617 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -70,6 +70,17 @@ template <FPType fp_type> struct FPBitsCommon : private FPProperties<fp_type> {
     return (bits & SIGN_MASK) != 0;
   }
 
+  LIBC_INLINE constexpr void set_biased_exponent(StorageType biased) {
+    // clear exponent bits
+    bits &= ~EXP_MASK;
+    // set exponent bits
+    bits |= (biased << EXP_MASK_SHIFT) & EXP_MASK;
+  }
+
+  LIBC_INLINE constexpr uint16_t get_biased_exponent() const {
+    return uint16_t((bits & EXP_MASK) >> EXP_MASK_SHIFT);
+  }
+
   LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
 };
 
@@ -105,16 +116,7 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
   using UP::SIGN_MASK;
   using UP::TOTAL_LEN;
 
-  LIBC_INLINE constexpr void set_biased_exponent(StorageType biased) {
-    // clear exponent bits
-    bits &= ~EXP_MASK;
-    // set exponent bits
-    bits |= (biased << EXP_MASK_SHIFT) & EXP_MASK;
-  }
-
-  LIBC_INLINE constexpr uint16_t get_biased_exponent() const {
-    return uint16_t((bits & EXP_MASK) >> EXP_MASK_SHIFT);
-  }
+  using UP::get_biased_exponent;
 
   // The function return mantissa with the implicit bit set iff the current
   // value is a valid normal number.
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index 88e3df8b66f103..73f966e115ea74 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -65,17 +65,6 @@ struct FPBits<long double>
     return bits & (FRACTION_MASK | EXPLICIT_BIT_MASK);
   }
 
-  LIBC_INLINE constexpr void set_biased_exponent(StorageType biased) {
-    // clear exponent bits
-    bits &= ~EXP_MASK;
-    // set exponent bits
-    bits |= (biased << EXP_MASK_SHIFT) & EXP_MASK;
-  }
-
-  LIBC_INLINE constexpr uint16_t get_biased_exponent() const {
-    return uint16_t((bits & EXP_MASK) >> EXP_MASK_SHIFT);
-  }
-
   LIBC_INLINE constexpr void set_implicit_bit(bool implicitVal) {
     bits &= ~(StorageType(1) << FRACTION_LEN);
     bits |= (StorageType(implicitVal) << FRACTION_LEN);

>From 36859e094967c197f2c22382862d18a256660625 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:28:05 +0000
Subject: [PATCH 07/10] Move get_exponent to common implementation

---
 libc/src/__support/FPUtil/FPBits.h                | 8 ++++----
 libc/src/__support/FPUtil/x86_64/LongDoubleBits.h | 4 ----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index a30d8dbd40a617..244bbd6efc00e4 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -81,6 +81,10 @@ template <FPType fp_type> struct FPBitsCommon : private FPProperties<fp_type> {
     return uint16_t((bits & EXP_MASK) >> EXP_MASK_SHIFT);
   }
 
+  LIBC_INLINE constexpr int get_exponent() const {
+    return int(get_biased_exponent()) - EXP_BIAS;
+  }
+
   LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
 };
 
@@ -154,10 +158,6 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
 
   LIBC_INLINE constexpr explicit operator T() const { return get_val(); }
 
-  LIBC_INLINE constexpr int get_exponent() const {
-    return int(get_biased_exponent()) - EXP_BIAS;
-  }
-
   // If the number is subnormal, the exponent is treated as if it were the
   // minimum exponent for a normal number. This is to keep continuity between
   // the normal and subnormal ranges, but it causes problems for functions where
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index 73f966e115ea74..0b7341fb545f88 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -97,10 +97,6 @@ struct FPBits<long double>
     return cpp::bit_cast<long double>(bits);
   }
 
-  LIBC_INLINE constexpr int get_exponent() const {
-    return int(get_biased_exponent()) - EXP_BIAS;
-  }
-
   // If the number is subnormal, the exponent is treated as if it were the
   // minimum exponent for a normal number. This is to keep continuity between
   // the normal and subnormal ranges, but it causes problems for functions where

>From 59590ce3a94978a3497d50c8f9256225b80bf47e Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:32:33 +0000
Subject: [PATCH 08/10] Unify is_zero implementation

---
 libc/src/__support/FPUtil/FPBits.h                | 3 +--
 libc/src/__support/FPUtil/x86_64/LongDoubleBits.h | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 244bbd6efc00e4..1a4f6b7c6d11a4 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -176,8 +176,7 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
   }
 
   LIBC_INLINE constexpr bool is_zero() const {
-    // Remove sign bit by shift
-    return (bits << 1) == 0;
+    return (bits & EXP_SIG_MASK) == 0;
   }
 
   LIBC_INLINE constexpr bool is_inf() const {
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index 0b7341fb545f88..cb120f898c2e34 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -115,8 +115,7 @@ struct FPBits<long double>
   }
 
   LIBC_INLINE constexpr bool is_zero() const {
-    return get_biased_exponent() == 0 && get_mantissa() == 0 &&
-           get_implicit_bit() == 0;
+    return (bits & EXP_SIG_MASK) == 0;
   }
 
   LIBC_INLINE constexpr bool is_inf() const {

>From 6a750393655154a95e93cf8cb4e52a338ed1d0eb Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:34:35 +0000
Subject: [PATCH 09/10] Move is_zero to common implementation

---
 libc/src/__support/FPUtil/FPBits.h                | 9 +++++----
 libc/src/__support/FPUtil/x86_64/LongDoubleBits.h | 4 ----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 1a4f6b7c6d11a4..3ec471f2e9072b 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -86,6 +86,10 @@ template <FPType fp_type> struct FPBitsCommon : private FPProperties<fp_type> {
   }
 
   LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
+
+  LIBC_INLINE constexpr bool is_zero() const {
+    return (bits & EXP_SIG_MASK) == 0;
+  }
 };
 
 } // namespace internal
@@ -121,6 +125,7 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
   using UP::TOTAL_LEN;
 
   using UP::get_biased_exponent;
+  using UP::is_zero;
 
   // The function return mantissa with the implicit bit set iff the current
   // value is a valid normal number.
@@ -175,10 +180,6 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
     }
   }
 
-  LIBC_INLINE constexpr bool is_zero() const {
-    return (bits & EXP_SIG_MASK) == 0;
-  }
-
   LIBC_INLINE constexpr bool is_inf() const {
     return (bits & EXP_SIG_MASK) == EXP_MASK;
   }
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index cb120f898c2e34..501175b0f6dd59 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -114,10 +114,6 @@ struct FPBits<long double>
     }
   }
 
-  LIBC_INLINE constexpr bool is_zero() const {
-    return (bits & EXP_SIG_MASK) == 0;
-  }
-
   LIBC_INLINE constexpr bool is_inf() const {
     return get_biased_exponent() == MAX_BIASED_EXPONENT &&
            get_mantissa() == 0 && get_implicit_bit() == 1;

>From f1b767dcdf453dd5b1beffbfc39c0ff8a5366916 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Wed, 20 Dec 2023 10:35:06 +0000
Subject: [PATCH 10/10] Move get_explicit_exponent to common implementation

---
 libc/src/__support/FPUtil/FPBits.h            | 34 +++++++++----------
 .../__support/FPUtil/x86_64/LongDoubleBits.h  | 17 ----------
 2 files changed, 17 insertions(+), 34 deletions(-)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 3ec471f2e9072b..4181772e8ea132 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -85,6 +85,23 @@ template <FPType fp_type> struct FPBitsCommon : private FPProperties<fp_type> {
     return int(get_biased_exponent()) - EXP_BIAS;
   }
 
+  // If the number is subnormal, the exponent is treated as if it were the
+  // minimum exponent for a normal number. This is to keep continuity between
+  // the normal and subnormal ranges, but it causes problems for functions where
+  // values are calculated from the exponent, since just subtracting the bias
+  // will give a slightly incorrect result. Additionally, zero has an exponent
+  // of zero, and that should actually be treated as zero.
+  LIBC_INLINE constexpr int get_explicit_exponent() const {
+    const int biased_exp = int(get_biased_exponent());
+    if (is_zero()) {
+      return 0;
+    } else if (biased_exp == 0) {
+      return 1 - EXP_BIAS;
+    } else {
+      return biased_exp - EXP_BIAS;
+    }
+  }
+
   LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
 
   LIBC_INLINE constexpr bool is_zero() const {
@@ -163,23 +180,6 @@ struct FPBits : public internal::FPBitsCommon<get_fp_type<T>()> {
 
   LIBC_INLINE constexpr explicit operator T() const { return get_val(); }
 
-  // If the number is subnormal, the exponent is treated as if it were the
-  // minimum exponent for a normal number. This is to keep continuity between
-  // the normal and subnormal ranges, but it causes problems for functions where
-  // values are calculated from the exponent, since just subtracting the bias
-  // will give a slightly incorrect result. Additionally, zero has an exponent
-  // of zero, and that should actually be treated as zero.
-  LIBC_INLINE constexpr int get_explicit_exponent() const {
-    const int biased_exp = int(get_biased_exponent());
-    if (is_zero()) {
-      return 0;
-    } else if (biased_exp == 0) {
-      return 1 - EXP_BIAS;
-    } else {
-      return biased_exp - EXP_BIAS;
-    }
-  }
-
   LIBC_INLINE constexpr bool is_inf() const {
     return (bits & EXP_SIG_MASK) == EXP_MASK;
   }
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index 501175b0f6dd59..1011e61f03fd6b 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -97,23 +97,6 @@ struct FPBits<long double>
     return cpp::bit_cast<long double>(bits);
   }
 
-  // If the number is subnormal, the exponent is treated as if it were the
-  // minimum exponent for a normal number. This is to keep continuity between
-  // the normal and subnormal ranges, but it causes problems for functions where
-  // values are calculated from the exponent, since just subtracting the bias
-  // will give a slightly incorrect result. Additionally, zero has an exponent
-  // of zero, and that should actually be treated as zero.
-  LIBC_INLINE constexpr int get_explicit_exponent() const {
-    const int biased_exp = int(get_biased_exponent());
-    if (is_zero()) {
-      return 0;
-    } else if (biased_exp == 0) {
-      return 1 - EXP_BIAS;
-    } else {
-      return biased_exp - EXP_BIAS;
-    }
-  }
-
   LIBC_INLINE constexpr bool is_inf() const {
     return get_biased_exponent() == MAX_BIASED_EXPONENT &&
            get_mantissa() == 0 && get_implicit_bit() == 1;



More information about the libc-commits mailing list