diff mbox series

fpu/softfloat: set invalid excp flag for RISC-V muladd instructions

Message ID 20210419055614.28361-1-frank.chang@sifive.com
State New
Headers show
Series fpu/softfloat: set invalid excp flag for RISC-V muladd instructions | expand

Commit Message

Frank Chang April 19, 2021, 5:56 a.m. UTC 
From: Frank Chang <frank.chang@sifive.com>

In IEEE 754-2008 spec:
  Invalid operation exception is signaled when doing:
  fusedMultiplyAdd(0, Inf, c) or fusedMultiplyAdd(Inf, 0, c)
  unless c is a quiet NaN; if c is a quiet NaN then it is
  implementation defined whether the invalid operation exception
  is signaled.

In RISC-V Unprivileged ISA spec:
  The fused multiply-add instructions must set the invalid
  operation exception flag when the multiplicands are Inf and
  zero, even when the addend is a quiet NaN.

This commit set invalid operation execption flag for RISC-V when
multiplicands of muladd instructions are Inf and zero.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
 fpu/softfloat-specialize.c.inc | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Richard Henderson April 19, 2021, 3:28 p.m. UTC | #1 
On 4/18/21 10:56 PM, frank.chang@sifive.com wrote:
> +#elif defined(TARGET_RISCV)
> +    /*
> +     * For RISC-V, InvalidOp is set when multiplicands are Inf and zero
> +     * and returns default NaN.
> +     */
> +    if (infzero) {
> +        float_raise(float_flag_invalid, status);
> +        return 3;
> +    }
> +
> +    if (is_nan(a_cls)) {
> +        return 0;
> +    } else if (is_nan(b_cls)) {
> +        return 1;
> +    } else {
> +        return 2;
> +    }

This second half of the function made me go look into the spec to make sure you 
had got that selection right.  But RISCV is always in default_nan mode, so all 
this is unused (and overridden in pick_nan_muladd).

I think for avoidance of confusion, you should use

     if (infzero) {
         float_raise(float_flag_invalid, status);
     }
     return 3; /* default nan */


r~
Frank Chang April 20, 2021, 1:18 a.m. UTC | #2 
On Mon, Apr 19, 2021 at 11:28 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 4/18/21 10:56 PM, frank.chang@sifive.com wrote:
> > +#elif defined(TARGET_RISCV)
> > +    /*
> > +     * For RISC-V, InvalidOp is set when multiplicands are Inf and zero
> > +     * and returns default NaN.
> > +     */
> > +    if (infzero) {
> > +        float_raise(float_flag_invalid, status);
> > +        return 3;
> > +    }
> > +
> > +    if (is_nan(a_cls)) {
> > +        return 0;
> > +    } else if (is_nan(b_cls)) {
> > +        return 1;
> > +    } else {
> > +        return 2;
> > +    }
>
> This second half of the function made me go look into the spec to make
> sure you
> had got that selection right.  But RISCV is always in default_nan mode, so
> all
> this is unused (and overridden in pick_nan_muladd).
>
> I think for avoidance of confusion, you should use
>
>      if (infzero) {
>          float_raise(float_flag_invalid, status);
>      }
>      return 3; /* default nan */
>
>
> r~
>

Sure, I'll update my patch and resend again.

Thanks
Frank Chang
diff mbox series

Patch

diff --git a/fpu/softfloat-specialize.c.inc b/fpu/softfloat-specialize.c.inc
index c2f87addb25..9c37265e20b 100644
--- a/fpu/softfloat-specialize.c.inc
+++ b/fpu/softfloat-specialize.c.inc
@@ -624,6 +624,23 @@  static int pickNaNMulAdd(FloatClass a_cls, FloatClass b_cls, FloatClass c_cls,
     } else {
         return 1;
     }
+#elif defined(TARGET_RISCV)
+    /*
+     * For RISC-V, InvalidOp is set when multiplicands are Inf and zero
+     * and returns default NaN.
+     */
+    if (infzero) {
+        float_raise(float_flag_invalid, status);
+        return 3;
+    }
+
+    if (is_nan(a_cls)) {
+        return 0;
+    } else if (is_nan(b_cls)) {
+        return 1;
+    } else {
+        return 2;
+    }
 #elif defined(TARGET_XTENSA)
     /*
      * For Xtensa, the (inf,zero,nan) case sets InvalidOp and returns