Re: [PATCH v2 02/17] fpu/softfloat: Refactor IEEE format NaN classification to share code

[Richard Henderson]

On 1/27/26 17:37, Max Chou wrote:

The floatN_is_[quiet|signaling]_nan functions for IEEE formats
(float16, bfloat16, float32, float64) contain duplicated logic that
should be shared.
This commit introduces internal helper functions:
[float16|bfloat16|float32|float64]_is_snan_internal
that determine if a NaN is signaling.

Suggested-by: Richard Henderson <richard.hender...@linaro.org>
Signed-off-by: Max Chou <max.c...@sifive.com>
---
  fpu/softfloat-specialize.c.inc | 152 +++++++++++++++++----------------
  1 file changed, 78 insertions(+), 74 deletions(-)

diff --git a/fpu/softfloat-specialize.c.inc b/fpu/softfloat-specialize.c.inc
index ba4fa08b7b..ce7315c996 100644
--- a/fpu/softfloat-specialize.c.inc
+++ b/fpu/softfloat-specialize.c.inc
@@ -225,6 +225,22 @@ floatx80 floatx80_default_inf(bool zSign, float_status 
*status)
      bool z = status->floatx80_behaviour & 
floatx80_default_inf_int_bit_is_zero;
      return packFloatx80(zSign, 0x7fff, z ? 0 : (1ULL << 63));
  }
+/*----------------------------------------------------------------------------
+| Internal helper: Determine if float16 NaN is signaling.
+*----------------------------------------------------------------------------*/
+
+static bool float16_is_snan_internal(float16 a, float_status *status)
+{
+    if (!float16_is_any_nan(a)) {
+        return false;
+    }
+    if (no_signaling_nans(status)) {
+        return false;
+    }
+    uint16_t frac = float16_val(a) & 0x3FF;
+    bool frac_msb = (frac >> 9) & 1;
+    return frac_msb == snan_bit_is_one(status);
+}

I think the "internal" functions should already assume any_nan.

  bool float16_is_quiet_nan(float16 a_, float_status *status)
  {
-    if (no_signaling_nans(status)) {
-        return float16_is_any_nan(a_);
-    } else {
-        uint16_t a = float16_val(a_);
-        if (snan_bit_is_one(status)) {
-            return (((a >> 9) & 0x3F) == 0x3E) && (a & 0x1FF);
-        } else {
+    if (!float16_is_any_nan(a_)) {
+        return false;
+    }
+    return !float16_is_snan_internal(a_, status);
+}

Otherwise you're double-testing it in the quiet_nan tests.

Perhaps the intent would be clearer by naming them <type>_nan_is_snan.

Perhaps clearer to merge the expressions:

    return float16_is_any_nan(a) && !float16_nan_is_snan(a)
and
    return float16_is_any_nan(a) && float16_nan_is_snan(a)

for the is_quiet_nan and is_signaling_nan functions respectively?


r~