[libc-commits] [libc] [libc][math] adds `nanf128` (PR #85201)

via libc-commits libc-commits at lists.llvm.org
Thu Mar 14 01:55:44 PDT 2024 

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Michael Flanders (Flandini)

<details>
<summary>Changes</summary>

Continuing  #<!-- -->84689, this one required more changes than the others, so I am making it a separate PR.

Extends some stuff in `str_to_float.h`, `str_to_integer.h` to work on types wider than `unsigned long long` and `uint64_t`.

This also edits `UInt.h` to add type traits for `make_unsigned` and `make_signed` since `make_unsigned<UInt<128>>` is used in `strtointeger` now. I also added the other cases of `make_unsigned` and `make_signed` for `BigInt` for completeness. 

I think the float128, and other code making use of `UInt.h`, are only being tested with `LIBC_TYPES_HAS_INT128` defined. When I removed the define of `LIBC_TYPES_HAS_INT128` in `src/__support/macros/properties/types.h`, I got a lot of compiler errors. I think this is because many type traits are not defined for BigInts, only for `__uint128_t` and `__int128_t`. `is_unsigned` is one type trait that is not defined for BigInts. I am also unsure if this is an issue or if I am just not 'not defining' `LIBC_TYPES_HAS_INT128` properly. If this is an issue, I also wonder if we should be testing both sides of the preprocessor condition in `UInt128.h`. Let me know if I should open an issue.

cc @<!-- -->lntue for review.

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


13 Files Affected:

- (modified) libc/config/linux/aarch64/entrypoints.txt (+1) 
- (modified) libc/config/linux/riscv/entrypoints.txt (+1) 
- (modified) libc/config/linux/x86_64/entrypoints.txt (+1) 
- (modified) libc/spec/stdc.td (+1) 
- (modified) libc/src/__support/UInt.h (+9) 
- (modified) libc/src/__support/str_to_float.h (+11-15) 
- (modified) libc/src/__support/str_to_integer.h (+15-9) 
- (modified) libc/src/math/CMakeLists.txt (+1) 
- (modified) libc/src/math/generic/CMakeLists.txt (+13) 
- (added) libc/src/math/generic/nanf128.cpp (+23) 
- (added) libc/src/math/nanf128.h (+20) 
- (modified) libc/test/src/math/smoke/CMakeLists.txt (+16) 
- (added) libc/test/src/math/smoke/nanf128_test.cpp (+64) 


``````````diff
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 7d69099e3cb9db..43c9e81f17833e 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -468,6 +468,7 @@ if(LIBC_TYPES_HAS_FLOAT128)
     libc.src.math.lrintf128
     libc.src.math.lroundf128
     libc.src.math.modff128
+    libc.src.math.nanf128
     libc.src.math.nextafterf128
     libc.src.math.rintf128
     libc.src.math.roundf128
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index b1c9dd0428eea5..99ef84d3f73974 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -476,6 +476,7 @@ if(LIBC_TYPES_HAS_FLOAT128)
     libc.src.math.lrintf128
     libc.src.math.lroundf128
     libc.src.math.modff128
+    libc.src.math.nanf128
     libc.src.math.nextafterf128
     libc.src.math.rintf128
     libc.src.math.roundf128
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 4fb31c593b9dc7..a8b416aa9a0cda 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -481,6 +481,7 @@ if(LIBC_TYPES_HAS_FLOAT128)
     libc.src.math.lrintf128
     libc.src.math.lroundf128
     libc.src.math.modff128
+    libc.src.math.nanf128
     libc.src.math.nextafterf128
     libc.src.math.rintf128
     libc.src.math.roundf128
diff --git a/libc/spec/stdc.td b/libc/spec/stdc.td
index afe01b1bb68566..2bc9bc8b9b1a6f 100644
--- a/libc/spec/stdc.td
+++ b/libc/spec/stdc.td
@@ -559,6 +559,7 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"nanf", RetValSpec<FloatType>, [ArgSpec<ConstCharPtr>]>,
           FunctionSpec<"nan", RetValSpec<DoubleType>, [ArgSpec<ConstCharPtr>]>,
           FunctionSpec<"nanl", RetValSpec<LongDoubleType>, [ArgSpec<ConstCharPtr>]>,
+          GuardedFunctionSpec<"nanf128", RetValSpec<Float128Type>, [ArgSpec<ConstCharPtr>], "LIBC_TYPES_HAS_FLOAT128">,
       ]
   >;
 
diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index d92d61ed094ebe..97ecfe8606dbfb 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -993,6 +993,15 @@ struct is_big_int<BigInt<Bits, Signed, T>> : cpp::true_type {};
 template <class T>
 LIBC_INLINE_VAR constexpr bool is_big_int_v = is_big_int<T>::value;
 
+// make_unsigned and make_signed type traits
+template <size_t Bits, bool Signed, typename T>
+struct cpp::make_unsigned<BigInt<Bits, Signed, T>>
+    : cpp::type_identity<BigInt<Bits, false, T>> {};
+
+template <size_t Bits, bool Signed, typename T>
+struct cpp::make_signed<BigInt<Bits, Signed, T>>
+    : cpp::type_identity<BigInt<Bits, true, T>> {};
+
 namespace cpp {
 
 // Specialization of cpp::bit_cast ('bit.h') from T to BigInt.
diff --git a/libc/src/__support/str_to_float.h b/libc/src/__support/str_to_float.h
index 073e1dc6721723..2cf2cfb027243e 100644
--- a/libc/src/__support/str_to_float.h
+++ b/libc/src/__support/str_to_float.h
@@ -1056,17 +1056,17 @@ hexadecimal_string_to_float(const char *__restrict src,
   return output;
 }
 
-LIBC_INLINE uint64_t
+template <class T>
+LIBC_INLINE typename fputil::FPBits<T>::StorageType
 nan_mantissa_from_ncharseq(const cpp::string_view ncharseq) {
-  uint64_t nan_mantissa = 0;
+  using FPBits = typename fputil::FPBits<T>;
+  using StorageType = typename FPBits::StorageType;
+
+  StorageType nan_mantissa = 0;
 
   if (ncharseq.data() != nullptr && isdigit(ncharseq[0])) {
-    // This is to prevent errors when StorageType is larger than 64
-    // bits, since strtointeger only supports up to 64 bits. This is
-    // actually more than is required by the specification, which says
-    // for the input type "NAN(n-char-sequence)" that "the meaning of
-    // the n-char sequence is implementation-defined."
-    auto strtoint_result = strtointeger<uint64_t>(ncharseq.data(), 0);
+    StrToNumResult<StorageType> strtoint_result =
+        strtointeger<StorageType>(ncharseq.data(), 0);
     if (!strtoint_result.has_error())
       nan_mantissa = strtoint_result.value;
 
@@ -1172,9 +1172,8 @@ LIBC_INLINE StrToNumResult<T> strtofloatingpoint(const char *__restrict src) {
           ++index;
         if (src[index] == ')') {
           ++index;
-          auto nan_mantissa_result = nan_mantissa_from_ncharseq(
+          nan_mantissa = nan_mantissa_from_ncharseq<T>(
               cpp::string_view(src + (left_paren + 1), index - left_paren - 2));
-          nan_mantissa = static_cast<StorageType>(nan_mantissa_result);
         } else {
           index = left_paren;
         }
@@ -1221,11 +1220,8 @@ template <class T> LIBC_INLINE StrToNumResult<T> strtonan(const char *arg) {
   while (isalnum(arg[index]) || arg[index] == '_')
     ++index;
 
-  if (arg[index] == '\0') {
-    auto nan_mantissa_result =
-        nan_mantissa_from_ncharseq(cpp::string_view(arg, index));
-    nan_mantissa = static_cast<StorageType>(nan_mantissa_result);
-  }
+  if (arg[index] == '\0')
+    nan_mantissa = nan_mantissa_from_ncharseq<T>(cpp::string_view(arg, index));
 
   result = FPBits::quiet_nan(fputil::Sign::POS, nan_mantissa);
   return {result.get_val(), 0, error};
diff --git a/libc/src/__support/str_to_integer.h b/libc/src/__support/str_to_integer.h
index b87808993fee50..85a46065203f17 100644
--- a/libc/src/__support/str_to_integer.h
+++ b/libc/src/__support/str_to_integer.h
@@ -11,6 +11,7 @@
 
 #include "src/__support/CPP/limits.h"
 #include "src/__support/CPP/type_traits.h"
+#include "src/__support/UInt128.h"
 #include "src/__support/common.h"
 #include "src/__support/ctype_utils.h"
 #include "src/__support/str_to_num_result.h"
@@ -75,8 +76,12 @@ template <class T>
 LIBC_INLINE StrToNumResult<T>
 strtointeger(const char *__restrict src, int base,
              const size_t src_len = cpp::numeric_limits<size_t>::max()) {
-  // TODO: Rewrite to support numbers longer than long long
-  unsigned long long result = 0;
+  using ResultType = typename cpp::conditional_t<(cpp::is_same_v<T, UInt128> ||
+                                                  cpp::is_same_v<T, Int128>),
+                                                 UInt128, unsigned long long>;
+
+  ResultType result = 0;
+
   bool is_number = false;
   size_t src_cur = 0;
   int error_val = 0;
@@ -101,15 +106,16 @@ strtointeger(const char *__restrict src, int base,
   if (base == 16 && is_hex_start(src + src_cur, src_len - src_cur))
     src_cur = src_cur + 2;
 
-  constexpr bool IS_UNSIGNED = (cpp::numeric_limits<T>::min() == 0);
+  constexpr bool IS_UNSIGNED = cpp::is_unsigned_v<T>;
   const bool is_positive = (result_sign == '+');
-  unsigned long long constexpr NEGATIVE_MAX =
-      !IS_UNSIGNED
-          ? static_cast<unsigned long long>(cpp::numeric_limits<T>::max()) + 1
-          : cpp::numeric_limits<T>::max();
-  unsigned long long const abs_max =
+
+  ResultType constexpr NEGATIVE_MAX =
+      !IS_UNSIGNED ? static_cast<ResultType>(cpp::numeric_limits<T>::max()) + 1
+                   : cpp::numeric_limits<T>::max();
+  ResultType const abs_max =
       (is_positive ? cpp::numeric_limits<T>::max() : NEGATIVE_MAX);
-  unsigned long long const abs_max_div_by_base = abs_max / base;
+  ResultType const abs_max_div_by_base = abs_max / base;
+
   while (src_cur < src_len && isalnum(src[src_cur])) {
     int cur_digit = b36_char_to_int(src[src_cur]);
     if (cur_digit >= base)
diff --git a/libc/src/math/CMakeLists.txt b/libc/src/math/CMakeLists.txt
index 750fd5f0e3a9ba..cd03065399480e 100644
--- a/libc/src/math/CMakeLists.txt
+++ b/libc/src/math/CMakeLists.txt
@@ -190,6 +190,7 @@ add_math_entrypoint_object(modff128)
 add_math_entrypoint_object(nan)
 add_math_entrypoint_object(nanf)
 add_math_entrypoint_object(nanl)
+add_math_entrypoint_object(nanf128)
 
 add_math_entrypoint_object(nearbyint)
 add_math_entrypoint_object(nearbyintf)
diff --git a/libc/src/math/generic/CMakeLists.txt b/libc/src/math/generic/CMakeLists.txt
index 667381d615d1e0..87f53105a1b317 100644
--- a/libc/src/math/generic/CMakeLists.txt
+++ b/libc/src/math/generic/CMakeLists.txt
@@ -1780,6 +1780,19 @@ add_entrypoint_object(
     -O3
 )
 
+add_entrypoint_object(
+  nanf128
+  SRCS
+    nanf128.cpp
+  HDRS
+    ../nanf128.h
+  DEPENDS
+    libc.src.__support.str_to_float
+    libc.src.errno.errno
+  COMPILE_OPTIONS
+    -O3
+)
+
 add_entrypoint_object(
   nextafter
   SRCS
diff --git a/libc/src/math/generic/nanf128.cpp b/libc/src/math/generic/nanf128.cpp
new file mode 100644
index 00000000000000..f087c9f074fde8
--- /dev/null
+++ b/libc/src/math/generic/nanf128.cpp
@@ -0,0 +1,23 @@
+//===-- Implementation of nanf128 function --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/math/nanf128.h"
+#include "src/__support/common.h"
+#include "src/__support/str_to_float.h"
+#include "src/errno/libc_errno.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(float128, nanf128, (const char *arg)) {
+  auto result = internal::strtonan<float128>(arg);
+  if (result.has_error())
+    libc_errno = result.error;
+  return result.value;
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/nanf128.h b/libc/src/math/nanf128.h
new file mode 100644
index 00000000000000..b06d14e2f945e1
--- /dev/null
+++ b/libc/src/math/nanf128.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for nanf128 -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_MATH_NANF128_H
+#define LLVM_LIBC_SRC_MATH_NANF128_H
+
+#include "src/__support/macros/properties/types.h"
+
+namespace LIBC_NAMESPACE {
+
+float128 nanf128(const char *arg);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_MATH_NANF128_H
diff --git a/libc/test/src/math/smoke/CMakeLists.txt b/libc/test/src/math/smoke/CMakeLists.txt
index 293e65abd44f5a..80ea9d1109a0cf 100644
--- a/libc/test/src/math/smoke/CMakeLists.txt
+++ b/libc/test/src/math/smoke/CMakeLists.txt
@@ -1506,6 +1506,22 @@ add_fp_unittest(
   UNIT_TEST_ONLY
 )
 
+add_fp_unittest(
+  nanf128_test
+  SUITE
+    libc-math-smoke-tests
+  SRCS
+    nanf128_test.cpp
+  DEPENDS
+    libc.include.math
+    libc.include.signal
+    libc.src.math.nanf128
+    libc.src.__support.FPUtil.fp_bits
+  # FIXME: The nan tests currently have death tests, which aren't supported for
+  # hermetic tests.
+  UNIT_TEST_ONLY
+)
+
 add_fp_unittest(
   nextafter_test
   SUITE
diff --git a/libc/test/src/math/smoke/nanf128_test.cpp b/libc/test/src/math/smoke/nanf128_test.cpp
new file mode 100644
index 00000000000000..f90f052ec254d8
--- /dev/null
+++ b/libc/test/src/math/smoke/nanf128_test.cpp
@@ -0,0 +1,64 @@
+//===-- Unittests for nanf128 ---------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/FPUtil/FPBits.h"
+#include "src/__support/UInt128.h"
+#include "src/math/nanf128.h"
+#include "test/UnitTest/FPMatcher.h"
+#include "test/UnitTest/Test.h"
+
+class LlvmLibcNanf128Test : public LIBC_NAMESPACE::testing::Test {
+public:
+  using FPBits128 = LIBC_NAMESPACE::fputil::FPBits<float128>;
+  using StorageType = FPBits128::StorageType;
+
+  const UInt128 QUIET_NAN = FPBits128::quiet_nan(
+      LIBC_NAMESPACE::fputil::Sign::POS, 0).uintval();
+
+  const UInt128 ONE = UInt128(1);
+  const UInt128 ZERO = UInt128(0);
+
+  void run_test(const char *input_str, StorageType bits) {
+    float128 result = LIBC_NAMESPACE::nanf128(input_str);
+    auto actual_fp = FPBits128(result);
+    auto expected_fp = FPBits128(bits);
+    EXPECT_EQ(actual_fp.uintval(), expected_fp.uintval());
+  };
+};
+
+TEST_F(LlvmLibcNanf128Test, NCharSeq) {
+  run_test("", QUIET_NAN);
+  run_test("1234", QUIET_NAN | 1234);
+  run_test("0x1234", QUIET_NAN | 0x1234);
+  run_test("2417851639229258349412352", QUIET_NAN | (ONE << 81));
+  run_test("0x200000000000000000000", QUIET_NAN | (ONE << 81));
+  run_test("10384593717069655257060992658440191",
+           QUIET_NAN | (~ZERO & FPBits128::SIG_MASK));
+  run_test("0x1ffffffffffffffffffffffffffff",
+           QUIET_NAN | (~ZERO & FPBits128::SIG_MASK));
+  run_test("10384593717069655257060992658440192", QUIET_NAN);
+  run_test("0x20000000000000000000000000000", QUIET_NAN);
+  run_test("1a", QUIET_NAN);
+  run_test("10000000000000000000000000000000000000000000000000", QUIET_NAN);
+}
+
+TEST_F(LlvmLibcNanf128Test, RandomString) {
+  run_test(" 1234", QUIET_NAN);
+  run_test("-1234", QUIET_NAN);
+  run_test("asd&f", QUIET_NAN);
+  run_test("123 ", QUIET_NAN);
+  run_test("1234567890qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJKLZXCVBNM_",
+           QUIET_NAN);
+}
+
+#ifndef LIBC_HAVE_ADDRESS_SANITIZER
+#include <signal.h>
+TEST_F(LlvmLibcNanf128Test, InvalidInput) {
+  EXPECT_DEATH([] { LIBC_NAMESPACE::nanf128(nullptr); }, WITH_SIGNAL(SIGSEGV));
+}
+#endif // LIBC_HAVE_ADDRESS_SANITIZER

``````````

</details>


https://github.com/llvm/llvm-project/pull/85201

More information about the libc-commits mailing list