[libc][NFC] unify nextafter and nexttoward code

[michaelrj-google]

Previously the nextafter and nexttoward implementations were almost
identical, with the exception of whether or not the second argument was
a template or just long double. This patch unifies them by making the
two argument templates independent.

[llvmbot]

@llvm/pr-subscribers-libc

Author: None (michaelrj-google)

Changes

Previously the nextafter and nexttoward implementations were almost
identical, with the exception of whether or not the second argument was
a template or just long double. This patch unifies them by making the
two argument templates independent.

---

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

4 Files Affected:

• (modified) libc/src/__support/FPUtil/ManipulationFunctions.h (+10-43)
• (modified) libc/src/math/generic/nexttoward.cpp (+3-1)
• (modified) libc/src/math/generic/nexttowardf.cpp (+3-1)
• (modified) libc/src/math/generic/nexttowardl.cpp (+2-3)

diff --git a/libc/src/__support/FPUtil/ManipulationFunctions.h b/libc/src/__support/FPUtil/ManipulationFunctions.h
index f1768885d4ca500..6624d8c34d29385 100644
--- a/libc/src/__support/FPUtil/ManipulationFunctions.h
+++ b/libc/src/__support/FPUtil/ManipulationFunctions.h
@@ -144,59 +144,26 @@ LIBC_INLINE T ldexp(T x, int exp) {
   return normal;
 }
 
-template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
-LIBC_INLINE T nextafter(T from, T to) {
-  FPBits<T> from_bits(from);
-  if (from_bits.is_nan())
-    return from;
-
-  FPBits<T> to_bits(to);
-  if (to_bits.is_nan())
-    return to;
-
-  if (from == to)
-    return to;
-
-  using UIntType = typename FPBits<T>::UIntType;
-  UIntType int_val = from_bits.uintval();
-  UIntType sign_mask = (UIntType(1) << (sizeof(T) * 8 - 1));
-  if (from != T(0.0)) {
-    if ((from < to) == (from > T(0.0))) {
-      ++int_val;
-    } else {
-      --int_val;
-    }
-  } else {
-    int_val = (to_bits.uintval() & sign_mask) + UIntType(1);
-  }
-
-  UIntType exponent_bits = int_val & FloatProperties<T>::EXPONENT_MASK;
-  if (exponent_bits == UIntType(0))
-    raise_except_if_required(FE_UNDERFLOW | FE_INEXACT);
-  else if (exponent_bits == FloatProperties<T>::EXPONENT_MASK)
-    raise_except_if_required(FE_OVERFLOW | FE_INEXACT);
-
-  return cpp::bit_cast<T>(int_val);
-}
-
-template <typename T>
-LIBC_INLINE cpp::enable_if_t<cpp::is_floating_point_v<T>, T>
-nexttoward(T from, long double to) {
+template <
+    typename T, typename U,
+    cpp::enable_if_t<cpp::is_floating_point_v<T> && cpp::is_floating_point_v<U>,
+                     int> = 0>
+LIBC_INLINE T nextafter(T from, U to) {
   FPBits<T> from_bits(from);
   if (from_bits.is_nan())
     return from;
 
-  FPBits<long double> to_bits(to);
+  FPBits<U> to_bits(to);
   if (to_bits.is_nan())
-    return to;
+    return static_cast<T>(to);
 
-  if ((long double)from == to)
-    return to;
+  if (static_cast<U>(from) == to)
+    return static_cast<T>(to);
 
   using UIntType = typename FPBits<T>::UIntType;
   UIntType int_val = from_bits.uintval();
   if (from != T(0.0)) {
-    if ((from < to) == (from > T(0.0))) {
+    if ((static_cast<U>(from) < to) == (from > T(0.0))) {
       ++int_val;
     } else {
       --int_val;
diff --git a/libc/src/math/generic/nexttoward.cpp b/libc/src/math/generic/nexttoward.cpp
index 38b45d6d2f65d9c..ce3e4e6a69ad231 100644
--- a/libc/src/math/generic/nexttoward.cpp
+++ b/libc/src/math/generic/nexttoward.cpp
@@ -13,7 +13,9 @@
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(double, nexttoward, (double x, long double y)) {
-  return fputil::nexttoward(x, y);
+  // We can reuse the nextafter implementation because the internal nextafter is
+  // templated on the types of the arguments.
+  return fputil::nextafter(x, y);
 }
 
 } // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/generic/nexttowardf.cpp b/libc/src/math/generic/nexttowardf.cpp
index 59a9f805a6946a1..3b0762c50160ac1 100644
--- a/libc/src/math/generic/nexttowardf.cpp
+++ b/libc/src/math/generic/nexttowardf.cpp
@@ -13,7 +13,9 @@
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(float, nexttowardf, (float x, long double y)) {
-  return fputil::nexttoward(x, y);
+  // We can reuse the nextafter implementation because the internal nextafter is
+  // templated on the types of the arguments.
+  return fputil::nextafter(x, y);
 }
 
 } // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/generic/nexttowardl.cpp b/libc/src/math/generic/nexttowardl.cpp
index 0c887ae0671bc98..e9f7f8390760342 100644
--- a/libc/src/math/generic/nexttowardl.cpp
+++ b/libc/src/math/generic/nexttowardl.cpp
@@ -13,9 +13,8 @@
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(long double, nexttowardl, (long double x, long double y)) {
-  // We can reuse the nextafter implementation because nexttoward behaves
-  // exactly same as nextafter in case of long doubles. Also, we have explcitly
-  // handled the special 80-bit long doubles in nextafter implementation.
+  // We can reuse the nextafter implementation because the internal nextafter is
+  // templated on the types of the arguments.
   return fputil::nextafter(x, y);
 }
 

[lntue]

Can you replace T(0.0) with FPBits<T>::zero()?

[nishantwrp]

Just a side note: This would work only if U is equal or higher precision than T. (Which is OK for our usecase but should we add a comment or maybe some condition in enable_if that ensures this?)

cc: @michaelrj-google @lntue

[michaelrj-google]

That would be good, yes. I'd say adding both a comment and a condition in the enableif such as sizeof(T) >= sizeof(U) would be the ideal solution.