[libc-commits] [libc] [libc][NFC] fix int warnings in float conversion (PR #74379)

via libc-commits libc-commits at lists.llvm.org
Mon Dec 4 16:04:31 PST 2023


https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/74379

>From 47091bc1052edc7b795f73320f5c5fe163a2b972 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Mon, 4 Dec 2023 14:17:04 -0800
Subject: [PATCH 1/3] [libc][NFC] fix int warnings in float conversion

The printf float to string conversion functions had some implicit
integer conversion warnings on gcc. This patch adds explicit casts to
these places.
---
 libc/src/__support/UInt.h                     |  1 +
 libc/src/__support/float_to_string.h          | 19 +++++++++++--------
 libc/src/math/generic/math_utils.h            |  4 +++-
 .../stdio/printf_core/float_dec_converter.h   | 13 ++++++++-----
 .../stdio/printf_core/float_hex_converter.h   |  6 +++---
 5 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index dbedfbc4c197e..248324c326dfb 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -121,6 +121,7 @@ template <size_t Bits, bool Signed> struct BigInt {
         return lo;
       }
     } else {
+      // TODO: silence shift warning
       return static_cast<T>((static_cast<T>(val[1]) << 64) + lo);
     }
   }
diff --git a/libc/src/__support/float_to_string.h b/libc/src/__support/float_to_string.h
index e198a5a7215fb..1a3e1385f606b 100644
--- a/libc/src/__support/float_to_string.h
+++ b/libc/src/__support/float_to_string.h
@@ -136,12 +136,12 @@ LIBC_INLINE constexpr uint32_t log10_pow2(const uint64_t e) {
   // us the floor, whereas counting the digits of the power of 2 gives us the
   // ceiling. With a similar loop I checked the maximum valid value and found
   // 42039.
-  return (e * 0x13441350fbdll) >> 42;
+  return static_cast<uint32_t>((e * 0x13441350fbdll) >> 42);
 }
 
 // Same as above, but with different constants.
 LIBC_INLINE constexpr uint32_t log2_pow5(const uint64_t e) {
-  return (e * 0x12934f0979bll) >> 39;
+  return static_cast<uint32_t>((e * 0x12934f0979bll) >> 39);
 }
 
 // Returns 1 + floor(log_10(2^e). This could technically be off by 1 if any
@@ -157,9 +157,10 @@ LIBC_INLINE constexpr uint32_t ceil_log10_pow2(const uint32_t e) {
 LIBC_INLINE constexpr uint32_t length_for_num(const uint32_t idx,
                                               const uint32_t mantissa_width) {
   //+8 to round up when dividing by 9
-  return (ceil_log10_pow2(idx) + ceil_log10_pow2(mantissa_width + 1) +
-          (BLOCK_SIZE - 1)) /
-         BLOCK_SIZE;
+  return static_cast<uint32_t>((ceil_log10_pow2(idx) +
+                                ceil_log10_pow2(mantissa_width + 1) +
+                                (BLOCK_SIZE - 1)) /
+                               BLOCK_SIZE);
   // return (ceil_log10_pow2(16 * idx + mantissa_width) + 8) / 9;
 }
 
@@ -488,7 +489,8 @@ class FloatToString {
 
       val = POW10_SPLIT[POW10_OFFSET[idx] + block_index];
 #endif
-      const uint32_t shift_amount = SHIFT_CONST + (IDX_SIZE * idx) - exponent;
+      const uint32_t shift_amount =
+          static_cast<uint32_t>(SHIFT_CONST + (IDX_SIZE * idx) - exponent);
       const uint32_t digits =
           internal::mul_shift_mod_1e9(mantissa, val, (int32_t)(shift_amount));
       return digits;
@@ -549,7 +551,7 @@ class FloatToString {
       val = POW10_SPLIT_2[p];
 #endif
       const int32_t shift_amount =
-          SHIFT_CONST + (-exponent - static_cast<int>(IDX_SIZE) * idx);
+          static_cast<int32_t>(SHIFT_CONST + (-exponent - IDX_SIZE * idx));
       uint32_t digits =
           internal::mul_shift_mod_1e9(mantissa, val, shift_amount);
       return digits;
@@ -747,7 +749,8 @@ FloatToString<long double>::get_negative_block(int block_index) {
                                                        block_index + 1);
     }
 #endif
-    const int32_t shift_amount = SHIFT_CONST + (-exponent - IDX_SIZE * idx);
+    const int32_t shift_amount =
+        SHIFT_CONST + (-exponent - static_cast<int>(IDX_SIZE * idx));
     BlockInt digits = internal::mul_shift_mod_1e9(mantissa, val, shift_amount);
     return digits;
   } else {
diff --git a/libc/src/math/generic/math_utils.h b/libc/src/math/generic/math_utils.h
index 38a14a47e88fa..e7fb193a08bea 100644
--- a/libc/src/math/generic/math_utils.h
+++ b/libc/src/math/generic/math_utils.h
@@ -34,7 +34,9 @@ LIBC_INLINE double as_double(uint64_t x) { return cpp::bit_cast<double>(x); }
 
 LIBC_INLINE uint32_t top12_bits(float x) { return as_uint32_bits(x) >> 20; }
 
-LIBC_INLINE uint32_t top12_bits(double x) { return as_uint64_bits(x) >> 52; }
+LIBC_INLINE uint32_t top12_bits(double x) {
+  return static_cast<uint32_t>(as_uint64_bits(x) >> 52);
+}
 
 // Values to trigger underflow and overflow.
 template <typename T> struct XFlowValues;
diff --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index 0e152a2602564..f329ba6e1dd3e 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -537,7 +537,7 @@ LIBC_INLINE int convert_float_decimal_typed(Writer *writer,
   }
 
   if (exponent < MANT_WIDTH) {
-    const uint32_t blocks = (precision / BLOCK_SIZE) + 1;
+    const uint32_t blocks = static_cast<uint32_t>(precision / BLOCK_SIZE) + 1;
     uint32_t i = 0;
     // if all the blocks we should write are zero
     if (blocks <= float_converter.zero_blocks_after_point()) {
@@ -561,7 +561,8 @@ LIBC_INLINE int convert_float_decimal_typed(Writer *writer,
         RET_IF_RESULT_NEGATIVE(float_writer.write_middle_block(digits));
       } else {
 
-        const uint32_t maximum = precision - BLOCK_SIZE * i;
+        const uint32_t maximum =
+            static_cast<uint32_t>(precision - BLOCK_SIZE * i);
         uint32_t last_digit = 0;
         for (uint32_t k = 0; k < BLOCK_SIZE - maximum; ++k) {
           last_digit = digits % 10;
@@ -646,7 +647,8 @@ LIBC_INLINE int convert_float_dec_exp_typed(Writer *writer,
 
   const size_t block_width = IntegerToString<intmax_t>(digits).size();
 
-  final_exponent = (cur_block * BLOCK_SIZE) + static_cast<int>(block_width - 1);
+  final_exponent = static_cast<int>(cur_block * BLOCK_SIZE) +
+                   static_cast<int>(block_width - 1);
   int positive_exponent = final_exponent < 0 ? -final_exponent : final_exponent;
 
   size_t exponent_width = IntegerToString<intmax_t>(positive_exponent).size();
@@ -819,7 +821,8 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer *writer,
   size_t trailing_zeroes = 0;
   size_t trailing_nines = 0;
 
-  base_10_exp = (cur_block * BLOCK_SIZE) + static_cast<int>(block_width - 1);
+  base_10_exp = static_cast<int>(cur_block * BLOCK_SIZE) +
+                static_cast<int>(block_width - 1);
 
   // If the first block is not also the last block
   if (block_width <= exp_precision + 1) {
@@ -864,7 +867,7 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer *writer,
              (cur_last_digit == 9 || cur_last_digit == 0)) {
         // If the next digit is not the same as the previous one, then there are
         // no more contiguous trailing digits.
-        if ((copy_of_digits % 10) != cur_last_digit) {
+        if (static_cast<int>(copy_of_digits % 10) != cur_last_digit) {
           break;
         }
         if (cur_last_digit == 9) {
diff --git a/libc/src/stdio/printf_core/float_hex_converter.h b/libc/src/stdio/printf_core/float_hex_converter.h
index 6a980a74d4a6f..1f105492e8e5a 100644
--- a/libc/src/stdio/printf_core/float_hex_converter.h
+++ b/libc/src/stdio/printf_core/float_hex_converter.h
@@ -139,8 +139,8 @@ LIBC_INLINE int convert_float_hex_exp(Writer *writer,
   size_t first_non_zero = 1;
   for (; mant_cur > 0; --mant_cur, mantissa >>= 4) {
     char mant_mod_16 = static_cast<char>(mantissa) & 15;
-    char new_digit =
-        (mant_mod_16 > 9) ? (mant_mod_16 - 10 + a) : (mant_mod_16 + '0');
+    char new_digit = static_cast<char>(
+        (mant_mod_16 > 9) ? (mant_mod_16 - 10 + a) : (mant_mod_16 + '0'));
     mant_buffer[mant_cur - 1] = new_digit;
     if (new_digit != '0' && first_non_zero < mant_cur)
       first_non_zero = mant_cur;
@@ -169,7 +169,7 @@ LIBC_INLINE int convert_float_hex_exp(Writer *writer,
 
   size_t exp_cur = EXP_LEN;
   for (; exponent > 0; --exp_cur, exponent /= 10) {
-    exp_buffer[exp_cur - 1] = (exponent % 10) + '0';
+    exp_buffer[exp_cur - 1] = static_cast<char>((exponent % 10) + '0');
   }
   if (exp_cur == EXP_LEN) { // if nothing else was written, write a 0.
     exp_buffer[EXP_LEN - 1] = '0';

>From 170b4b558a90fbc7e272ba6bd15b2057783271aa Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Mon, 4 Dec 2023 14:33:26 -0800
Subject: [PATCH 2/3] Rebase and narrow scope of casts in float_to_string

Patch #74369 fixed one of the fixed in the previous commit. This rebases
on top of that commit and also adjusts the casts in that file to better
match.
---
 libc/src/__support/float_to_string.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libc/src/__support/float_to_string.h b/libc/src/__support/float_to_string.h
index 1a3e1385f606b..c93c87ec53f87 100644
--- a/libc/src/__support/float_to_string.h
+++ b/libc/src/__support/float_to_string.h
@@ -287,11 +287,11 @@ LIBC_INLINE cpp::UInt<MID_INT_SIZE> get_table_negative(int exponent, size_t i) {
     if (block_shifts < static_cast<int>(ten_blocks)) {
       ten_blocks = ten_blocks - block_shifts;
       five_blocks = block_shifts;
-      shift_amount = shift_amount + (block_shifts * BLOCK_SIZE);
+      shift_amount = shift_amount + static_cast<int>(block_shifts * BLOCK_SIZE);
     } else {
       ten_blocks = 0;
       five_blocks = i;
-      shift_amount = static_cast<int>(shift_amount + (i * BLOCK_SIZE));
+      shift_amount = shift_amount + static_cast<int>(i * BLOCK_SIZE);
     }
   }
 
@@ -490,7 +490,7 @@ class FloatToString {
       val = POW10_SPLIT[POW10_OFFSET[idx] + block_index];
 #endif
       const uint32_t shift_amount =
-          static_cast<uint32_t>(SHIFT_CONST + (IDX_SIZE * idx) - exponent);
+          SHIFT_CONST + static_cast<uint32_t>(IDX_SIZE * idx) - exponent;
       const uint32_t digits =
           internal::mul_shift_mod_1e9(mantissa, val, (int32_t)(shift_amount));
       return digits;
@@ -551,7 +551,7 @@ class FloatToString {
       val = POW10_SPLIT_2[p];
 #endif
       const int32_t shift_amount =
-          static_cast<int32_t>(SHIFT_CONST + (-exponent - IDX_SIZE * idx));
+          SHIFT_CONST + (-exponent - static_cast<int32_t>(IDX_SIZE * idx));
       uint32_t digits =
           internal::mul_shift_mod_1e9(mantissa, val, shift_amount);
       return digits;

>From dec336770ee8db6450fa16970691ae703a58ff3d Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Mon, 4 Dec 2023 16:04:03 -0800
Subject: [PATCH 3/3] more type tweaks

---
 libc/src/__support/float_to_string.h          | 17 ++++++++---------
 libc/src/math/generic/math_utils.h            | 19 +++----------------
 .../stdio/printf_core/float_dec_converter.h   |  4 ++--
 3 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/libc/src/__support/float_to_string.h b/libc/src/__support/float_to_string.h
index c93c87ec53f87..34c0c0ceef286 100644
--- a/libc/src/__support/float_to_string.h
+++ b/libc/src/__support/float_to_string.h
@@ -103,7 +103,7 @@ constexpr size_t MID_INT_SIZE = 192;
 namespace LIBC_NAMESPACE {
 
 using BlockInt = uint32_t;
-constexpr size_t BLOCK_SIZE = 9;
+constexpr uint32_t BLOCK_SIZE = 9;
 
 using MantissaInt = fputil::FPBits<long double>::UIntType;
 
@@ -157,10 +157,9 @@ LIBC_INLINE constexpr uint32_t ceil_log10_pow2(const uint32_t e) {
 LIBC_INLINE constexpr uint32_t length_for_num(const uint32_t idx,
                                               const uint32_t mantissa_width) {
   //+8 to round up when dividing by 9
-  return static_cast<uint32_t>((ceil_log10_pow2(idx) +
-                                ceil_log10_pow2(mantissa_width + 1) +
-                                (BLOCK_SIZE - 1)) /
-                               BLOCK_SIZE);
+  return (ceil_log10_pow2(idx) + ceil_log10_pow2(mantissa_width + 1) +
+          (BLOCK_SIZE - 1)) /
+         BLOCK_SIZE;
   // return (ceil_log10_pow2(16 * idx + mantissa_width) + 8) / 9;
 }
 
@@ -287,11 +286,11 @@ LIBC_INLINE cpp::UInt<MID_INT_SIZE> get_table_negative(int exponent, size_t i) {
     if (block_shifts < static_cast<int>(ten_blocks)) {
       ten_blocks = ten_blocks - block_shifts;
       five_blocks = block_shifts;
-      shift_amount = shift_amount + static_cast<int>(block_shifts * BLOCK_SIZE);
+      shift_amount = shift_amount + (block_shifts * BLOCK_SIZE);
     } else {
       ten_blocks = 0;
       five_blocks = i;
-      shift_amount = shift_amount + static_cast<int>(i * BLOCK_SIZE);
+      shift_amount = shift_amount + (static_cast<int>(i) * BLOCK_SIZE);
     }
   }
 
@@ -490,7 +489,7 @@ class FloatToString {
       val = POW10_SPLIT[POW10_OFFSET[idx] + block_index];
 #endif
       const uint32_t shift_amount =
-          SHIFT_CONST + static_cast<uint32_t>(IDX_SIZE * idx) - exponent;
+          SHIFT_CONST + (static_cast<uint32_t>(IDX_SIZE) * idx) - exponent;
       const uint32_t digits =
           internal::mul_shift_mod_1e9(mantissa, val, (int32_t)(shift_amount));
       return digits;
@@ -551,7 +550,7 @@ class FloatToString {
       val = POW10_SPLIT_2[p];
 #endif
       const int32_t shift_amount =
-          SHIFT_CONST + (-exponent - static_cast<int32_t>(IDX_SIZE * idx));
+          SHIFT_CONST + (-exponent - (static_cast<int32_t>(IDX_SIZE) * idx));
       uint32_t digits =
           internal::mul_shift_mod_1e9(mantissa, val, shift_amount);
       return digits;
diff --git a/libc/src/math/generic/math_utils.h b/libc/src/math/generic/math_utils.h
index e7fb193a08bea..e884fe2deae28 100644
--- a/libc/src/math/generic/math_utils.h
+++ b/libc/src/math/generic/math_utils.h
@@ -18,26 +18,13 @@
 
 #include <stdint.h>
 
-namespace LIBC_NAMESPACE {
-
-LIBC_INLINE uint32_t as_uint32_bits(float x) {
-  return cpp::bit_cast<uint32_t>(x);
-}
-
-LIBC_INLINE uint64_t as_uint64_bits(double x) {
-  return cpp::bit_cast<uint64_t>(x);
-}
+// TODO: evaluate which functions from this file are actually used.
 
-LIBC_INLINE float as_float(uint32_t x) { return cpp::bit_cast<float>(x); }
+namespace LIBC_NAMESPACE {
 
+// TODO: Remove this, or move it to exp_utils.cpp which is its only user.
 LIBC_INLINE double as_double(uint64_t x) { return cpp::bit_cast<double>(x); }
 
-LIBC_INLINE uint32_t top12_bits(float x) { return as_uint32_bits(x) >> 20; }
-
-LIBC_INLINE uint32_t top12_bits(double x) {
-  return static_cast<uint32_t>(as_uint64_bits(x) >> 52);
-}
-
 // Values to trigger underflow and overflow.
 template <typename T> struct XFlowValues;
 
diff --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index f329ba6e1dd3e..d1b6633504cb2 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -861,13 +861,13 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer *writer,
       trailing_nines = 0;
       trailing_zeroes = 0;
       BlockInt copy_of_digits = digits;
-      int cur_last_digit = copy_of_digits % 10;
+      BlockInt cur_last_digit = copy_of_digits % 10;
       // We only care if it ends in nines or zeroes.
       while (copy_of_digits > 0 &&
              (cur_last_digit == 9 || cur_last_digit == 0)) {
         // If the next digit is not the same as the previous one, then there are
         // no more contiguous trailing digits.
-        if (static_cast<int>(copy_of_digits % 10) != cur_last_digit) {
+        if (copy_of_digits % 10 != cur_last_digit) {
           break;
         }
         if (cur_last_digit == 9) {



More information about the libc-commits mailing list