Re: [PATCH] softfloat: Handle default NaN mode after pickNaNMulAdd, not before

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> It is implementation defined whether a multiply-add of
> (0,inf,qnan) or (inf,0,qnan) raises InvalidaOperation or
> not, so we let the target-specific pickNaNMulAdd function
> handle this. This means that we must do the "return the
> default NaN in default NaN mode" check after the call,
> not before. Correct the ordering, and restore the comment
> from the old propagateFloat64MulAddNaN() that warned about
> this corner case.
>
> This fixes a regression from 2.11 for Arm guests where we would
> incorrectly fail to set the Invalid flag for these cases.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>


> ---
>  fpu/softfloat.c | 52 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 30 insertions(+), 22 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 70e0c40a1c..8401b37bd4 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -602,34 +602,42 @@ static FloatParts pick_nan(FloatParts a, FloatParts b, float_status *s)
>  static FloatParts pick_nan_muladd(FloatParts a, FloatParts b, FloatParts c,
>                                    bool inf_zero, float_status *s)
>  {
> +    int which;
> +
>      if (is_snan(a.cls) || is_snan(b.cls) || is_snan(c.cls)) {
>          s->float_exception_flags |= float_flag_invalid;
>      }
>
> -    if (s->default_nan_mode) {
> -        a.cls = float_class_dnan;
> -    } else {
> -        switch (pickNaNMulAdd(is_qnan(a.cls), is_snan(a.cls),
> -                              is_qnan(b.cls), is_snan(b.cls),
> -                              is_qnan(c.cls), is_snan(c.cls),
> -                              inf_zero, s)) {
> -        case 0:
> -            break;
> -        case 1:
> -            a = b;
> -            break;
> -        case 2:
> -            a = c;
> -            break;
> -        case 3:
> -            a.cls = float_class_dnan;
> -            return a;
> -        default:
> -            g_assert_not_reached();
> -        }
> +    which = pickNaNMulAdd(is_qnan(a.cls), is_snan(a.cls),
> +                          is_qnan(b.cls), is_snan(b.cls),
> +                          is_qnan(c.cls), is_snan(c.cls),
> +                          inf_zero, s);
>
> -        a.cls = float_class_msnan;
> +    if (s->default_nan_mode) {
> +        /* Note that this check is after pickNaNMulAdd so that function
> +         * has an opportunity to set the Invalid flag.
> +         */
> +        a.cls = float_class_dnan;
> +        return a;
>      }
> +
> +    switch (which) {
> +    case 0:
> +        break;
> +    case 1:
> +        a = b;
> +        break;
> +    case 2:
> +        a = c;
> +        break;
> +    case 3:
> +        a.cls = float_class_dnan;
> +        return a;
> +    default:
> +        g_assert_not_reached();
> +    }
> +    a.cls = float_class_msnan;
> +
>      return a;
>  }


--
Alex Bennée