Re: [Qemu-devel] [PATCH v1 06/23] s390x/tcg: Implement VECTOR FP COMPARE (EQUAL|HIGH|HIGH OR EQUAL)

[Richard Henderson]

On 5/31/19 5:44 AM, David Hildenbrand wrote:
> +static int vfc64(S390Vector *v1, const S390Vector *v2, const S390Vector *v3,
> +                 CPUS390XState *env, bool s, bool test_equal, bool test_high,
> +                 uintptr_t retaddr)
> +{
> +    uint8_t vxc, vec_exc = 0;
> +    S390Vector tmp = {};
> +    int match = 0;
> +    int i;
> +
> +    for (i = 0; i < 2; i++) {
> +        const float64 a = make_float64(s390_vec_read_element64(v2, i));
> +        const float64 b = make_float64(s390_vec_read_element64(v3, i));
> +        const int cmp = float64_compare_quiet(a, b, &env->fpu_status);
> +
> +        if ((cmp == float_relation_equal && test_equal) ||
> +            (cmp == float_relation_greater && test_high)) {

It might be easier to pass in the comparison function instead of test_equal and
test_high (float64_eq_quiet, float64_lt_quiet) and swap the arguments to turn
lt into gt (not affecting eq).

This will let you pass float64_eq and float64_lt when it comes time to support
the SQ bit for the vector-enhancment-1 facility.

Otherwise you'll have 3 bools passed in and a bit of a mess here.


> +    if (match == i + 1) {
> +        return 0;

This doesn't look right.  How can match == 3,
with i == 2 when not exiting the loop early.

The vxc case is handled via longjmp, I think,
which leaves the S case to handle here.

Perhaps better as

        if (match) {
            return s || match == 2 ? 0 : 1;
        }
        return 3;


r~