[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 01/20] softfloat: Implement float128_(min|minnum|minnummag
From: |
David Hildenbrand |
Subject: |
Re: [PATCH v1 01/20] softfloat: Implement float128_(min|minnum|minnummag|max|maxnum|maxnummag) |
Date: |
Thu, 1 Oct 2020 14:40:39 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
On 30.09.20 18:10, Alex Bennée wrote:
>
> David Hildenbrand <david@redhat.com> writes:
>
>> Implementation inspired by minmax_floats(). Unfortuantely, we don't have
>> any tests we can simply adjust/unlock.
>>
>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: "Alex Bennée" <alex.bennee@linaro.org>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> fpu/softfloat.c | 100 ++++++++++++++++++++++++++++++++++++++++
>> include/fpu/softfloat.h | 6 +++
>> 2 files changed, 106 insertions(+)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index 9af75b9146..9463c5ea56 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -621,6 +621,8 @@ static inline FloatParts float64_unpack_raw(float64 f)
>> return unpack_raw(float64_params, f);
>> }
>>
>> +static void float128_unpack(FloatParts128 *p, float128 a, float_status
>> *status);
>> +
>> /* Pack a float from parts, but do not canonicalize. */
>> static inline uint64_t pack_raw(FloatFmt fmt, FloatParts p)
>> {
>> @@ -3180,6 +3182,89 @@ static FloatParts minmax_floats(FloatParts a,
>> FloatParts b, bool ismin,
>> }
>> }
>
> It would be desirable to share as much logic for this as possible with
> the existing minmax_floats code. I appreciate at some point we end up
> having to deal with fractions and we haven't found a good way to
> efficiently handle dealing with FloatParts and FloatParts128 in the same
> unrolled code, however:
>
>>
>> +static float128 float128_minmax(float128 a, float128 b, bool ismin, bool
>> ieee,
>> + bool ismag, float_status *s)
>> +{
>> + FloatParts128 pa, pb;
>> + int a_exp, b_exp;
>> + bool a_less;
>> +
>> + float128_unpack(&pa, a, s);
>> + float128_unpack(&pb, b, s);
>> +
>
> From here:
>> + if (unlikely(is_nan(pa.cls) || is_nan(pb.cls))) {
>> + /* See comment in minmax_floats() */
>> + if (ieee && !is_snan(pa.cls) && !is_snan(pb.cls)) {
>> + if (is_nan(pa.cls) && !is_nan(pb.cls)) {
>> + return b;
>> + } else if (is_nan(pb.cls) && !is_nan(pa.cls)) {
>> + return a;
>> + }
>> + }
>> +
>> + /* Similar logic to pick_nan(), avoiding re-packing. */
>> + if (is_snan(pa.cls) || is_snan(pb.cls)) {
>> + s->float_exception_flags |= float_flag_invalid;
>> + }
>> + if (s->default_nan_mode) {
>> + return float128_default_nan(s);
>> + }
>
> to here is common logic - is there anyway we could share it?
I can try to factor it out, similar to pickNaN() - passing weird boolean
flags and such. It most certainly won't win in a beauty contest, that's
for sure.
>
>> + if (pickNaN(pa.cls, pb.cls,
>> + pa.frac0 > pb.frac0 ||
>> + (pa.frac0 == pb.frac0 && pa.frac1 > pb.frac1) ||
>> + (pa.frac0 == pb.frac0 && pa.frac1 == pb.frac1 &&
>> + pa.sign < pb.sign), s)) {
>> + return is_snan(pb.cls) ? float128_silence_nan(b, s) : b;
>> + }
>> + return is_snan(pa.cls) ? float128_silence_nan(a, s) : a;
>> + }
>> +
>> + switch (pa.cls) {
>> + case float_class_normal:
>> + a_exp = pa.exp;
>> + break;
>> + case float_class_inf:
>> + a_exp = INT_MAX;
>> + break;
>> + case float_class_zero:
>> + a_exp = INT_MIN;
>> + break;
>> + default:
>> + g_assert_not_reached();
>> + break;
>> + }
>
> Likewise I wonder if there is scope for a float_minmax_exp helper that
> could be shared here?
I'll try, but I have the feeling that it might make the code harder to
read than actually help. Will give it a try.
Thanks!
--
Thanks,
David / dhildenb
- Re: [PATCH v1 01/20] softfloat: Implement float128_(min|minnum|minnummag|max|maxnum|maxnummag),
David Hildenbrand <=