Skip to content

Conversation

lntue
Copy link
Contributor

@lntue lntue commented Oct 15, 2024

In IEEE 754 and C standards, when calling frexp with Inf/Nan inputs, the exponent result is unspecified. In this case, FreeBSD libc and musl just passthrough exp, while glibc, FreeBSD libm set exp = 0, and MSVC set exp = -1.

By default, LLVM libc will passthrough exp just as FreeBSD libc and musl, but we also allow users to explicitly choose the return exp value in this case for compatibility with other libc.

Notice that, gcc did generate passthrough exp for frexp(NaN/Inf, exp): https://godbolt.org/z/sM8fEej4E

@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2024

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

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

5 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCCompileOptionRules.cmake (+8-3)
  • (modified) libc/config/config.json (+4)
  • (modified) libc/docs/configure.rst (+1)
  • (modified) libc/src/__support/FPUtil/ManipulationFunctions.h (+5-1)
  • (modified) libc/test/src/math/smoke/FrexpTest.h (+11)
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 737ac87f4c7a21..0c658c6866c437 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -79,6 +79,14 @@ function(_get_compile_options_from_config output_var)
     list(APPEND config_options "-DLIBC_ADD_NULL_CHECKS")
   endif()
 
+  if(NOT "${LIBC_CONF_FREXP_INF_NAN_EXPONENT}" STREQUAL "")
+    list(APPEND config_options "-DLIBC_FREXP_INF_NAN_EXPONENT=${LIBC_CONF_FREXP_INF_NAN_EXPONENT}")
+  endif()
+
+  if(LIBC_CONF_MATH_OPTIMIZATIONS)
+    list(APPEND compile_options "-DLIBC_MATH=${LIBC_CONF_MATH_OPTIMIZATIONS}")
+  endif()
+
   set(${output_var} ${config_options} PARENT_SCOPE)
 endfunction(_get_compile_options_from_config)
 
@@ -170,9 +178,6 @@ function(_get_common_compile_options output_var flags)
       list(APPEND compile_options "-Wthread-safety")
       list(APPEND compile_options "-Wglobal-constructors")
     endif()
-    if(LIBC_CONF_MATH_OPTIMIZATIONS)
-      list(APPEND compile_options "-DLIBC_MATH=${LIBC_CONF_MATH_OPTIMIZATIONS}")
-    endif()
   elseif(MSVC)
     list(APPEND compile_options "/EHs-c-")
     list(APPEND compile_options "/GR-")
diff --git a/libc/config/config.json b/libc/config/config.json
index 2e4f878778e6e0..77d9a8d248e191 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -87,6 +87,10 @@
     "LIBC_CONF_MATH_OPTIMIZATIONS": {
       "value": 0,
       "doc": "Configures optimizations for math functions. Values accepted are LIBC_MATH_SKIP_ACCURATE_PASS, LIBC_MATH_SMALL_TABLES, LIBC_MATH_NO_ERRNO, LIBC_MATH_NO_EXCEPT, and LIBC_MATH_FAST."
+    },
+    "LIBC_CONF_FREXP_INF_NAN_EXPONENT": {
+      "value": "",
+      "doc": "Set the specific exp value for Inf/NaN inputs."
     }
   },
   "qsort": {
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 867bb807d10ac9..e225e6b566dfb2 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -33,6 +33,7 @@ to learn about the defaults for your platform and target.
 * **"general" options**
     - ``LIBC_ADD_NULL_CHECKS``: Add nullptr checks in the library's implementations to some functions for which passing nullptr is undefined behavior.
 * **"math" options**
+    - ``LIBC_CONF_FREXP_INF_NAN_EXPONENT``: Set the specific exp value for Inf/NaN inputs.
     - ``LIBC_CONF_MATH_OPTIMIZATIONS``: Configures optimizations for math functions. Values accepted are LIBC_MATH_SKIP_ACCURATE_PASS, LIBC_MATH_SMALL_TABLES, LIBC_MATH_NO_ERRNO, LIBC_MATH_NO_EXCEPT, and LIBC_MATH_FAST.
 * **"printf" options**
     - ``LIBC_CONF_PRINTF_DISABLE_FIXED_POINT``: Disable printing fixed point values in printf and friends.
diff --git a/libc/src/__support/FPUtil/ManipulationFunctions.h b/libc/src/__support/FPUtil/ManipulationFunctions.h
index 66bfe2aa377f99..c3853e93026ad3 100644
--- a/libc/src/__support/FPUtil/ManipulationFunctions.h
+++ b/libc/src/__support/FPUtil/ManipulationFunctions.h
@@ -31,8 +31,12 @@ namespace fputil {
 template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
 LIBC_INLINE T frexp(T x, int &exp) {
   FPBits<T> bits(x);
-  if (bits.is_inf_or_nan())
+  if (bits.is_inf_or_nan()) {
+#ifdef LIBC_FREXP_INF_NAN_EXPONENT
+    exp = LIBC_FREXP_INF_NAN_EXPONENT;
+#endif // LIBC_FREXP_INF_NAN_EXPONENT
     return x;
+  }
   if (bits.is_zero()) {
     exp = 0;
     return x;
diff --git a/libc/test/src/math/smoke/FrexpTest.h b/libc/test/src/math/smoke/FrexpTest.h
index 11641fc6743c44..3fb3a2e1688c81 100644
--- a/libc/test/src/math/smoke/FrexpTest.h
+++ b/libc/test/src/math/smoke/FrexpTest.h
@@ -21,8 +21,19 @@ class FrexpTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
   void testSpecialNumbers(FrexpFunc func) {
     int exponent;
     EXPECT_FP_EQ_ALL_ROUNDING(aNaN, func(aNaN, &exponent));
+#ifdef LIBC_FREXP_INF_NAN_EXPONENT
+    EXPECT_EQ(LIBC_FREXP_INF_NAN_EXPONENT, exponent);
+#endif // LIBC_FREXP_INF_NAN_EXPONENT
+
     EXPECT_FP_EQ_ALL_ROUNDING(inf, func(inf, &exponent));
+#ifdef LIBC_FREXP_INF_NAN_EXPONENT
+    EXPECT_EQ(LIBC_FREXP_INF_NAN_EXPONENT, exponent);
+#endif // LIBC_FREXP_INF_NAN_EXPONENT
+
     EXPECT_FP_EQ_ALL_ROUNDING(neg_inf, func(neg_inf, &exponent));
+#ifdef LIBC_FREXP_INF_NAN_EXPONENT
+    EXPECT_EQ(LIBC_FREXP_INF_NAN_EXPONENT, exponent);
+#endif // LIBC_FREXP_INF_NAN_EXPONENT
 
     EXPECT_FP_EQ_ALL_ROUNDING(zero, func(zero, &exponent));
     EXPECT_EQ(exponent, 0);

@lntue
Copy link
Contributor Author

lntue commented Oct 15, 2024

Maybe I should keep the cmake config flag and the macro definition of the same name.

@nickdesaulniers
Copy link
Member

Why do we want to make this configurable? Mind adding more context to the PR description?

@lntue
Copy link
Contributor Author

lntue commented Oct 15, 2024

Why do we want to make this configurable? Mind adding more context to the PR description?

I updated the PR description to describe the issue.

@lntue lntue merged commit b0dbd2c into llvm:main Oct 18, 2024
8 checks passed
@lntue lntue deleted the frexp branch October 18, 2024 13:58
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