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

Max Chou posted 17 patches 1 week, 3 days ago
There is a newer version of this series
[PATCH v2 02/17] fpu/softfloat: Refactor IEEE format NaN classification to share code
Posted by Max Chou 1 week, 3 days ago
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.henderson@linaro.org>
Signed-off-by: Max Chou <max.chou@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);
+}
 
 /*----------------------------------------------------------------------------
 | Returns 1 if the half-precision floating-point value `a' is a quiet
@@ -233,17 +249,27 @@ floatx80 floatx80_default_inf(bool zSign, float_status *status)
 
 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);
+}
 
-            return ((a >> 9) & 0x3F) == 0x3F;
-        }
+/*----------------------------------------------------------------------------
+| Internal helper: Determine if bfloat16 NaN is signaling.
+*----------------------------------------------------------------------------*/
+
+static bool bfloat16_is_snan_internal(bfloat16 a, float_status *status)
+{
+    if (!bfloat16_is_any_nan(a)) {
+        return false;
+    }
+    if (no_signaling_nans(status)) {
+        return false;
     }
+    uint16_t frac = a & 0x7F;
+    bool frac_msb = (frac >> 6) & 1;
+    return frac_msb == snan_bit_is_one(status);
 }
 
 /*----------------------------------------------------------------------------
@@ -253,16 +279,10 @@ bool float16_is_quiet_nan(float16 a_, float_status *status)
 
 bool bfloat16_is_quiet_nan(bfloat16 a_, float_status *status)
 {
-    if (no_signaling_nans(status)) {
-        return bfloat16_is_any_nan(a_);
-    } else {
-        uint16_t a = a_;
-        if (snan_bit_is_one(status)) {
-            return (((a >> 6) & 0x1FF) == 0x1FE) && (a & 0x3F);
-        } else {
-            return ((a >> 6) & 0x1FF) == 0x1FF;
-        }
+    if (!bfloat16_is_any_nan(a_)) {
+        return false;
     }
+    return !bfloat16_is_snan_internal(a_, status);
 }
 
 /*----------------------------------------------------------------------------
@@ -272,16 +292,7 @@ bool bfloat16_is_quiet_nan(bfloat16 a_, float_status *status)
 
 bool float16_is_signaling_nan(float16 a_, float_status *status)
 {
-    if (no_signaling_nans(status)) {
-        return 0;
-    } else {
-        uint16_t a = float16_val(a_);
-        if (snan_bit_is_one(status)) {
-            return ((a >> 9) & 0x3F) == 0x3F;
-        } else {
-            return (((a >> 9) & 0x3F) == 0x3E) && (a & 0x1FF);
-        }
-    }
+    return float16_is_snan_internal(a_, status);
 }
 
 /*----------------------------------------------------------------------------
@@ -291,16 +302,24 @@ bool float16_is_signaling_nan(float16 a_, float_status *status)
 
 bool bfloat16_is_signaling_nan(bfloat16 a_, float_status *status)
 {
+    return bfloat16_is_snan_internal(a_, status);
+}
+
+/*----------------------------------------------------------------------------
+| Internal helper: Determine if float32 NaN is signaling.
+*----------------------------------------------------------------------------*/
+
+static bool float32_is_snan_internal(float32 a, float_status *status)
+{
+    if (!float32_is_any_nan(a)) {
+        return false;
+    }
     if (no_signaling_nans(status)) {
-        return 0;
-    } else {
-        uint16_t a = a_;
-        if (snan_bit_is_one(status)) {
-            return ((a >> 6) & 0x1FF) == 0x1FF;
-        } else {
-            return (((a >> 6) & 0x1FF) == 0x1FE) && (a & 0x3F);
-        }
+        return false;
     }
+    uint32_t frac = float32_val(a) & 0x7FFFFF;
+    bool frac_msb = (frac >> 22) & 1;
+    return frac_msb == snan_bit_is_one(status);
 }
 
 /*----------------------------------------------------------------------------
@@ -310,16 +329,10 @@ bool bfloat16_is_signaling_nan(bfloat16 a_, float_status *status)
 
 bool float32_is_quiet_nan(float32 a_, float_status *status)
 {
-    if (no_signaling_nans(status)) {
-        return float32_is_any_nan(a_);
-    } else {
-        uint32_t a = float32_val(a_);
-        if (snan_bit_is_one(status)) {
-            return (((a >> 22) & 0x1FF) == 0x1FE) && (a & 0x003FFFFF);
-        } else {
-            return ((uint32_t)(a << 1) >= 0xFF800000);
-        }
+    if (!float32_is_any_nan(a_)) {
+        return false;
     }
+    return !float32_is_snan_internal(a_, status);
 }
 
 /*----------------------------------------------------------------------------
@@ -329,16 +342,24 @@ bool float32_is_quiet_nan(float32 a_, float_status *status)
 
 bool float32_is_signaling_nan(float32 a_, float_status *status)
 {
+    return float32_is_snan_internal(a_, status);
+}
+
+/*----------------------------------------------------------------------------
+| Internal helper: Determine if float64 NaN is signaling.
+*----------------------------------------------------------------------------*/
+
+static bool float64_is_snan_internal(float64 a, float_status *status)
+{
+    if (!float64_is_any_nan(a)) {
+        return false;
+    }
     if (no_signaling_nans(status)) {
-        return 0;
-    } else {
-        uint32_t a = float32_val(a_);
-        if (snan_bit_is_one(status)) {
-            return ((uint32_t)(a << 1) >= 0xFF800000);
-        } else {
-            return (((a >> 22) & 0x1FF) == 0x1FE) && (a & 0x003FFFFF);
-        }
+        return false;
     }
+    uint64_t frac = float64_val(a) & 0xFFFFFFFFFFFFFULL;
+    bool frac_msb = (frac >> 51) & 1;
+    return frac_msb == snan_bit_is_one(status);
 }
 
 /*----------------------------------------------------------------------------
@@ -348,17 +369,10 @@ bool float32_is_signaling_nan(float32 a_, float_status *status)
 
 bool float64_is_quiet_nan(float64 a_, float_status *status)
 {
-    if (no_signaling_nans(status)) {
-        return float64_is_any_nan(a_);
-    } else {
-        uint64_t a = float64_val(a_);
-        if (snan_bit_is_one(status)) {
-            return (((a >> 51) & 0xFFF) == 0xFFE)
-                && (a & 0x0007FFFFFFFFFFFFULL);
-        } else {
-            return ((a << 1) >= 0xFFF0000000000000ULL);
-        }
+    if (!float64_is_any_nan(a_)) {
+        return false;
     }
+    return !float64_is_snan_internal(a_, status);
 }
 
 /*----------------------------------------------------------------------------
@@ -368,17 +382,7 @@ bool float64_is_quiet_nan(float64 a_, float_status *status)
 
 bool float64_is_signaling_nan(float64 a_, float_status *status)
 {
-    if (no_signaling_nans(status)) {
-        return 0;
-    } else {
-        uint64_t a = float64_val(a_);
-        if (snan_bit_is_one(status)) {
-            return ((a << 1) >= 0xFFF0000000000000ULL);
-        } else {
-            return (((a >> 51) & 0xFFF) == 0xFFE)
-                && (a & UINT64_C(0x0007FFFFFFFFFFFF));
-        }
-    }
+    return float64_is_snan_internal(a_, status);
 }
 
 /*----------------------------------------------------------------------------
-- 
2.52.0
Re: [PATCH v2 02/17] fpu/softfloat: Refactor IEEE format NaN classification to share code
Posted by Richard Henderson 1 week, 3 days ago
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.henderson@linaro.org>
> Signed-off-by: Max Chou <max.chou@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~
Re: [PATCH v2 02/17] fpu/softfloat: Refactor IEEE format NaN classification to share code
Posted by Max Chou 1 week, 3 days ago
On 2026-01-28 12:34, Richard Henderson wrote:
> 
> I think the "internal" functions should already assume any_nan.
> 
> 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~

Hi Richard,

I agree with your suggestions and I will update v3 for this part.

Thanks,
rnax