[libc][NFC] Simplify logic in sqrt

[gchatelet]

No description provided.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

---

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

2 Files Affected:

• (modified) libc/src/__support/FPUtil/generic/sqrt.h (+5-11)
• (modified) libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h (+5-11)

diff --git a/libc/src/__support/FPUtil/generic/sqrt.h b/libc/src/__support/FPUtil/generic/sqrt.h
index 702de3f04a9bf..15ac1d3545e95 100644
--- a/libc/src/__support/FPUtil/generic/sqrt.h
+++ b/libc/src/__support/FPUtil/generic/sqrt.h
@@ -78,20 +78,14 @@ LIBC_INLINE cpp::enable_if_t<cpp::is_floating_point_v<T>, T> sqrt(T x) {
 
     FPBits_t bits(x);
 
-    if (bits.is_inf_or_nan()) {
-      if (bits.is_neg() && (bits.get_mantissa() == 0)) {
-        // sqrt(-Inf) = NaN
-        return FLT_NAN;
-      } else {
-        // sqrt(NaN) = NaN
-        // sqrt(+Inf) = +Inf
-        return x;
-      }
-    } else if (bits.is_zero()) {
+    if (bits == FPBits_t::inf(Sign::POS) || bits.is_zero()) {
+      // sqrt(+Inf) = +Inf
       // sqrt(+0) = +0
       // sqrt(-0) = -0
       return x;
-    } else if (bits.is_neg()) {
+    } else if (bits.is_inf_or_nan() || bits.is_neg()) {
+      // sqrt(-Inf) = NaN
+      // sqrt(NaN) = NaN
       // sqrt( negative numbers ) = NaN
       return FLT_NAN;
     } else {
diff --git a/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h b/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
index 74a536c04817c..2146019011986 100644
--- a/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
+++ b/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
@@ -43,20 +43,14 @@ LIBC_INLINE long double sqrt(long double x) {
 
   LDBits bits(x);
 
-  if (bits.is_inf_or_nan()) {
-    if (bits.is_neg() && (bits.get_mantissa() == 0)) {
-      // sqrt(-Inf) = NaN
-      return LDNAN;
-    } else {
-      // sqrt(NaN) = NaN
-      // sqrt(+Inf) = +Inf
-      return x;
-    }
-  } else if (bits.is_zero()) {
+  if (bits == LDBits::inf(Sign::POS) || bits.is_zero()) {
+    // sqrt(+Inf) = +Inf
     // sqrt(+0) = +0
     // sqrt(-0) = -0
     return x;
-  } else if (bits.is_neg()) {
+  } else if (bits.is_inf_or_nan() || bits.is_neg()) {
+    // sqrt(-Inf) = NaN
+    // sqrt(NaN) = NaN
     // sqrt( negative numbers ) = NaN
     return LDNAN;
   } else {

[lntue]

This will change the NaN payload when x is NaN. When x is NaN we should return it as-is.

[gchatelet]

Is this required by the standard?
It is not explicit from the cppreference article.

[gchatelet]

The C standard (ISO/IEC 9899:201x) says
5.2.4.2.2
A quiet NaN propagates through almost every arithmetic operation without raising a floating-point exception; a signaling NaN generally raises a floating-point exception when occurring as an arithmetic operand.

I'm still unclear about the exact semantics as it doesn't tell whether it should be the exact same NaN or just NaN read as "the floating point class of NaN values".

[gchatelet]

Anyways, I think returning the same NaN makes more sense so I've changed the code accordingly.

[lntue]

Yes, returning NaN as-is helps with the signaling / quiet NaN semantic, and also it follows the IEEE 754's recommendation:

To facilitate propagation of diagnostic information contained in NaNs,
as much of that information as possible should be preserved in NaN 
results of operations.