Re: [Qemu-devel] [PATCH v1 01/15] s390x/tcg: Fix TEST DATA CLASS instructions

[Richard Henderson <richard.henderson@linaro.org>]

On 2/12/19 3:02 AM, David Hildenbrand wrote:
> +static bool s390_tdc32(CPUS390XState *env, float32 f1, uint16_t dc_mask)
> +{
> +        const bool neg = float32_is_neg(f1);
> +        const bool zero = float32_is_zero(f1);
> +        const bool normal = float32_is_normal(f1);
> +        const bool denormal = float32_is_denormal(f1);
> +        const bool infinity = float32_is_infinity(f1);
> +        const bool quiet_nan = float32_is_quiet_nan(f1, &env->fpu_status);
> +        const bool sig_nan = float32_is_signaling_nan(f1, &env->fpu_status);
> +
> +        return (zero && test_dc_mask(dc_mask, 0, neg)) ||
> +               (normal && test_dc_mask(dc_mask, 2, neg)) ||
> +               (denormal && test_dc_mask(dc_mask, 4, neg)) ||
> +               (infinity && test_dc_mask(dc_mask, 6, neg)) ||
> +               (quiet_nan && test_dc_mask(dc_mask, 8, neg)) ||
> +               (sig_nan && test_dc_mask(dc_mask, 10, neg));
> +}
> +

This is doing more work than necessary, since any one fp value can only be one
of these.

I think it would be better to structure this like the riscv helper_fclass_*:

static inline uint32_t dcmask(int bit, bool neg)
{
    return 1 << (11 - bit - neg);
}

static uint32_t float32_dcmask(CPUS390XState *env, float32 f1)
{
    bool neg = float32_is_neg(f1);

    /* Sorted by most common cases.  */
    if (float32_is_normal(f1)) {
        return dc_mask(2, neg);
    } else if (float32_is_zero(f1)) {
        return dc_mask(0, neg);
    } else if (float32_is_zero_or_denormal(f1)) {
        /* denormal, since zero is eliminated */
        return dc_mask(4, neg);
    } else if (float32_is_infinity(f1)) {
        return dc_mask(6, neg);
    } else if (float64_is_quiet_nan(f1, &env->fpu_status)) {
        return dc_mask(8, neg);
    } else {
        /* signaling nan, as last remaining case */
        return dc_mask(10, neg);
    }
}

uint32_t HELPER(tceb)(CPUS390XState *env, uint64_t f1, uint64_t m2)
{
    return (m2 & float32_dcmask(env, f1)) != 0;
}

You may or may not wish to macro-ise float32_dcmask for the float type.


r~