[libc-commits] [libc] b39f1e5 - [libc] Fix printf %f rounding condition

[Michael Jones via libc-commits]

Author: Michael Jones
Date: 2023-03-01T10:57:26-08:00
New Revision: b39f1e5c13897b69c314febd014895a5ed23696e

URL: https://github.com/llvm/llvm-project/commit/b39f1e5c13897b69c314febd014895a5ed23696e
DIFF: https://github.com/llvm/llvm-project/commit/b39f1e5c13897b69c314febd014895a5ed23696e.diff

LOG: [libc] Fix printf %f rounding condition

When running the tbin2dec tests I found a rounding error in my code.
Upon inspection I realized it was due to a lack of parenthesis when
calculating the number of trailing digits. This patch fixes that mistake
and adds unit tests to catch regressions in future.

Reviewed By: lntue

Differential Revision: https://reviews.llvm.org/D145011

Added: 
    

Modified: 
    libc/src/stdio/printf_core/float_dec_converter.h
    libc/test/src/stdio/sprintf_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index 805f3e1ccf04f..8d220ceeebb02 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -493,6 +493,10 @@ class FloatWriter {
   }
 };
 
+// This implementation is based on the Ryu Printf algorithm by Ulf Adams:
+// Ulf Adams. 2019. Ryū revisited: printf floating point conversion.
+// Proc. ACM Program. Lang. 3, OOPSLA, Article 169 (October 2019), 23 pages.
+// https://doi.org/10.1145/3360595
 template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
 LIBC_INLINE int convert_float_decimal_typed(Writer *writer,
                                             const FormatSection &to_conv,
@@ -590,7 +594,7 @@ LIBC_INLINE int convert_float_decimal_typed(Writer *writer,
         RoundDirection round;
         // Is m * 10^(additionalDigits + 1) / 2^(-exponent) integer?
         const int32_t requiredTwos =
-            -exponent - MANT_WIDTH - static_cast<int32_t>(precision) - 1;
+            -(exponent - MANT_WIDTH) - static_cast<int32_t>(precision) - 1;
         const bool trailingZeros =
             requiredTwos <= 0 ||
             (requiredTwos < 60 &&
@@ -764,7 +768,7 @@ LIBC_INLINE int convert_float_dec_exp_typed(Writer *writer,
   RoundDirection round;
   // Is m * 10^(additionalDigits + 1) / 2^(-exponent) integer?
   const int32_t requiredTwos =
-      -exponent - MANT_WIDTH - static_cast<int32_t>(precision) - 1;
+      -(exponent - MANT_WIDTH) - static_cast<int32_t>(precision) - 1;
   const bool trailingZeros =
       requiredTwos <= 0 ||
       (requiredTwos < 60 &&
@@ -1011,7 +1015,7 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer *writer,
   RoundDirection round;
   // Is m * 10^(additionalDigits + 1) / 2^(-exponent) integer?
   const int32_t requiredTwos =
-      -exponent - MANT_WIDTH - static_cast<int32_t>(exp_precision) - 1;
+      -(exponent - MANT_WIDTH) - static_cast<int32_t>(exp_precision) - 1;
   // TODO: rename this variable to remove confusion with trailing_zeroes
   const bool trailingZeros =
       requiredTwos <= 0 ||

diff  --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 84a0faabb20ad..a6a83154970a2 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -1140,6 +1140,13 @@ TEST_F(LlvmLibcSPrintfTest, FloatDecimalConv) {
   written = __llvm_libc::sprintf(buff, "%.5f", 1.008e3);
   ASSERT_STREQ_LEN(written, buff, "1008.00000");
 
+  // Found with the help of Fred Tydeman's tbin2dec test.
+  written = __llvm_libc::sprintf(buff, "%.1f", 0x1.1000000000006p+3);
+  ASSERT_STREQ_LEN(written, buff, "8.5");
+
+  written = __llvm_libc::sprintf(buff, "%.0f", 0x1.1000000000006p+3);
+  ASSERT_STREQ_LEN(written, buff, "9");
+
   // Subnormal Precision Tests
 
   written = __llvm_libc::sprintf(buff, "%.310f", 0x1.0p-1022);