[v1,01/15] s390x/tcg: Fix TEST DATA CLASS instructions

Let's detect normal and denormal ("subnormal") numbers reliably. Also
test for quiet NaN's.

While at it, use a better check to test for the mask bits in the data
class mask. The data class mask has 12 bits, whereby bit 0 is the
leftmost bit and bit 11 the rightmost bit. In the PoP an easy to read
table with the numbers is provided for the VECTOR FP TEST DATA CLASS
IMMEDIATE instruction, the table for TEST DATA CLASS is more confusing
as it is based on 64 bit values.

Factor the checks out into separate functions, as they will also be
needed for floating point vector instructions.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/fpu_helper.c | 96 ++++++++++++++++++++++++++-------------
 1 file changed, 65 insertions(+), 31 deletions(-)

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~

On 12.02.19 19:01, Richard Henderson wrote:
> 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~
> 

Indeed, makes sense! Also thought about macros, but I guess the huge
pile of code for vector instruction support I have lying around make me
lazy :)

Thanks!