[PATCH 3/3] fpu: Simplify OCP FP8 E4M3 NaN classification in parts_canonicalize

Max Chou posted 3 patches 1 month, 2 weeks ago
Maintainers: Aurelien Jarno <aurelien@aurel32.net>, Peter Maydell <peter.maydell@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>
There is a newer version of this series
[PATCH 3/3] fpu: Simplify OCP FP8 E4M3 NaN classification in parts_canonicalize
Posted by Max Chou 1 month, 2 weeks ago
The OCP FP8 E4M3 format has only a single NaN encoding (0x7F/0xFF),
Replace the indirect check via parts_is_snan_frac with a direct
check of no_signaling_nans to make the intent clearer.

Signed-off-by: Max Chou <max.chou@sifive.com>
---
 fpu/softfloat-parts.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
index 3c323c0cec..5ef7e2d921 100644
--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -245,8 +245,8 @@ static void partsN(canonicalize)(FloatPartsN *p, float_status *status,
         case float_expmax_e4m3:
             if (p->frac_hi == 0b111) {
                 frac_shl(p, fmt->frac_shift);
-                p->cls = (parts_is_snan_frac(p->frac_hi, status)
-                          ? float_class_snan : float_class_qnan);
+                p->cls = no_signaling_nans(status) ? float_class_qnan :
+                                                     float_class_snan;
                 return;
             }
             /* otherwise normal */
-- 
2.52.0
Re: [PATCH 3/3] fpu: Simplify OCP FP8 E4M3 NaN classification in parts_canonicalize
Posted by Richard Henderson 1 month, 2 weeks ago
On 2/25/26 22:08, Max Chou wrote:
> The OCP FP8 E4M3 format has only a single NaN encoding (0x7F/0xFF),
> Replace the indirect check via parts_is_snan_frac with a direct
> check of no_signaling_nans to make the intent clearer.
> 
> Signed-off-by: Max Chou <max.chou@sifive.com>
> ---
>   fpu/softfloat-parts.c.inc | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
> index 3c323c0cec..5ef7e2d921 100644
> --- a/fpu/softfloat-parts.c.inc
> +++ b/fpu/softfloat-parts.c.inc
> @@ -245,8 +245,8 @@ static void partsN(canonicalize)(FloatPartsN *p, float_status *status,
>           case float_expmax_e4m3:
>               if (p->frac_hi == 0b111) {
>                   frac_shl(p, fmt->frac_shift);
> -                p->cls = (parts_is_snan_frac(p->frac_hi, status)
> -                          ? float_class_snan : float_class_qnan);
> +                p->cls = no_signaling_nans(status) ? float_class_qnan :
> +                                                     float_class_snan;
>                   return;
>               }
>               /* otherwise normal */

But parts_is_snan_frac also checks snan_bit_is_one.


r~
Re: [PATCH 3/3] fpu: Simplify OCP FP8 E4M3 NaN classification in parts_canonicalize
Posted by Max Chou 1 month, 2 weeks ago
On 2026-02-26 08:06, Richard Henderson wrote:
> On 2/25/26 22:08, Max Chou wrote:
> > The OCP FP8 E4M3 format has only a single NaN encoding (0x7F/0xFF),
> > Replace the indirect check via parts_is_snan_frac with a direct
> > check of no_signaling_nans to make the intent clearer.
> > 
> > Signed-off-by: Max Chou <max.chou@sifive.com>
> > ---
> >   fpu/softfloat-parts.c.inc | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
> > index 3c323c0cec..5ef7e2d921 100644
> > --- a/fpu/softfloat-parts.c.inc
> > +++ b/fpu/softfloat-parts.c.inc
> > @@ -245,8 +245,8 @@ static void partsN(canonicalize)(FloatPartsN *p, float_status *status,
> >           case float_expmax_e4m3:
> >               if (p->frac_hi == 0b111) {
> >                   frac_shl(p, fmt->frac_shift);
> > -                p->cls = (parts_is_snan_frac(p->frac_hi, status)
> > -                          ? float_class_snan : float_class_qnan);
> > +                p->cls = no_signaling_nans(status) ? float_class_qnan :
> > +                                                     float_class_snan;
> >                   return;
> >               }
> >               /* otherwise normal */
> 
> But parts_is_snan_frac also checks snan_bit_is_one.
> 
> 
> r~

Hi Richard,

You're right. I missed that parts_is_snan_frac respects both
snan_bit_is_one and no_signaling_nans flags and I just assumed that user
will use no_signaling_nans flag to define the E4M3 NaN.
The current implementation correctly allows targets to control E4M3 NaN
classification through either mechanism, and this change would have removed
the snan_bit_is_one flexibility.

I'll drop this patch and send a v2 series containing only the two bug fix
patches:

Thanks,
rnax
Re: [PATCH 3/3] fpu: Simplify OCP FP8 E4M3 NaN classification in parts_canonicalize
Posted by Chao Liu 1 month, 2 weeks ago
On Wed, Feb 25, 2026 at 07:08:02PM +0800, Max Chou wrote:
> The OCP FP8 E4M3 format has only a single NaN encoding (0x7F/0xFF),
> Replace the indirect check via parts_is_snan_frac with a direct
> check of no_signaling_nans to make the intent clearer.
> 
> Signed-off-by: Max Chou <max.chou@sifive.com>
> ---
>  fpu/softfloat-parts.c.inc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
> index 3c323c0cec..5ef7e2d921 100644
> --- a/fpu/softfloat-parts.c.inc
> +++ b/fpu/softfloat-parts.c.inc
> @@ -245,8 +245,8 @@ static void partsN(canonicalize)(FloatPartsN *p, float_status *status,
>          case float_expmax_e4m3:
>              if (p->frac_hi == 0b111) {
>                  frac_shl(p, fmt->frac_shift);
> -                p->cls = (parts_is_snan_frac(p->frac_hi, status)
> -                          ? float_class_snan : float_class_qnan);
> +                p->cls = no_signaling_nans(status) ? float_class_qnan :
> +                                                     float_class_snan;
>                  return;
>              }
>              /* otherwise normal */
> -- 
> 2.52.0
> 
> 

Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>

Thanks,
Chao