[libc-commits] [libc] [libc][NFC] Make EXPONENT_BIAS int32_t (PR #75046)

Guillaume Chatelet via libc-commits libc-commits at lists.llvm.org
Mon Dec 11 05:03:08 PST 2023


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

>From 66a504356e5e75424a58114eb2cd7a47073f8d74 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Mon, 11 Dec 2023 12:56:33 +0000
Subject: [PATCH 1/2] [libc][NFC] Make EXPONENT_BIAS int32_t

`EXPONENT_BIAS` is almost always used with signed arithmetic. Making it an `int32_t` from the start reduces the chances to run into implementation defined behavior (cast from unsigned to signed is implementation-defined until C++20).

https://en.cppreference.com/w/cpp/language/implicit_conversion#:~:text=If%20the%20destination%20type%20is%20signed,arithmetic%20overflow%2C%20which%20is%20undefined).
---
 libc/src/__support/FPUtil/FloatProperties.h |  8 +++---
 libc/src/__support/detailed_powers_of_ten.h |  3 ++-
 libc/src/__support/str_to_float.h           | 27 ++++++++++-----------
 libc/src/math/generic/powf.cpp              |  6 ++---
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/libc/src/__support/FPUtil/FloatProperties.h b/libc/src/__support/FPUtil/FloatProperties.h
index 59e261e1047f1a..10db54a41ce78a 100644
--- a/libc/src/__support/FPUtil/FloatProperties.h
+++ b/libc/src/__support/FPUtil/FloatProperties.h
@@ -41,7 +41,7 @@ template <> struct FPProperties<FPType::IEEE754_Binary32> {
   static constexpr BitsType SIGN_MASK = BitsType(1)
                                         << (EXPONENT_WIDTH + MANTISSA_WIDTH);
   static constexpr BitsType EXPONENT_MASK = ~(SIGN_MASK | MANTISSA_MASK);
-  static constexpr uint32_t EXPONENT_BIAS = 127;
+  static constexpr int32_t EXPONENT_BIAS = 127;
 
   static constexpr BitsType EXP_MANT_MASK = MANTISSA_MASK + EXPONENT_MASK;
   static_assert(EXP_MANT_MASK == ~SIGN_MASK,
@@ -65,7 +65,7 @@ template <> struct FPProperties<FPType::IEEE754_Binary64> {
   static constexpr BitsType SIGN_MASK = BitsType(1)
                                         << (EXPONENT_WIDTH + MANTISSA_WIDTH);
   static constexpr BitsType EXPONENT_MASK = ~(SIGN_MASK | MANTISSA_MASK);
-  static constexpr uint32_t EXPONENT_BIAS = 1023;
+  static constexpr int32_t EXPONENT_BIAS = 1023;
 
   static constexpr BitsType EXP_MANT_MASK = MANTISSA_MASK + EXPONENT_MASK;
   static_assert(EXP_MANT_MASK == ~SIGN_MASK,
@@ -98,7 +98,7 @@ template <> struct FPProperties<FPType::X86_Binary80> {
       BitsType(1) << (EXPONENT_WIDTH + MANTISSA_WIDTH + 1);
   static constexpr BitsType EXPONENT_MASK =
       ((BitsType(1) << EXPONENT_WIDTH) - 1) << (MANTISSA_WIDTH + 1);
-  static constexpr uint32_t EXPONENT_BIAS = 16383;
+  static constexpr int32_t EXPONENT_BIAS = 16383;
 
   static constexpr BitsType EXP_MANT_MASK =
       MANTISSA_MASK | EXPLICIT_BIT_MASK | EXPONENT_MASK;
@@ -126,7 +126,7 @@ template <> struct FPProperties<FPType::IEEE754_Binary128> {
   static constexpr BitsType SIGN_MASK = BitsType(1)
                                         << (EXPONENT_WIDTH + MANTISSA_WIDTH);
   static constexpr BitsType EXPONENT_MASK = ~(SIGN_MASK | MANTISSA_MASK);
-  static constexpr uint32_t EXPONENT_BIAS = 16383;
+  static constexpr int32_t EXPONENT_BIAS = 16383;
 
   static constexpr BitsType EXP_MANT_MASK = MANTISSA_MASK | EXPONENT_MASK;
   static_assert(EXP_MANT_MASK == ~SIGN_MASK,
diff --git a/libc/src/__support/detailed_powers_of_ten.h b/libc/src/__support/detailed_powers_of_ten.h
index f9628484bbe609..1c30eff9b3f11c 100644
--- a/libc/src/__support/detailed_powers_of_ten.h
+++ b/libc/src/__support/detailed_powers_of_ten.h
@@ -29,7 +29,8 @@ constexpr int32_t DETAILED_POWERS_OF_TEN_MIN_EXP_10 = -348;
 constexpr int32_t DETAILED_POWERS_OF_TEN_MAX_EXP_10 = 347;
 
 // This rescales the base 10 exponent by a factor of log(10)/log(2).
-LIBC_INLINE int64_t exp10_to_exp2(int64_t exp10) {
+LIBC_INLINE int32_t exp10_to_exp2(int32_t exp10) {
+  // Valid if exp10 < 646 456 636.
   return (217706 * exp10) >> 16;
 }
 
diff --git a/libc/src/__support/str_to_float.h b/libc/src/__support/str_to_float.h
index ad73e93f6faa81..e2e088fe97f02a 100644
--- a/libc/src/__support/str_to_float.h
+++ b/libc/src/__support/str_to_float.h
@@ -131,9 +131,8 @@ eisel_lemire(ExpandedFloat<T> init_num,
   uint32_t clz = leading_zeroes<BitsType>(mantissa);
   mantissa <<= clz;
 
-  uint32_t exp2 = static_cast<uint32_t>(exp10_to_exp2(exp10)) +
-                  BITS_IN_MANTISSA + fputil::FloatProperties<T>::EXPONENT_BIAS -
-                  clz;
+  int32_t exp2 = exp10_to_exp2(exp10) + BITS_IN_MANTISSA +
+                 fputil::FloatProperties<T>::EXPONENT_BIAS - clz;
 
   // Multiplication
   const uint64_t *power_of_ten =
@@ -210,7 +209,8 @@ eisel_lemire(ExpandedFloat<T> init_num,
 
   // The if block is equivalent to (but has fewer branches than):
   //   if exp2 <= 0 || exp2 >= 0x7FF { etc }
-  if (exp2 - 1 >= (1 << fputil::FloatProperties<T>::EXPONENT_WIDTH) - 2) {
+  if (static_cast<uint32_t>(exp2) - 1 >=
+      (1 << fputil::FloatProperties<T>::EXPONENT_WIDTH) - 2) {
     return cpp::nullopt;
   }
 
@@ -251,9 +251,8 @@ eisel_lemire<long double>(ExpandedFloat<long double> init_num,
   uint32_t clz = leading_zeroes<BitsType>(mantissa);
   mantissa <<= clz;
 
-  uint32_t exp2 = static_cast<uint32_t>(exp10_to_exp2(exp10)) +
-                  BITS_IN_MANTISSA +
-                  fputil::FloatProperties<long double>::EXPONENT_BIAS - clz;
+  int32_t exp2 = exp10_to_exp2(exp10) + BITS_IN_MANTISSA +
+                 fputil::FloatProperties<long double>::EXPONENT_BIAS - clz;
 
   // Multiplication
   const uint64_t *power_of_ten =
@@ -383,7 +382,7 @@ simple_decimal_conversion(const char *__restrict numStart,
   // float, return inf.
   if (hpd.get_decimal_point() > 0 &&
       exp10_to_exp2(hpd.get_decimal_point() - 1) >
-          static_cast<int64_t>(fputil::FloatProperties<T>::EXPONENT_BIAS)) {
+          fputil::FloatProperties<T>::EXPONENT_BIAS) {
     output.num = {0, fputil::FPBits<T>::MAX_EXPONENT};
     output.error = ERANGE;
     return output;
@@ -391,8 +390,8 @@ simple_decimal_conversion(const char *__restrict numStart,
   // If the exponent is too small even for a subnormal, return 0.
   if (hpd.get_decimal_point() < 0 &&
       exp10_to_exp2(-hpd.get_decimal_point()) >
-          static_cast<int64_t>(fputil::FloatProperties<T>::EXPONENT_BIAS +
-                               fputil::FloatProperties<T>::MANTISSA_WIDTH)) {
+          (fputil::FloatProperties<T>::EXPONENT_BIAS +
+           fputil::FloatProperties<T>::MANTISSA_WIDTH)) {
     output.num = {0, 0};
     output.error = ERANGE;
     return output;
@@ -652,7 +651,7 @@ clinger_fast_path(ExpandedFloat<T> init_num,
 // log10(2^(exponent bias)).
 // The generic approximation uses the fact that log10(2^x) ~= x/3
 template <typename T> constexpr int32_t get_upper_bound() {
-  return static_cast<int32_t>(fputil::FloatProperties<T>::EXPONENT_BIAS) / 3;
+  return fputil::FloatProperties<T>::EXPONENT_BIAS / 3;
 }
 
 template <> constexpr int32_t get_upper_bound<float>() { return 39; }
@@ -668,9 +667,9 @@ template <> constexpr int32_t get_upper_bound<double>() { return 309; }
 // other out, and subnormal numbers allow for the result to be at the very low
 // end of the final mantissa.
 template <typename T> constexpr int32_t get_lower_bound() {
-  return -(static_cast<int32_t>(fputil::FloatProperties<T>::EXPONENT_BIAS +
-                                fputil::FloatProperties<T>::MANTISSA_WIDTH +
-                                (sizeof(T) * 8)) /
+  return -((fputil::FloatProperties<T>::EXPONENT_BIAS +
+            static_cast<int32_t>(fputil::FloatProperties<T>::MANTISSA_WIDTH +
+                                 (sizeof(T) * 8))) /
            3);
 }
 
diff --git a/libc/src/math/generic/powf.cpp b/libc/src/math/generic/powf.cpp
index 5f2e95b44e5287..cbfb12cf645278 100644
--- a/libc/src/math/generic/powf.cpp
+++ b/libc/src/math/generic/powf.cpp
@@ -392,8 +392,8 @@ LIBC_INLINE bool is_odd_integer(float x) {
   int x_e = static_cast<int>((x_u & FloatProp::EXPONENT_MASK) >>
                              FloatProp::MANTISSA_WIDTH);
   int lsb = cpp::countr_zero(x_u | FloatProp::EXPONENT_MASK);
-  constexpr int UNIT_EXPONENT =
-      static_cast<int>(FloatProp::EXPONENT_BIAS + FloatProp::MANTISSA_WIDTH);
+  constexpr int32_t UNIT_EXPONENT =
+      FloatProp::EXPONENT_BIAS + FloatProp::MANTISSA_WIDTH;
   return (x_e + lsb == UNIT_EXPONENT);
 }
 
@@ -404,7 +404,7 @@ LIBC_INLINE bool is_integer(float x) {
                              FloatProp::MANTISSA_WIDTH);
   int lsb = cpp::countr_zero(x_u | FloatProp::EXPONENT_MASK);
   constexpr int UNIT_EXPONENT =
-      static_cast<int>(FloatProp::EXPONENT_BIAS + FloatProp::MANTISSA_WIDTH);
+      FloatProp::EXPONENT_BIAS + FloatProp::MANTISSA_WIDTH;
   return (x_e + lsb >= UNIT_EXPONENT);
 }
 

>From b4fb6809a8fbf9d18e6c937d1e242d0542b56fe0 Mon Sep 17 00:00:00 2001
From: Guillaume Chatelet <gchatelet at google.com>
Date: Mon, 11 Dec 2023 13:02:54 +0000
Subject: [PATCH 2/2] Fix possible overflow in computation

---
 libc/src/__support/detailed_powers_of_ten.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libc/src/__support/detailed_powers_of_ten.h b/libc/src/__support/detailed_powers_of_ten.h
index 1c30eff9b3f11c..c8340fda06eed0 100644
--- a/libc/src/__support/detailed_powers_of_ten.h
+++ b/libc/src/__support/detailed_powers_of_ten.h
@@ -31,7 +31,7 @@ constexpr int32_t DETAILED_POWERS_OF_TEN_MAX_EXP_10 = 347;
 // This rescales the base 10 exponent by a factor of log(10)/log(2).
 LIBC_INLINE int32_t exp10_to_exp2(int32_t exp10) {
   // Valid if exp10 < 646 456 636.
-  return (217706 * exp10) >> 16;
+  return static_cast<int32_t>((217706 * static_cast<int64_t>(exp10)) >> 16);
 }
 
 static constexpr uint64_t DETAILED_POWERS_OF_TEN[696][2] = {



More information about the libc-commits mailing list