Skip to content

[libc] Enable dyadic float for float printf #110765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

michaelrj-google
Copy link
Contributor

Dyadic floats were an existing option for float to string conversion,
but it had become stale. This patch fixes it up as well as adding proper
config options and test support. Due to the test changes this is a
followup to #110759

Dyadic floats were an existing option for float to string conversion,
but it had become stale. This patch fixes it up as well as adding proper
config options and test support. Due to the test changes this is a
followup to llvm#110759
@llvmbot
Copy link
Member

llvmbot commented Oct 1, 2024

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

Dyadic floats were an existing option for float to string conversion,
but it had become stale. This patch fixes it up as well as adding proper
config options and test support. Due to the test changes this is a
followup to #110759


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

8 Files Affected:

  • (modified) libc/config/config.json (+8)
  • (modified) libc/docs/configure.rst (+2)
  • (modified) libc/src/__support/FPUtil/dyadic_float.h (+1-1)
  • (modified) libc/src/__support/float_to_string.h (+11-8)
  • (modified) libc/src/stdio/printf_core/CMakeLists.txt (+6)
  • (modified) libc/test/src/stdio/CMakeLists.txt (+3)
  • (modified) libc/test/src/stdlib/CMakeLists.txt (+13-3)
  • (modified) libc/test/src/stdlib/StrfromTest.h (+4)
diff --git a/libc/config/config.json b/libc/config/config.json
index caeb3e89a84d8d..2e4f878778e6e0 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -22,6 +22,14 @@
       "value": false,
       "doc": "Use large table for better printf long double performance."
     },
+    "LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_DYADIC_FLOAT": {
+      "value": false,
+      "doc": "Use dyadic float for faster and smaller but less accurate printf doubles."
+    },
+    "LIBC_CONF_PRINTF_FLOAT_TO_STR_NO_SPECIALIZE_LD": {
+      "value": false,
+      "doc": "Use the same mode for double and long double in printf."
+    },
     "LIBC_CONF_PRINTF_DISABLE_FIXED_POINT": {
       "value": false,
       "doc": "Disable printing fixed point values in printf and friends."
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index c28d3a36ada748..867bb807d10ac9 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -40,6 +40,8 @@ to learn about the defaults for your platform and target.
     - ``LIBC_CONF_PRINTF_DISABLE_INDEX_MODE``: Disable index mode in the printf format string.
     - ``LIBC_CONF_PRINTF_DISABLE_STRERROR``: Disable handling of %m to print strerror in printf and friends.
     - ``LIBC_CONF_PRINTF_DISABLE_WRITE_INT``: Disable handling of %n in printf format string.
+    - ``LIBC_CONF_PRINTF_FLOAT_TO_STR_NO_SPECIALIZE_LD``: Use the same mode for double and long double in printf.
+    - ``LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_DYADIC_FLOAT``: Use dyadic float for faster and smaller but less accurate printf doubles.
     - ``LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE``: Use large table for better printf long double performance.
 * **"pthread" options**
     - ``LIBC_CONF_RAW_MUTEX_DEFAULT_SPIN_COUNT``: Default number of spins before blocking if a mutex is in contention (default to 100).
diff --git a/libc/src/__support/FPUtil/dyadic_float.h b/libc/src/__support/FPUtil/dyadic_float.h
index 165ffc7c922025..289fd01680547d 100644
--- a/libc/src/__support/FPUtil/dyadic_float.h
+++ b/libc/src/__support/FPUtil/dyadic_float.h
@@ -357,7 +357,7 @@ template <size_t Bits> struct DyadicFloat {
     return as<T, /*ShouldSignalExceptions=*/false>();
   }
 
-  LIBC_INLINE explicit constexpr operator MantissaType() const {
+  LIBC_INLINE constexpr MantissaType as_mantissa_type() const {
     if (mantissa.is_zero())
       return 0;
 
diff --git a/libc/src/__support/float_to_string.h b/libc/src/__support/float_to_string.h
index c4c7a15179f95c..e2e06cd0492a90 100644
--- a/libc/src/__support/float_to_string.h
+++ b/libc/src/__support/float_to_string.h
@@ -20,6 +20,7 @@
 #include "src/__support/libc_assert.h"
 #include "src/__support/macros/attributes.h"
 #include "src/__support/macros/config.h"
+#include "src/__support/sign.h"
 
 // This file has 5 compile-time flags to allow the user to configure the float
 // to string behavior. These were used to explore tradeoffs during the design
@@ -232,7 +233,7 @@ LIBC_INLINE UInt<MID_INT_SIZE> get_table_positive_df(int exponent, size_t i) {
   if (shift_amount < 0) {
     return 1;
   }
-  fputil::DyadicFloat<INT_SIZE> num(false, 0, 1);
+  fputil::DyadicFloat<INT_SIZE> num(Sign::POS, 0, 1);
   constexpr UInt<INT_SIZE> MOD_SIZE =
       (UInt<INT_SIZE>(EXP10_9)
        << (CALC_SHIFT_CONST + (IDX_SIZE > 1 ? IDX_SIZE : 0)));
@@ -242,16 +243,17 @@ LIBC_INLINE UInt<MID_INT_SIZE> get_table_positive_df(int exponent, size_t i) {
        0x89705f4136b4a597}};
 
   static const fputil::DyadicFloat<INT_SIZE> FIVE_EXP_MINUS_NINE(
-      false, -276, FIVE_EXP_MINUS_NINE_MANT);
+      Sign::POS, -276, FIVE_EXP_MINUS_NINE_MANT);
 
   if (i > 0) {
-    fputil::DyadicFloat<INT_SIZE> fives = fputil::pow_n(FIVE_EXP_MINUS_NINE, i);
+    fputil::DyadicFloat<INT_SIZE> fives =
+        fputil::pow_n(FIVE_EXP_MINUS_NINE, static_cast<uint32_t>(i));
     num = fives;
   }
   num = mul_pow_2(num, shift_amount);
 
   // Adding one is part of the formula.
-  UInt<INT_SIZE> int_num = static_cast<UInt<INT_SIZE>>(num) + 1;
+  UInt<INT_SIZE> int_num = num.as_mantissa_type() + 1;
   if (int_num > MOD_SIZE) {
     auto rem =
         int_num
@@ -339,23 +341,24 @@ LIBC_INLINE UInt<MID_INT_SIZE> get_table_negative_df(int exponent, size_t i) {
 
   int shift_amount = CALC_SHIFT_CONST - exponent;
 
-  fputil::DyadicFloat<INT_SIZE> num(false, 0, 1);
+  fputil::DyadicFloat<INT_SIZE> num(Sign::POS, 0, 1);
   constexpr UInt<INT_SIZE> MOD_SIZE =
       (UInt<INT_SIZE>(EXP10_9)
        << (CALC_SHIFT_CONST + (IDX_SIZE > 1 ? IDX_SIZE : 0)));
 
   constexpr UInt<INT_SIZE> TEN_EXP_NINE_MANT(EXP10_9);
 
-  static const fputil::DyadicFloat<INT_SIZE> TEN_EXP_NINE(false, 0,
+  static const fputil::DyadicFloat<INT_SIZE> TEN_EXP_NINE(Sign::POS, 0,
                                                           TEN_EXP_NINE_MANT);
 
   if (i > 0) {
-    fputil::DyadicFloat<INT_SIZE> tens = fputil::pow_n(TEN_EXP_NINE, i);
+    fputil::DyadicFloat<INT_SIZE> tens =
+        fputil::pow_n(TEN_EXP_NINE, static_cast<uint32_t>(i));
     num = tens;
   }
   num = mul_pow_2(num, shift_amount);
 
-  UInt<INT_SIZE> int_num = static_cast<UInt<INT_SIZE>>(num);
+  UInt<INT_SIZE> int_num = num.as_mantissa_type();
   if (int_num > MOD_SIZE) {
     auto rem =
         int_num
diff --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index 542327ad5a49a9..8172fda1e866ed 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -10,6 +10,12 @@ endif()
 if(LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE)
   list(APPEND printf_config_copts "-DLIBC_COPT_FLOAT_TO_STR_USE_MEGA_LONG_DOUBLE_TABLE")
 endif()
+if(LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_DYADIC_FLOAT)
+  list(APPEND printf_config_copts "-DLIBC_COPT_FLOAT_TO_STR_USE_DYADIC_FLOAT")
+endif()
+if(LIBC_CONF_PRINTF_FLOAT_TO_STR_NO_SPECIALIZE_LD)
+  list(APPEND printf_config_copts "-DLIBC_COPT_FLOAT_TO_STR_NO_SPECIALIZE_LD")
+endif()
 if(LIBC_CONF_PRINTF_DISABLE_FIXED_POINT)
   list(APPEND printf_config_copts "-DLIBC_COPT_PRINTF_DISABLE_FIXED_POINT")
 endif()
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index e970e4cf014052..ec94f5aaaf9b62 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -116,6 +116,9 @@ add_libc_test(
 if(LIBC_CONF_PRINTF_DISABLE_FLOAT)
   list(APPEND sprintf_test_copts "-DLIBC_COPT_PRINTF_DISABLE_FLOAT")
 endif()
+if(LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_DYADIC_FLOAT)
+  list(APPEND sprintf_test_copts "-DLIBC_COPT_FLOAT_TO_STR_REDUCED_PRECISION")
+endif()
 if(LIBC_CONF_PRINTF_DISABLE_INDEX_MODE)
   list(APPEND sprintf_test_copts "-DLIBC_COPT_PRINTF_DISABLE_INDEX_MODE")
 endif()
diff --git a/libc/test/src/stdlib/CMakeLists.txt b/libc/test/src/stdlib/CMakeLists.txt
index db90d9a4741ebf..c68627a6687419 100644
--- a/libc/test/src/stdlib/CMakeLists.txt
+++ b/libc/test/src/stdlib/CMakeLists.txt
@@ -168,13 +168,17 @@ add_libc_test(
     .strtol_test_support
 )
 
+if(LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_DYADIC_FLOAT)
+  list(APPEND strfrom_test_copts "-DLIBC_COPT_FLOAT_TO_STR_REDUCED_PRECISION")
+endif()
+
 add_header_library(
   strfrom_test_support
   HDRS
-  StrfromTest.h
+    StrfromTest.h
   DEPENDS
-  libc.src.__support.CPP.type_traits
-  libc.src.__support.FPUtil.fp_bits
+    libc.src.__support.CPP.type_traits
+    libc.src.__support.FPUtil.fp_bits
 )
 
 add_libc_test(
@@ -186,6 +190,8 @@ add_libc_test(
   DEPENDS
     .strfrom_test_support
     libc.src.stdlib.strfromf
+  COMPILE_OPTIONS
+    ${strfrom_test_copts}
 )
 
 add_libc_test(
@@ -197,6 +203,8 @@ add_libc_test(
   DEPENDS
     .strfrom_test_support
     libc.src.stdlib.strfromd
+  COMPILE_OPTIONS
+    ${strfrom_test_copts}
 )
 
 add_libc_test(
@@ -208,6 +216,8 @@ add_libc_test(
   DEPENDS
     .strfrom_test_support
     libc.src.stdlib.strfroml
+  COMPILE_OPTIONS
+    ${strfrom_test_copts}
 )
 
 add_libc_test(
diff --git a/libc/test/src/stdlib/StrfromTest.h b/libc/test/src/stdlib/StrfromTest.h
index 0db507ef0716e9..5209472886f78e 100644
--- a/libc/test/src/stdlib/StrfromTest.h
+++ b/libc/test/src/stdlib/StrfromTest.h
@@ -156,6 +156,9 @@ class StrfromTest : public LIBC_NAMESPACE::testing::Test {
     written = func(buff, 99, "%f", 1.5);
     ASSERT_STREQ_LEN(written, buff, "1.500000");
 
+// Dyadic float is only accurate to ~50 digits, so skip this 300 digit test.
+// TODO: Create way to test just the first ~50 digits of a number.
+#ifndef LIBC_COPT_FLOAT_TO_STR_REDUCED_PRECISION
     written = func(buff, 499, "%f", 1e300);
     ASSERT_STREQ_LEN(written, buff,
                      "100000000000000005250476025520442024870446858110815915491"
@@ -167,6 +170,7 @@ class StrfromTest : public LIBC_NAMESPACE::testing::Test {
                      "111903896764088007465274278014249457925878882005684283811"
                      "566947219638686"
                      "5459400540160.000000");
+#endif // DLIBC_COPT_FLOAT_TO_STR_REDUCED_PRECISION
 
     written = func(buff, 99, "%f", 0.1);
     ASSERT_STREQ_LEN(written, buff, "0.100000");

@michaelrj-google michaelrj-google merged commit e1d64b7 into llvm:main Oct 2, 2024
10 checks passed
@michaelrj-google michaelrj-google deleted the libcPrintfDyadicFloatFix branch October 2, 2024 16:42
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
Dyadic floats were an existing option for float to string conversion,
but it had become stale. This patch fixes it up as well as adding proper
config options and test support. Due to the test changes this is a
followup to llvm#110759
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants