Re: [Qemu-devel] [PATCH v1 03/14] softfloat: fix {min, max}nummag for same-abs-value inputs

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

Emilio G. Cota <cota@braap.org> writes:

> Before 8936006 ("fpu/softfloat: re-factor minmax", 2018-02-21),
> we used to return +Zero for maxnummag(-Zero,+Zero); after that
> commit, we return -Zero.
>
> Fix it by making {min,max}nummag consistent with {min,max}num,
> deferring to the latter when the absolute value of the operands
> is the same.
>
> With this fix we now pass fp-test.
>
> Signed-off-by: Emilio G. Cota <cota@braap.org>

Oops. I guess it didn't show up in any of the testing, which is likely
given how poorly RISU exercises the boundary conditions.

I'll grab this patch as I'm fixing up some other bugs in ARM handling.

> ---
>  fpu/softfloat.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index e124df9..ee615a9 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -1700,7 +1700,6 @@ static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
>          return pick_nan(a, b, s);
>      } else {
>          int a_exp, b_exp;
> -        bool a_sign, b_sign;
>
>          switch (a.cls) {
>          case float_class_normal:
> @@ -1731,20 +1730,22 @@ static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
>              break;
>          }
>
> -        a_sign = a.sign;
> -        b_sign = b.sign;
> -        if (ismag) {
> -            a_sign = b_sign = 0;
> +        if (ismag && (a_exp != b_exp || a.frac != b.frac)) {
> +            bool a_less = a_exp < b_exp;
> +            if (a_exp == b_exp) {
> +                a_less = a.frac < b.frac;
> +            }
> +            return a_less ^ ismin ? b : a;
>          }
>
> -        if (a_sign == b_sign) {
> +        if (a.sign == b.sign) {
>              bool a_less = a_exp < b_exp;
>              if (a_exp == b_exp) {
>                  a_less = a.frac < b.frac;
>              }
> -            return a_sign ^ a_less ^ ismin ? b : a;
> +            return a.sign ^ a_less ^ ismin ? b : a;
>          } else {
> -            return a_sign ^ ismin ? b : a;
> +            return a.sign ^ ismin ? b : a;
>          }
>      }
>  }


--
Alex Bennée