Re: [Qemu-devel] [PATCH v2 12/20] fpu/softfloat: re-factor mul

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 9 January 2018 at 12:22, Alex Bennée <alex.bennee@linaro.org> wrote:
>> We can now add float16_mul and use the common decompose and
>> canonicalize functions to have a single implementation for
>> float16/32/64 versions.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  fpu/softfloat.c         | 207 ++++++++++++++++++------------------------------
>>  include/fpu/softfloat.h |   1 +
>>  2 files changed, 80 insertions(+), 128 deletions(-)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index f89e47e3ef..6e9d4c172c 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -730,6 +730,85 @@ float64 float64_sub(float64 a, float64 b, float_status *status)
>>      return float64_round_pack_canonical(pr, status);
>>  }
>>
>> +/*
>> + * Returns the result of multiplying the floating-point values `a' and
>> + * `b'. The operation is performed according to the IEC/IEEE Standard
>> + * for Binary Floating-Point Arithmetic.
>> + */
>> +
>> +static decomposed_parts mul_decomposed(decomposed_parts a, decomposed_parts b,
>> +                                       float_status *s)
>> +{
>> +    bool sign = a.sign ^ b.sign;
>> +
>> +    if (a.cls == float_class_normal && b.cls == float_class_normal) {
>> +        uint64_t hi, lo;
>> +        int exp = a.exp + b.exp;
>> +
>> +        mul64To128(a.frac, b.frac, &hi, &lo);
>> +        shift128RightJamming(hi, lo, DECOMPOSED_BINARY_POINT, &hi, &lo);
>> +        if (lo & DECOMPOSED_OVERFLOW_BIT) {
>> +            shift64RightJamming(lo, 1, &lo);
>> +            exp += 1;
>> +        }
>> +
>> +        /* Re-use a */
>> +        a.exp = exp;
>> +        a.sign = sign;
>> +        a.frac = lo;
>> +        return a;
>> +    }
>> +    /* handle all the NaN cases */
>> +    if (a.cls >= float_class_qnan || b.cls >= float_class_qnan) {
>> +        return pick_nan_parts(a, b, s);
>> +    }
>> +    /* Inf * Zero == NaN */
>> +    if (((1 << a.cls) | (1 << b.cls)) ==
>> +        ((1 << float_class_inf) | (1 << float_class_zero))) {
>
> This is kinda confusing...

Yeah it's a bit of a shortcut to:

  if ((a.cls == float_class_inf && b.cls == float_class_zero)
     ||
      (a.cls == float_class_zero && b.cls == float_class_inf))

Would you prefer it long hand or tidied away to a helper?

  if (cls_combination(a, b, float_class_inf, float_class_zero))

?

>
>> +        s->float_exception_flags |= float_flag_invalid;
>> +        a.cls = float_class_dnan;
>> +        a.sign = sign;
>> +        return a;
>> +    }
>> +    /* Multiply by 0 or Inf */
>> +    if (a.cls == float_class_inf || a.cls == float_class_zero) {
>> +        a.sign = sign;
>> +        return a;
>> +    }
>> +    if (b.cls == float_class_inf || b.cls == float_class_zero) {
>> +        b.sign = sign;
>> +        return b;
>> +    }
>> +    g_assert_not_reached();
>> +}
>
> thanks
> -- PMM


--
Alex Bennée