[libc][math] Adjust rsqrtf16 exception checks.

[lntue]

No description provided.

[llvmbot]

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

---

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

3 Files Affected:

• (modified) libc/src/__support/math/rsqrtf16.h (+27-24)
• (modified) libc/test/src/math/rsqrtf16_test.cpp (+3-2)
• (modified) libc/test/src/math/smoke/rsqrtf16_test.cpp (+14-5)

diff --git a/libc/src/__support/math/rsqrtf16.h b/libc/src/__support/math/rsqrtf16.h
index 8a107b4881a66..30ab58f8a5798 100644
--- a/libc/src/__support/math/rsqrtf16.h
+++ b/libc/src/__support/math/rsqrtf16.h
@@ -28,36 +28,39 @@ LIBC_INLINE static constexpr float16 rsqrtf16(float16 x) {
   using FPBits = fputil::FPBits<float16>;
   FPBits xbits(x);
 
-  const uint16_t x_u = xbits.uintval();
-  const uint16_t x_abs = x_u & 0x7fff;
-  const uint16_t x_sign = x_u >> 15;
+  uint16_t x_u = xbits.uintval();
+  uint16_t x_abs = x_u & 0x7fff;
+
+  constexpr uint16_t INF_BIT = FPBits::inf().uintval();
+
+  // x is 0, inf/nan, or negative.
+  if (LIBC_UNLIKELY(x_u == 0 || x_u >= INF_BIT)) {
+    // x is NaN
+    if (x_abs > INF_BIT) {
+      if (xbits.is_signaling_nan()) {
+        fputil::raise_except_if_required(FE_INVALID);
+        return FPBits::quiet_nan().get_val();
+      }
+      return x;
+    }
+
+    // |x| = 0
+    if (x_abs == 0) {
+      fputil::raise_except_if_required(FE_DIVBYZERO);
+      fputil::set_errno_if_required(ERANGE);
+      return FPBits::inf(xbits.sign()).get_val();
+    }
 
-  // x is NaN
-  if (LIBC_UNLIKELY(xbits.is_nan())) {
-    if (xbits.is_signaling_nan()) {
+    // -inf <= x < 0
+    if (x_u > 0x7fff) {
       fputil::raise_except_if_required(FE_INVALID);
+      fputil::set_errno_if_required(EDOM);
       return FPBits::quiet_nan().get_val();
     }
-    return x;
-  }
 
-  // |x| = 0
-  if (LIBC_UNLIKELY(x_abs == 0x0)) {
-    fputil::raise_except_if_required(FE_DIVBYZERO);
-    fputil::set_errno_if_required(ERANGE);
-    return FPBits::inf(Sign::POS).get_val();
-  }
-
-  // -inf <= x < 0
-  if (LIBC_UNLIKELY(x_sign == 1)) {
-    fputil::raise_except_if_required(FE_INVALID);
-    fputil::set_errno_if_required(EDOM);
-    return FPBits::quiet_nan().get_val();
-  }
-
-  // x = +inf => rsqrt(x) = 0
-  if (LIBC_UNLIKELY(xbits.is_inf()))
+    // x = +inf => rsqrt(x) = 0
     return FPBits::zero().get_val();
+  }
 
   // TODO: add integer based implementation when LIBC_TARGET_CPU_HAS_FPU_FLOAT
   // is not defined
diff --git a/libc/test/src/math/rsqrtf16_test.cpp b/libc/test/src/math/rsqrtf16_test.cpp
index d2f3fe8f49b92..d01c3f94f08cc 100644
--- a/libc/test/src/math/rsqrtf16_test.cpp
+++ b/libc/test/src/math/rsqrtf16_test.cpp
@@ -19,8 +19,9 @@ namespace mpfr = LIBC_NAMESPACE::testing::mpfr;
 static constexpr uint16_t POS_START = 0x0000U;
 static constexpr uint16_t POS_STOP = 0x7c00U;
 
-// Range: [-Inf, 0]
-static constexpr uint16_t NEG_START = 0x8000U;
+// Range: [-Inf, 0)
+// rsqrt(-0.0) is -inf, not the same for mpfr.
+static constexpr uint16_t NEG_START = 0x8001U;
 static constexpr uint16_t NEG_STOP = 0xfc00U;
 
 TEST_F(LlvmLibcRsqrtf16Test, PositiveRange) {
diff --git a/libc/test/src/math/smoke/rsqrtf16_test.cpp b/libc/test/src/math/smoke/rsqrtf16_test.cpp
index be88336e2b195..5eb3e2fd6692c 100644
--- a/libc/test/src/math/smoke/rsqrtf16_test.cpp
+++ b/libc/test/src/math/smoke/rsqrtf16_test.cpp
@@ -7,30 +7,39 @@
 //===----------------------------------------------------------------------===//
 
 #include "hdr/errno_macros.h"
+#include "src/__support/FPUtil/cast.h"
 #include "src/math/rsqrtf16.h"
 #include "test/UnitTest/FPMatcher.h"
 #include "test/UnitTest/Test.h"
 
 using LlvmLibcRsqrtf16Test = LIBC_NAMESPACE::testing::FPTest<float16>;
+using LIBC_NAMESPACE::fputil::cast;
+
 TEST_F(LlvmLibcRsqrtf16Test, SpecialNumbers) {
   EXPECT_FP_EQ(aNaN, LIBC_NAMESPACE::rsqrtf16(aNaN));
   EXPECT_MATH_ERRNO(0);
 
-  EXPECT_FP_EQ_WITH_EXCEPTION(aNaN, LIBC_NAMESPACE::rsqrtf16(sNaN), FE_INVALID);
+  EXPECT_FP_IS_NAN_WITH_EXCEPTION(LIBC_NAMESPACE::rsqrtf16(sNaN), FE_INVALID);
   EXPECT_MATH_ERRNO(0);
 
-  EXPECT_FP_EQ(inf, LIBC_NAMESPACE::rsqrtf16(0.0f));
+  EXPECT_FP_EQ(inf, LIBC_NAMESPACE::rsqrtf16(zero));
+  EXPECT_MATH_ERRNO(ERANGE);
+
+  EXPECT_FP_EQ(neg_inf, LIBC_NAMESPACE::rsqrtf16(neg_zero));
   EXPECT_MATH_ERRNO(ERANGE);
 
-  EXPECT_FP_EQ(1.0f, LIBC_NAMESPACE::rsqrtf16(1.0f));
+  EXPECT_FP_EQ(
+      LIBC_NAMESPACE::fputil::cast<float16>(1.0f),
+      LIBC_NAMESPACE::rsqrtf16(LIBC_NAMESPACE::fputil::cast<float16>(1.0f)));
   EXPECT_MATH_ERRNO(0);
 
-  EXPECT_FP_EQ(0.0f, LIBC_NAMESPACE::rsqrtf16(inf));
+  EXPECT_FP_EQ(zero, LIBC_NAMESPACE::rsqrtf16(inf));
   EXPECT_MATH_ERRNO(0);
 
   EXPECT_FP_EQ(aNaN, LIBC_NAMESPACE::rsqrtf16(neg_inf));
   EXPECT_MATH_ERRNO(EDOM);
 
-  EXPECT_FP_EQ(aNaN, LIBC_NAMESPACE::rsqrtf16(-2.0f));
+  EXPECT_FP_EQ(aNaN, LIBC_NAMESPACE::rsqrtf16(
+                         LIBC_NAMESPACE::fputil::cast<float16>(-2.0f)));
   EXPECT_MATH_ERRNO(EDOM);
 }

[michaelrj-google]

Overall LGTM

[michaelrj-google]

in future: We should replace these magic constants with proper variable names, or provide a way for fpbits to give these values directly.