1. Patchew
  2. QEMU
  3. [PATCH v5 00/30] fpu: Export some internals for targets
  4. View patch

[PATCH v5 29/30] fpu: Fix NaN encoding for E4M3 in parts64_uncanon

Richard Henderson posted 30 patches 2 months, 3 weeks ago
Maintainers: Aurelien Jarno <aurelien@aurel32.net>, Peter Maydell <peter.maydell@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Brian Cain <brian.cain@oss.qualcomm.com>, Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>, Helge Deller <deller@gmx.de>, Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Laurent Vivier <laurent@vivier.eu>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <arikalo@gmail.com>, Nicholas Piggin <npiggin@gmail.com>, Chinmay Rath <rathc@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Cornelia Huck <cohuck@redhat.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Max Filippov <jcmvbkbc@gmail.com>
[PATCH v5 29/30] fpu: Fix NaN encoding for E4M3 in parts64_uncanon
Posted by Richard Henderson 2 months, 3 weeks ago 
There is only one NaN fractional encoding for E4M3.  Retain the
incoming sign, but force the outgoing fraction to the unique value.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c           |  2 ++
 fpu/softfloat-parts.c.inc | 29 ++++++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index be6b02d866..2c3bf01213 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -499,6 +499,8 @@ const FloatFmt float8_e4m3_params = {
 
 /* 110 << frac_shift, with the implicit bit set */
 #define E4M3_NORMAL_FRAC_MAX  0xe000000000000000ull
+/* 111 << frac_shift, no implicit bit */
+#define E4M3_NAN_FRAC         0x7000000000000000ull
 
 const FloatFmt float8_e5m2_params = {
     FLOAT_PARAMS(5, 2)
diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
index 559e40d196..d6687df982 100644
--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -278,11 +278,16 @@ static void partsN(uncanon_e4m3_overflow)(FloatPartsN *p, float_status *s,
                                           const FloatFmt *fmt, bool saturate)
 {
     assert(N == 64);
+    p->exp = fmt->exp_max;
     if (saturate) {
-        p->exp = fmt->exp_max;
         p->frac_hi = E4M3_NORMAL_FRAC_MAX;
     } else {
-        *p = partsN(default_nan)(s);
+        /*
+         * The class isn't actually used after this point in uncanon,
+         * but for clarity while debugging, don't leave it set to normal.
+         */
+        p->cls = float_class_qnan;
+        p->frac_hi = E4M3_NAN_FRAC;
     }
 }
 
@@ -507,10 +512,24 @@ static void partsN(uncanon)(FloatPartsN *p, float_status *s,
             return;
         case float_class_qnan:
         case float_class_snan:
-            assert(fmt->exp_max_kind != float_expmax_normal);
             p->exp = fmt->exp_max;
-            fracN(shr)(p, fmt->frac_shift);
-            return;
+            switch (fmt->exp_max_kind) {
+            case float_expmax_e4m3:
+                /*
+                 * There is only one NaN encoding for E4M3, and with a
+                 * conversion from another format, the input NaN fraction
+                 * may not apply.
+                 */
+                assert(N == 64);
+                p->frac_hi = E4M3_NAN_FRAC;
+                /* fall through */
+            case float_expmax_ieee:
+                fracN(shr)(p, fmt->frac_shift);
+                return;
+            case float_expmax_normal:
+                break;
+            }
+            g_assert_not_reached();
         default:
             break;
         }
-- 
2.43.0
Re: [PATCH v5 29/30] fpu: Fix NaN encoding for E4M3 in parts64_uncanon
Posted by Peter Maydell 2 months, 3 weeks ago 
On Wed, 20 May 2026 at 18:18, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> There is only one NaN fractional encoding for E4M3.  Retain the
> incoming sign, but force the outgoing fraction to the unique value.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c           |  2 ++
>  fpu/softfloat-parts.c.inc | 29 ++++++++++++++++++++++++-----
>  2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index be6b02d866..2c3bf01213 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -499,6 +499,8 @@ const FloatFmt float8_e4m3_params = {
>
>  /* 110 << frac_shift, with the implicit bit set */
>  #define E4M3_NORMAL_FRAC_MAX  0xe000000000000000ull
> +/* 111 << frac_shift, no implicit bit */
> +#define E4M3_NAN_FRAC         0x7000000000000000ull
>
>  const FloatFmt float8_e5m2_params = {
>      FLOAT_PARAMS(5, 2)
> diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
> index 559e40d196..d6687df982 100644
> --- a/fpu/softfloat-parts.c.inc
> +++ b/fpu/softfloat-parts.c.inc
> @@ -278,11 +278,16 @@ static void partsN(uncanon_e4m3_overflow)(FloatPartsN *p, float_status *s,
>                                            const FloatFmt *fmt, bool saturate)
>  {
>      assert(N == 64);
> +    p->exp = fmt->exp_max;
>      if (saturate) {
> -        p->exp = fmt->exp_max;
>          p->frac_hi = E4M3_NORMAL_FRAC_MAX;
>      } else {
> -        *p = partsN(default_nan)(s);
> +        /*
> +         * The class isn't actually used after this point in uncanon,
> +         * but for clarity while debugging, don't leave it set to normal.
> +         */
> +        p->cls = float_class_qnan;
> +        p->frac_hi = E4M3_NAN_FRAC;
>      }
>  }
>
> @@ -507,10 +512,24 @@ static void partsN(uncanon)(FloatPartsN *p, float_status *s,
>              return;
>          case float_class_qnan:
>          case float_class_snan:
> -            assert(fmt->exp_max_kind != float_expmax_normal);
>              p->exp = fmt->exp_max;
> -            fracN(shr)(p, fmt->frac_shift);
> -            return;
> +            switch (fmt->exp_max_kind) {
> +            case float_expmax_e4m3:
> +                /*
> +                 * There is only one NaN encoding for E4M3, and with a
> +                 * conversion from another format, the input NaN fraction
> +                 * may not apply.
> +                 */
> +                assert(N == 64);
> +                p->frac_hi = E4M3_NAN_FRAC;
> +                /* fall through */
> +            case float_expmax_ieee:
> +                fracN(shr)(p, fmt->frac_shift);
> +                return;
> +            case float_expmax_normal:

We used to assert() that exp_max_kind wasn't "expmax_normal",
but now we don't. What does it mean to have a FloatPartsN that
says it's a NaN when the format says there isn't a NaN
representation ? Either way, the "break" here means we
won't treat it like we do NaNs and we won't treat it
like we do normals either...

> +                break;
> +            }
> +            g_assert_not_reached();
>          default:
>              break;
>          }

thanks
-- PMM
Re: [PATCH v5 29/30] fpu: Fix NaN encoding for E4M3 in parts64_uncanon
Posted by Peter Maydell 2 months, 3 weeks ago 
On Wed, 20 May 2026 at 19:20, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 20 May 2026 at 18:18, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > There is only one NaN fractional encoding for E4M3.  Retain the
> > incoming sign, but force the outgoing fraction to the unique value.
> >
> > Reported-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> > @@ -507,10 +512,24 @@ static void partsN(uncanon)(FloatPartsN *p, float_status *s,
> >              return;
> >          case float_class_qnan:
> >          case float_class_snan:
> > -            assert(fmt->exp_max_kind != float_expmax_normal);
> >              p->exp = fmt->exp_max;
> > -            fracN(shr)(p, fmt->frac_shift);
> > -            return;
> > +            switch (fmt->exp_max_kind) {
> > +            case float_expmax_e4m3:
> > +                /*
> > +                 * There is only one NaN encoding for E4M3, and with a
> > +                 * conversion from another format, the input NaN fraction
> > +                 * may not apply.
> > +                 */
> > +                assert(N == 64);
> > +                p->frac_hi = E4M3_NAN_FRAC;
> > +                /* fall through */
> > +            case float_expmax_ieee:
> > +                fracN(shr)(p, fmt->frac_shift);
> > +                return;
> > +            case float_expmax_normal:
>
> We used to assert() that exp_max_kind wasn't "expmax_normal",
> but now we don't. What does it mean to have a FloatPartsN that
> says it's a NaN when the format says there isn't a NaN
> representation ? Either way, the "break" here means we
> won't treat it like we do NaNs and we won't treat it
> like we do normals either...
>
> > +                break;
> > +            }
> > +            g_assert_not_reached();

...ah, I misread it. I think it would be clearer to
have the float_expmax_normal case be "g_assert_not_reached()"
rather than "break, and rely on the thing immediately next
after the switch to be g_assert_not_reached()", because
it more obviously says "this case can't occur".

-- PMM
Re: [PATCH v5 29/30] fpu: Fix NaN encoding for E4M3 in parts64_uncanon
Posted by Richard Henderson 2 months, 3 weeks ago 
On 5/20/26 11:21, Peter Maydell wrote:
>>> +                break;
>>> +            }
>>> +            g_assert_not_reached();
> 
> ...ah, I misread it. I think it would be clearer to
> have the float_expmax_normal case be "g_assert_not_reached()"
> rather than "break, and rely on the thing immediately next
> after the switch to be g_assert_not_reached()", because
> it more obviously says "this case can't occur".

Mm.  I wrote the switch to let -Wall diagnose a missing enumerator, rather than

     case float_expmax_normal:
     default:
         g_assert_not_reached();


r~
Re: [PATCH v5 29/30] fpu: Fix NaN encoding for E4M3 in parts64_uncanon
Posted by Peter Maydell 2 months, 3 weeks ago 
On Wed, 20 May 2026 at 19:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/20/26 11:21, Peter Maydell wrote:
> >>> +                break;
> >>> +            }
> >>> +            g_assert_not_reached();
> >
> > ...ah, I misread it. I think it would be clearer to
> > have the float_expmax_normal case be "g_assert_not_reached()"
> > rather than "break, and rely on the thing immediately next
> > after the switch to be g_assert_not_reached()", because
> > it more obviously says "this case can't occur".
>
> Mm.  I wrote the switch to let -Wall diagnose a missing enumerator, rather than
>
>      case float_expmax_normal:
>      default:
>          g_assert_not_reached();

Since this is the last unreviewed patch in this series,
you can have my

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.