[RFC PATCH] softfloat: Allow runtime choice of default NaN value

Peter Maydell posted 1 patch 1 week, 1 day ago
include/fpu/softfloat-types.h  | 10 ++++++
fpu/softfloat-specialize.c.inc | 61 ++++++++++++++++++++++------------
2 files changed, 49 insertions(+), 22 deletions(-)
[RFC PATCH] softfloat: Allow runtime choice of default NaN value
Posted by Peter Maydell 1 week, 1 day ago
Currently we hardcode the default NaN value in parts64_default_nan()
using a compile-time ifdef ladder. This is awkward for two cases:
 * for single-QEMU-binary we can't hard-code target-specifics like this
 * for Arm FEAT_AFP the default NaN value depends on FPCR.AH
   (specifically the sign bit is different)

Add a field to float_status to specify the default NaN value; fall
back to the old ifdef behaviour if these are not set.

The default NaN value is specified by setting a uint8_t to a
pattern corresponding to the sign and upper fraction parts of
the NaN; the lower bits of the fraction are set from bit 0 of
the pattern.

This is an RFC to ask for opinions on whether this is the right
way to let the target set its default NaN. I can't decide whether
I think encoding it into a uint8_t like that is clever, or merely
too clever :-)

The other options would be e.g. separate
  bool default_nan_sign;
  uint8_t default_nan_frac;
or an enum for the frac possibilities ("default_nan_frac_01__1",
"default_nan_frac_010__0", etc ???). The ones we currently need are:

 frac 1....1    sign 0 (m68k, sparc), sign 1 (hexagon)
  (with sign 0 for m68k, sparc; sign 1 for hexagon)
 frac 10...0
  (with sign 0 for many targets, sign 1 for i386, microblaze)
 frac 01...1
  (always with sign 0; sh4, some MIPS configs)
 frac 010..0
  (always with sign 0; hppa)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/fpu/softfloat-types.h  | 10 ++++++
 fpu/softfloat-specialize.c.inc | 61 ++++++++++++++++++++++------------
 2 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 6e237fb697d..0a0508fe2d8 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -285,6 +285,16 @@ typedef struct float_status {
     /* should denormalised inputs go to zero and set the input_denormal flag? */
     bool flush_inputs_to_zero;
     bool default_nan_mode;
+    /*
+     * The pattern to use for the default NaN. Here the high bit specifies
+     * the default NaN's sign bit, and bits 6..0 specify the high bits of the
+     * fractional part. The low bits of the fractional part are copies of bit 0.
+     * The exponent of the default NaN is (as for any NaN) always all 1s.
+     * Note that a value of 0 here is not a valid NaN. The target must set
+     * this to the correct non-zero value, or we will assert when trying to
+     * create a default NaN.
+     */
+    uint8_t default_nan_pattern;
     /*
      * The flags below are not used on all specializations and may
      * constant fold away (see snan_bit_is_one()/no_signalling_nans() in
diff --git a/fpu/softfloat-specialize.c.inc b/fpu/softfloat-specialize.c.inc
index 353b524d2de..90b6c18a733 100644
--- a/fpu/softfloat-specialize.c.inc
+++ b/fpu/softfloat-specialize.c.inc
@@ -133,35 +133,52 @@ static void parts64_default_nan(FloatParts64 *p, float_status *status)
 {
     bool sign = 0;
     uint64_t frac;
+    uint8_t dnan_pattern = status->default_nan_pattern;
 
+    if (dnan_pattern == 0) {
 #if defined(TARGET_SPARC) || defined(TARGET_M68K)
-    /* !snan_bit_is_one, set all bits */
-    frac = (1ULL << DECOMPOSED_BINARY_POINT) - 1;
-#elif defined(TARGET_I386) || defined(TARGET_X86_64) \
+        /* Sign bit clear, all frac bits set */
+        dnan_pattern = 0b01111111;
+#elif defined(TARGET_I386) || defined(TARGET_X86_64)    \
     || defined(TARGET_MICROBLAZE)
-    /* !snan_bit_is_one, set sign and msb */
-    frac = 1ULL << (DECOMPOSED_BINARY_POINT - 1);
-    sign = 1;
+        /* Sign bit set, most significant frac bit set */
+        dnan_pattern = 0b11000000;
 #elif defined(TARGET_HPPA)
-    /* snan_bit_is_one, set msb-1.  */
-    frac = 1ULL << (DECOMPOSED_BINARY_POINT - 2);
+        /* Sign bit clear, msb-1 frac bit set */
+        dnan_pattern = 0b00100000;
 #elif defined(TARGET_HEXAGON)
-    sign = 1;
-    frac = ~0ULL;
+        /*
+         * Sign bit set, all frac bits set. This is an odd special case,
+         * where our value doesn't match up with the snan_bit_is_one setting.
+         * This is because for Hexagon the returned value is always -1,
+         * not a real NaN value.
+         */
+        dnan_pattern = 0b11111111;
 #else
-    /*
-     * This case is true for Alpha, ARM, MIPS, OpenRISC, PPC, RISC-V,
-     * S390, SH4, TriCore, and Xtensa.  Our other supported targets
-     * do not have floating-point.
-     */
-    if (snan_bit_is_one(status)) {
-        /* set all bits other than msb */
-        frac = (1ULL << (DECOMPOSED_BINARY_POINT - 1)) - 1;
-    } else {
-        /* set msb */
-        frac = 1ULL << (DECOMPOSED_BINARY_POINT - 1);
-    }
+        /*
+         * This case is true for Alpha, ARM, MIPS, OpenRISC, PPC, RISC-V,
+         * S390, SH4, TriCore, and Xtensa.  Our other supported targets
+         * do not have floating-point.
+         */
+        if (snan_bit_is_one(status)) {
+            /* sign bit clear, set all frac bits other than msb */
+            dnan_pattern = 0b00111111;
+        } else {
+            /* sign bit clear, set frac msb */
+            dnan_pattern = 0b01000000;
+        }
 #endif
+    }
+
+    sign = status->default_nan_pattern >> 7;
+    /*
+     * Place default_nan_pattern [6:0] into bits [62:56],
+     * and replicate bit [0] down into [55:0]
+     */
+    frac = deposit64(0, DECOMPOSED_BINARY_POINT - 7, 7,
+                     status->default_nan_pattern);
+    frac = deposit64(frac, 0, DECOMPOSED_BINARY_POINT - 7,
+                     -(status->default_nan_pattern & 1));
 
     *p = (FloatParts64) {
         .cls = float_class_qnan,
-- 
2.34.1
Re: [RFC PATCH] softfloat: Allow runtime choice of default NaN value
Posted by Richard Henderson 1 week, 1 day ago
On 11/28/24 08:54, Peter Maydell wrote:
> Currently we hardcode the default NaN value in parts64_default_nan()
> using a compile-time ifdef ladder. This is awkward for two cases:
>   * for single-QEMU-binary we can't hard-code target-specifics like this
>   * for Arm FEAT_AFP the default NaN value depends on FPCR.AH
>     (specifically the sign bit is different)
> 
> Add a field to float_status to specify the default NaN value; fall
> back to the old ifdef behaviour if these are not set.
> 
> The default NaN value is specified by setting a uint8_t to a
> pattern corresponding to the sign and upper fraction parts of
> the NaN; the lower bits of the fraction are set from bit 0 of
> the pattern.
> 
> This is an RFC to ask for opinions on whether this is the right
> way to let the target set its default NaN. I can't decide whether
> I think encoding it into a uint8_t like that is clever, or merely
> too clever :-)

I think that's perfectly fine.

>   #elif defined(TARGET_HEXAGON)
> -    sign = 1;
> -    frac = ~0ULL;
> +        /*
> +         * Sign bit set, all frac bits set. This is an odd special case,
> +         * where our value doesn't match up with the snan_bit_is_one setting.
> +         * This is because for Hexagon the returned value is always -1,
> +         * not a real NaN value.
> +         */
> +        dnan_pattern = 0b11111111;

It's a real (signalling) nan value -- the oddity is that the default nan is not quiet.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~