[libc-commits] [PATCH] D145011: [libc] Fix printf %f rounding condition

[Michael Jones via Phabricator via libc-commits]

michaelrj created this revision.
michaelrj added reviewers: sivachandra, lntue.
Herald added subscribers: libc-commits, ecnelises, tschuett.
Herald added projects: libc-project, All.
michaelrj requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145011

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


Index: libc/test/src/stdio/sprintf_test.cpp
===================================================================
--- libc/test/src/stdio/sprintf_test.cpp
+++ libc/test/src/stdio/sprintf_test.cpp
@@ -1140,6 +1140,13 @@
   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);
Index: libc/src/stdio/printf_core/float_dec_converter.h
===================================================================
--- libc/src/stdio/printf_core/float_dec_converter.h
+++ libc/src/stdio/printf_core/float_dec_converter.h
@@ -493,6 +493,10 @@
   }
 };
 
+// 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 @@
         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 @@
   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 @@
   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 ||


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145011.501305.patch
Type: text/x-patch
Size: 2658 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230228/fc1b0adf/attachment-0001.bin>