Re: [PATCH 12/18] i386: reimplement AVX comparison helpers

[Richard Henderson]

On 8/25/22 15:14, Paolo Bonzini wrote:
> From: Paul Brook <paul@nowt.org>
>
> AVX includes additional a more extensive set of comparison predicates,
> some of some of which our softfloat implementation does not expose directly.

Not quite right, that wording.  floatN_compare is the more general operation, directly 
exposed.

> +#define SSE_HELPER_CMP_P(name, F, C)                                    \
> +    void glue(helper_ ## name ## ps, SUFFIX)(CPUX86State *env,          \
> +                                             Reg *d, Reg *s)    \
>       {                                                                   \
> -        d->ZMM_L(0) = F(32, d->ZMM_S(0), s->ZMM_S(0));                  \
> -        d->ZMM_L(1) = F(32, d->ZMM_S(1), s->ZMM_S(1));                  \
> -        d->ZMM_L(2) = F(32, d->ZMM_S(2), s->ZMM_S(2));                  \
> -        d->ZMM_L(3) = F(32, d->ZMM_S(3), s->ZMM_S(3));                  \
> +        Reg *v = d;                                                     \
> +        int i;                                                          \
> +        for (i = 0; i < 2 << SHIFT; i++) {                              \
> +            d->ZMM_L(i) = F(32, C, v->ZMM_S(i), s->ZMM_S(i));           \
> +        }                                                               \

...

> +#define FPU_EQ(x) (x == float_relation_equal)
> +#define FPU_LT(x) (x == float_relation_less)
> +#define FPU_LE(x) (x <= float_relation_equal)
> +#define FPU_UNORD(x) (x == float_relation_unordered)
> +
> +#define FPU_CMPQ(size, COND, a, b) \
> +    (COND(float ## size ## _compare_quiet(a, b, &env->sse_status)) ? -1 : 0)
> +#define FPU_CMPS(size, COND, a, b) \
> +    (COND(float ## size ## _compare(a, b, &env->sse_status)) ? -1 : 0)

I think clearer with C unnested from F, i.e.

    d->ZMM_S(i) = C(F(32, v->ZMM_S(i), s->ZMM_S(i)));


Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~