[libc][NFC] Move functions from `FPBits` to `FPRep`, make `bits` member private #79974

gchatelet · 2024-01-30T09:50:00Z

No description provided.

…er private

llvmbot · 2024-01-30T09:50:32Z

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/79974.diff

9 Files Affected:

(modified) libc/src/__support/FPUtil/FPBits.h (+46-51)
(modified) libc/src/math/generic/explogxf.h (+2-2)
(modified) libc/src/math/generic/hypotf.cpp (+2-2)
(modified) libc/src/math/generic/log1pf.cpp (+1-1)
(modified) libc/src/math/generic/range_reduction_fma.h (+4-2)
(modified) libc/test/src/math/atanhf_test.cpp (+1-1)
(modified) libc/test/src/math/smoke/atanhf_test.cpp (+1-1)
(modified) libc/test/src/math/smoke/nan_test.cpp (+1-1)
(modified) libc/test/src/math/smoke/nanf_test.cpp (+1-1)

diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 5277892ac3bb2..4d441706509cf 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -551,10 +551,13 @@ struct FPRep : public FPRepSem<fp_type, RetT> {
   using UP::SIG_LEN;
 
 public:
+  // Constants.
   using UP::EXP_BIAS;
   using UP::EXP_MASK;
   using UP::FRACTION_MASK;
   using UP::SIGN_MASK;
+  LIBC_INLINE_VAR static constexpr int MAX_BIASED_EXPONENT =
+      (1 << UP::EXP_LEN) - 1;
 
   LIBC_INLINE constexpr FPRep() = default;
   LIBC_INLINE constexpr explicit FPRep(StorageType x) : UP(x) {}
@@ -652,6 +655,44 @@ struct FPRep : public FPRepSem<fp_type, RetT> {
     bits = merge(bits, mantVal, FRACTION_MASK);
   }
 
+  // TODO: Use an uint32_t for 'biased_exp'.
+  LIBC_INLINE static constexpr RetT
+  create_value(Sign sign, StorageType biased_exp, StorageType mantissa) {
+    static_assert(fp_type != FPType::X86_Binary80,
+                  "This function is not tested for X86 Extended Precision");
+    return RetT(encode(sign, BiasedExp(static_cast<uint32_t>(biased_exp)),
+                       Sig(mantissa)));
+  }
+
+  // The function convert integer number and unbiased exponent to proper float
+  // T type:
+  //   Result = number * 2^(ep+1 - exponent_bias)
+  // Be careful!
+  //   1) "ep" is raw exponent value.
+  //   2) The function add to +1 to ep for seamless normalized to denormalized
+  //      transition.
+  //   3) The function did not check exponent high limit.
+  //   4) "number" zero value is not processed correctly.
+  //   5) Number is unsigned, so the result can be only positive.
+  LIBC_INLINE static constexpr RetT make_value(StorageType number, int ep) {
+    static_assert(fp_type != FPType::X86_Binary80,
+                  "This function is not tested for X86 Extended Precision");
+    FPRep result;
+    // offset: +1 for sign, but -1 for implicit first bit
+    int lz = cpp::countl_zero(number) - UP::EXP_LEN;
+    number <<= lz;
+    ep -= lz;
+
+    if (LIBC_LIKELY(ep >= 0)) {
+      // Implicit number bit will be removed by mask
+      result.set_mantissa(number);
+      result.set_biased_exponent(ep + 1);
+    } else {
+      result.set_mantissa(number >> -ep);
+    }
+    return RetT(result.uintval());
+  }
+
 private:
   // Merge bits from 'a' and 'b' values according to 'mask'.
   // Use 'a' bits when corresponding 'mask' bits are zeroes and 'b' bits when
@@ -696,79 +737,33 @@ template <typename T> LIBC_INLINE static constexpr FPType get_fp_type() {
     static_assert(cpp::always_false<UnqualT>, "Unsupported type");
 }
 
-// A generic class to manipulate floating point formats.
+// A generic class to manipulate C++ floating point formats.
 // It derives most of its functionality to FPRep above.
 template <typename T>
 struct FPBits final : public internal::FPRep<get_fp_type<T>(), FPBits<T>> {
   static_assert(cpp::is_floating_point_v<T>,
                 "FPBits instantiated with invalid type.");
   using UP = internal::FPRep<get_fp_type<T>(), FPBits<T>>;
-  using Rep = UP;
   using StorageType = typename UP::StorageType;
 
-  using UP::bits;
-
-  // Constants.
-  LIBC_INLINE_VAR static constexpr int MAX_BIASED_EXPONENT =
-      (1 << UP::EXP_LEN) - 1;
-
   // Constructors.
   LIBC_INLINE constexpr FPBits() = default;
 
   template <typename XType> LIBC_INLINE constexpr explicit FPBits(XType x) {
     using Unqual = typename cpp::remove_cv_t<XType>;
     if constexpr (cpp::is_same_v<Unqual, T>) {
-      bits = cpp::bit_cast<StorageType>(x);
+      UP::bits = cpp::bit_cast<StorageType>(x);
     } else if constexpr (cpp::is_same_v<Unqual, StorageType>) {
-      bits = x;
+      UP::bits = x;
     } else {
       // We don't want accidental type promotions/conversions, so we require
       // exact type match.
       static_assert(cpp::always_false<XType>);
     }
   }
-  // Floating-point conversions.
-  LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(bits); }
-
-  // TODO: Use an uint32_t for 'biased_exp'.
-  LIBC_INLINE static constexpr FPBits<T>
-  create_value(Sign sign, StorageType biased_exp, StorageType mantissa) {
-    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::BiasedExponent(static_cast<uint32_t>(biased_exp)),
-        typename UP::Significand(mantissa)));
-  }
 
-  // The function convert integer number and unbiased exponent to proper float
-  // T type:
-  //   Result = number * 2^(ep+1 - exponent_bias)
-  // Be careful!
-  //   1) "ep" is raw exponent value.
-  //   2) The function add to +1 to ep for seamless normalized to denormalized
-  //      transition.
-  //   3) The function did not check exponent high limit.
-  //   4) "number" zero value is not processed correctly.
-  //   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) - UP::EXP_LEN;
-    number <<= lz;
-    ep -= lz;
-
-    if (LIBC_LIKELY(ep >= 0)) {
-      // Implicit number bit will be removed by mask
-      result.set_mantissa(number);
-      result.set_biased_exponent(ep + 1);
-    } else {
-      result.set_mantissa(number >> -ep);
-    }
-    return result;
-  }
+  // Floating-point conversions.
+  LIBC_INLINE constexpr T get_val() const { return cpp::bit_cast<T>(UP::bits); }
 };
 
 } // namespace fputil
diff --git a/libc/src/math/generic/explogxf.h b/libc/src/math/generic/explogxf.h
index c5e35663acbe1..8817ba1011a8c 100644
--- a/libc/src/math/generic/explogxf.h
+++ b/libc/src/math/generic/explogxf.h
@@ -283,7 +283,7 @@ LIBC_INLINE static double log2_eval(double x) {
   int p1 = (bs.get_mantissa() >> (FPB::FRACTION_LEN - LOG_P1_BITS)) &
            (LOG_P1_SIZE - 1);
 
-  bs.bits &= FPB::FRACTION_MASK >> LOG_P1_BITS;
+  bs.set_uintval(bs.uintval() & (FPB::FRACTION_MASK >> LOG_P1_BITS));
   bs.set_biased_exponent(FPB::EXP_BIAS);
   double dx = (bs.get_val() - 1.0) * LOG_P1_1_OVER[p1];
 
@@ -313,7 +313,7 @@ LIBC_INLINE static double log_eval(double x) {
   int p1 = static_cast<int>(bs.get_mantissa() >> (FPB::FRACTION_LEN - 7));
 
   // Set bs to (1 + (mx - p1*2^(-7))
-  bs.bits &= FPB::FRACTION_MASK >> 7;
+  bs.set_uintval(bs.uintval() & (FPB::FRACTION_MASK >> 7));
   bs.set_biased_exponent(FPB::EXP_BIAS);
   // dx = (mx - p1*2^(-7)) / (1 + p1*2^(-7)).
   double dx = (bs.get_val() - 1.0) * ONE_OVER_F[p1];
diff --git a/libc/src/math/generic/hypotf.cpp b/libc/src/math/generic/hypotf.cpp
index 4c94b3eb7b988..614aa399fcc21 100644
--- a/libc/src/math/generic/hypotf.cpp
+++ b/libc/src/math/generic/hypotf.cpp
@@ -52,9 +52,9 @@ LLVM_LIBC_FUNCTION(float, hypotf, (float x, float y)) {
     uint64_t lrs = result.uintval() & mask;
 
     if (lrs == 0x0000'0000'1000'0000ULL && err < diff) {
-      result.bits |= 1ULL;
+      result.set_uintval(result.uintval() | 1ULL);
     } else if (lrs == 0x0000'0000'3000'0000ULL && err > diff) {
-      result.bits -= 1ULL;
+      result.set_uintval(result.uintval() - 1ULL);
     }
   } else {
     FPBits bits_x(x), bits_y(y);
diff --git a/libc/src/math/generic/log1pf.cpp b/libc/src/math/generic/log1pf.cpp
index 8de4a2067f9d3..a8ed28f8484b4 100644
--- a/libc/src/math/generic/log1pf.cpp
+++ b/libc/src/math/generic/log1pf.cpp
@@ -64,7 +64,7 @@ LIBC_INLINE float log(double x) {
   FPBits f = xbits;
 
   // Clear the lowest 45 bits.
-  f.bits &= ~0x0000'1FFF'FFFF'FFFFULL;
+  f.set_uintval(f.uintval() & ~0x0000'1FFF'FFFF'FFFFULL);
 
   double d = xbits.get_val() - f.get_val();
   d *= ONE_OVER_F[f_index];
diff --git a/libc/src/math/generic/range_reduction_fma.h b/libc/src/math/generic/range_reduction_fma.h
index d2ca5d348aadd..aee8cbb1332a6 100644
--- a/libc/src/math/generic/range_reduction_fma.h
+++ b/libc/src/math/generic/range_reduction_fma.h
@@ -51,7 +51,8 @@ LIBC_INLINE int64_t large_range_reduction(double x, int x_exp, double &y) {
     // - When |x| >= 2^55, the LSB of double(x * THIRTYTWO_OVER_PI[0]) is at
     // least 2^6.
     fputil::FPBits<double> prod_hi(x * THIRTYTWO_OVER_PI[0]);
-    prod_hi.bits &= (x_exp < 55) ? (~0xfffULL) : (~0ULL); // |x| < 2^55
+    prod_hi.set_uintval(prod_hi.uintval() &
+                        ((x_exp < 55) ? (~0xfffULL) : (~0ULL))); // |x| < 2^55
     double k_hi = fputil::nearest_integer(prod_hi.get_val());
     double truncated_prod = fputil::fma(x, THIRTYTWO_OVER_PI[0], -k_hi);
     double prod_lo = fputil::fma(x, THIRTYTWO_OVER_PI[1], truncated_prod);
@@ -70,7 +71,8 @@ LIBC_INLINE int64_t large_range_reduction(double x, int x_exp, double &y) {
   // - When |x| >= 2^110, the LSB of double(x * THIRTYTWO_OVER_PI[1]) is at
   // least 64.
   fputil::FPBits<double> prod_hi(x * THIRTYTWO_OVER_PI[1]);
-  prod_hi.bits &= (x_exp < 110) ? (~0xfffULL) : (~0ULL); // |x| < 2^110
+  prod_hi.set_uintval(prod_hi.uintval() &
+                      ((x_exp < 110) ? (~0xfffULL) : (~0ULL))); // |x| < 2^110
   double k_hi = fputil::nearest_integer(prod_hi.get_val());
   double truncated_prod = fputil::fma(x, THIRTYTWO_OVER_PI[1], -k_hi);
   double prod_lo = fputil::fma(x, THIRTYTWO_OVER_PI[2], truncated_prod);
diff --git a/libc/test/src/math/atanhf_test.cpp b/libc/test/src/math/atanhf_test.cpp
index 0080a328fbea6..9dea65dccd8fd 100644
--- a/libc/test/src/math/atanhf_test.cpp
+++ b/libc/test/src/math/atanhf_test.cpp
@@ -50,7 +50,7 @@ TEST_F(LlvmLibcAtanhfTest, SpecialNumbers) {
   EXPECT_MATH_ERRNO(ERANGE);
 
   auto bt = FPBits(1.0f);
-  bt.bits += 1;
+  bt.set_uintval(bt.uintval() + 1);
 
   LIBC_NAMESPACE::fputil::clear_except(FE_ALL_EXCEPT);
   EXPECT_FP_EQ_ALL_ROUNDING(aNaN, LIBC_NAMESPACE::atanhf(bt.get_val()));
diff --git a/libc/test/src/math/smoke/atanhf_test.cpp b/libc/test/src/math/smoke/atanhf_test.cpp
index f27b8e130fc91..c613f9bdf35f3 100644
--- a/libc/test/src/math/smoke/atanhf_test.cpp
+++ b/libc/test/src/math/smoke/atanhf_test.cpp
@@ -45,7 +45,7 @@ TEST_F(LlvmLibcAtanhfTest, SpecialNumbers) {
   EXPECT_MATH_ERRNO(ERANGE);
 
   auto bt = FPBits(1.0f);
-  bt.bits += 1;
+  bt.set_uintval(bt.uintval() + 1);
 
   EXPECT_FP_IS_NAN_WITH_EXCEPTION(LIBC_NAMESPACE::atanhf(bt.get_val()),
                                   FE_INVALID);
diff --git a/libc/test/src/math/smoke/nan_test.cpp b/libc/test/src/math/smoke/nan_test.cpp
index 81e1400f0bb6b..56c1e9164df41 100644
--- a/libc/test/src/math/smoke/nan_test.cpp
+++ b/libc/test/src/math/smoke/nan_test.cpp
@@ -20,7 +20,7 @@ class LlvmLibcNanTest : public LIBC_NAMESPACE::testing::Test {
     double result = LIBC_NAMESPACE::nan(input_str);
     auto actual_fp = LIBC_NAMESPACE::fputil::FPBits<double>(result);
     auto expected_fp = LIBC_NAMESPACE::fputil::FPBits<double>(bits);
-    EXPECT_EQ(actual_fp.bits, expected_fp.bits);
+    EXPECT_EQ(actual_fp.uintval(), expected_fp.uintval());
   };
 };
 
diff --git a/libc/test/src/math/smoke/nanf_test.cpp b/libc/test/src/math/smoke/nanf_test.cpp
index 1d337ecf1dcd0..bce495f1a9738 100644
--- a/libc/test/src/math/smoke/nanf_test.cpp
+++ b/libc/test/src/math/smoke/nanf_test.cpp
@@ -20,7 +20,7 @@ class LlvmLibcNanfTest : public LIBC_NAMESPACE::testing::Test {
     float result = LIBC_NAMESPACE::nanf(input_str);
     auto actual_fp = LIBC_NAMESPACE::fputil::FPBits<float>(result);
     auto expected_fp = LIBC_NAMESPACE::fputil::FPBits<float>(bits);
-    EXPECT_EQ(actual_fp.bits, expected_fp.bits);
+    EXPECT_EQ(actual_fp.uintval(), expected_fp.uintval());
   };
 };

gchatelet · 2024-01-30T09:52:50Z

We can probably rewrite the code that directly manipulates the bit representation with higher level functions.

libc/src/__support/FPUtil/FPBits.h

[libc][NFC] Move functions from FPBits to FPRep, make bits memb…

d8e4da0

…er private

gchatelet requested review from legrosbuffle and lntue and removed request for legrosbuffle January 30, 2024 09:50

llvmbot added the libc label Jan 30, 2024

gchatelet requested a review from legrosbuffle January 30, 2024 09:50

legrosbuffle approved these changes Jan 30, 2024

View reviewed changes

Address comments

8358cb3

gchatelet merged commit 24a903c into llvm:main Jan 30, 2024
3 of 4 checks passed

gchatelet deleted the move_last_functions_from_FPBit_to_FPRep branch January 30, 2024 14:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc][NFC] Move functions from `FPBits` to `FPRep`, make `bits` member private #79974

[libc][NFC] Move functions from `FPBits` to `FPRep`, make `bits` member private #79974

gchatelet commented Jan 30, 2024

llvmbot commented Jan 30, 2024

gchatelet commented Jan 30, 2024

[libc][NFC] Move functions from FPBits to FPRep, make bits member private #79974

[libc][NFC] Move functions from FPBits to FPRep, make bits member private #79974

Conversation

gchatelet commented Jan 30, 2024

llvmbot commented Jan 30, 2024

gchatelet commented Jan 30, 2024

[libc][NFC] Move functions from `FPBits` to `FPRep`, make `bits` member private #79974

[libc][NFC] Move functions from `FPBits` to `FPRep`, make `bits` member private #79974