[libc++] Simplify std::strong_order

[philnik777]

This patch does two things:

1. The branches for sizeof(_Dp) == sizeof(int32_t) and sizeof(_Dp) == sizeof(int64_t) are merged, since these are basically identical.
2. The branch where numeric_limits<_Dp>::radix != 2 is removed, since no platform we support (nor any I'm aware of) have a floating point type with radix != 2. This means that the branch is basically dead code.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This patch does two things:

1. The branches for sizeof(_Dp) == sizeof(int32_t) and sizeof(_Dp) == sizeof(int64_t) are merged, since these are basically identical.
2. The branch where numeric_limits<_Dp>::radix != 2 is removed, since no platform we support (nor any I'm aware of) have a floating point type with radix != 2. This means that the branch is basically dead code.

---

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

1 Files Affected:

• (modified) libcxx/include/__compare/strong_order.h (+12-30)

diff --git a/libcxx/include/__compare/strong_order.h b/libcxx/include/__compare/strong_order.h
index 8c363b5638222..ba6de446433ad 100644
--- a/libcxx/include/__compare/strong_order.h
+++ b/libcxx/include/__compare/strong_order.h
@@ -13,7 +13,6 @@
 #include <__compare/compare_three_way.h>
 #include <__compare/ordering.h>
 #include <__config>
-#include <__math/exponential_functions.h>
 #include <__math/traits.h>
 #include <__type_traits/conditional.h>
 #include <__type_traits/decay.h>
@@ -53,38 +52,21 @@ struct __fn {
   template <class _Tp, class _Up, class _Dp = decay_t<_Tp>>
     requires is_same_v<_Dp, decay_t<_Up>> && is_floating_point_v<_Dp>
   _LIBCPP_HIDE_FROM_ABI static constexpr strong_ordering __go(_Tp&& __t, _Up&& __u, __priority_tag<1>) noexcept {
-    if constexpr (numeric_limits<_Dp>::is_iec559 && sizeof(_Dp) == sizeof(int32_t)) {
-      int32_t __rx = std::bit_cast<int32_t>(__t);
-      int32_t __ry = std::bit_cast<int32_t>(__u);
-      __rx         = (__rx < 0) ? (numeric_limits<int32_t>::min() - __rx - 1) : __rx;
-      __ry         = (__ry < 0) ? (numeric_limits<int32_t>::min() - __ry - 1) : __ry;
-      return (__rx <=> __ry);
-    } else if constexpr (numeric_limits<_Dp>::is_iec559 && sizeof(_Dp) == sizeof(int64_t)) {
-      int64_t __rx = std::bit_cast<int64_t>(__t);
-      int64_t __ry = std::bit_cast<int64_t>(__u);
-      __rx         = (__rx < 0) ? (numeric_limits<int64_t>::min() - __rx - 1) : __rx;
-      __ry         = (__ry < 0) ? (numeric_limits<int64_t>::min() - __ry - 1) : __ry;
+    if constexpr (numeric_limits<_Dp>::is_iec559 &&
+                  (sizeof(_Dp) == sizeof(int32_t) || sizeof(_Dp) == sizeof(int64_t))) {
+      using _IntT = conditional_t<sizeof(_Dp) == sizeof(int32_t), int32_t, int64_t>;
+      _IntT __rx  = std::bit_cast<_IntT>(__t);
+      _IntT __ry  = std::bit_cast<_IntT>(__u);
+      __rx        = (__rx < 0) ? (numeric_limits<_IntT>::min() - __rx - 1) : __rx;
+      __ry        = (__ry < 0) ? (numeric_limits<_IntT>::min() - __ry - 1) : __ry;
       return (__rx <=> __ry);
     } else if (__t < __u) {
       return strong_ordering::less;
     } else if (__t > __u) {
       return strong_ordering::greater;
     } else if (__t == __u) {
-      if constexpr (numeric_limits<_Dp>::radix == 2) {
-        return __math::signbit(__u) <=> __math::signbit(__t);
-      } else {
-        // This is bullet 3 of the IEEE754 algorithm, relevant
-        // only for decimal floating-point;
-        // see https://stackoverflow.com/questions/69068075/
-        if (__t == 0 || __math::isinf(__t)) {
-          return __math::signbit(__u) <=> __math::signbit(__t);
-        } else {
-          int __texp, __uexp;
-          (void)__math::frexp(__t, &__texp);
-          (void)__math::frexp(__u, &__uexp);
-          return (__t < 0) ? (__texp <=> __uexp) : (__uexp <=> __texp);
-        }
-      }
+      static_assert(numeric_limits<_Dp>::radix == 2, "floating point type with a radix other than 2?");
+      return __math::signbit(__u) <=> __math::signbit(__t);
     } else {
       // They're unordered, so one of them must be a NAN.
       // The order is -QNAN, -SNAN, numbers, +SNAN, +QNAN.
@@ -93,9 +75,9 @@ struct __fn {
       bool __t_is_negative = __math::signbit(__t);
       bool __u_is_negative = __math::signbit(__u);
       using _IntType =
-          conditional_t< sizeof(__t) == sizeof(int32_t),
-                         int32_t,
-                         conditional_t< sizeof(__t) == sizeof(int64_t), int64_t, void> >;
+          conditional_t<sizeof(__t) == sizeof(int32_t),
+                        int32_t,
+                        conditional_t<sizeof(__t) == sizeof(int64_t), int64_t, void>>;
       if constexpr (is_same_v<_IntType, void>) {
         static_assert(sizeof(_Dp) == 0, "std::strong_order is unimplemented for this floating-point type");
       } else if (__t_is_nan && __u_is_nan) {

[frederick-vs-ja]

No change requested. Looks like that we can generalize this branch for other FP types later. (For float{16,128}_t and possibly bfloat16_t, but not Intel 80-bit long double or double-double format.)

[philnik777]

Yeah, this is part of the reason I did this change - once we work on supporting the optional floating point types it will probably simplify this place.

[philnik777]

@frederick-vs-ja Please don't merge my PRs. I wanted Louis to have a look first.

[frederick-vs-ja]

Oh, I'm sorry...😿