Re: [Qemu-devel] [PATCH v1 18/19] fpu/softfloat: re-factor minmax

[Richard Henderson]

On 12/11/2017 04:57 AM, Alex Bennée wrote:
> +static decomposed_parts minmax_decomposed(decomposed_parts a,
> +                                          decomposed_parts b,
> +                                          bool ismin, bool ieee, bool ismag,
> +                                          float_status *s)
> +{
> +        if (a.cls >= float_class_qnan

Indent is off by 4.

> +                    if (s->default_nan_mode) {
> +                        a.cls = float_class_msnan;
> +                        return a;

float_class_dnan.  That said, can't you fall through to pick_nan_parts and have
that (and float_flag_invalid) handled already?

> +        if (a.cls == float_class_zero || b.cls == float_class_zero) {
> +            if (a.cls == float_class_normal) {
> +                if (a.sign) {
> +                    return ismin ? a : b;
> +                } else {
> +                    return ismin ? b : a;
> +                }
> +            } else if (b.cls == float_class_normal) {
> +                if (b.sign) {
> +                    return ismin ? b : a;
> +                } else {
> +                    return ismin ? a : b;
> +                }
> +            }
> +        }

With respect to zero, normal and inf should be handled the same.
Both of those middle tests should be

  a.cls == float_class_normal || a.cls == float_class_inf

It would appear this section is not honoring ismag.

This is one case where I think it might be helpful to do

    int a_exp, b_exp;
    bool a_sign, b_sign;

    if (nans) {
        ...
    }

    switch (a.cls) {
    case float_class_normal:
        a_exp = a.exp;
        break;
    case float_class_inf:
        a_exp = INT_MAX;
        break;
    case float_class_zero:
        a_exp = INT_MIN;
        break;
    }
    switch (b.cls) {
        ....
    }
    a_sign = a.sign;
    b_sign = b.sign
    if (ismag) {
        a_sign = b_sign = 0;
    }

    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;
    } else {
        return a_sign ^ ismin ? b : a;
    }


r~