diff mbox series

[v1,01/20] softfloat: Implement float128_(min|minnum|minnummag|max|maxnum|maxnummag)

Message ID 20200930145523.71087-2-david@redhat.com
State New
Headers show
Series s390x/tcg: Implement Vector enhancements facility and switch to z14 | expand

Commit Message

David Hildenbrand Sept. 30, 2020, 2:55 p.m. UTC
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(+)

Comments

Alex Bennée Sept. 30, 2020, 4:10 p.m. UTC | #1
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?

> +        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?

> +    switch (pb.cls) {
> +    case float_class_normal:
> +        b_exp = pb.exp;
> +        break;
> +    case float_class_inf:
> +        b_exp = INT_MAX;
> +        break;
> +    case float_class_zero:
> +        b_exp = INT_MIN;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +        break;
> +    }
> +
> +    a_less = a_exp < b_exp;
> +    if (a_exp == b_exp) {
> +        a_less = pa.frac0 < pb.frac0;
> +        if (pa.frac0 == pb.frac0) {
> +            a_less = pa.frac1 < pb.frac1;
> +        }
> +    }
> +
> +    if (ismag &&
> +        (a_exp != b_exp || pa.frac0 != pb.frac0 || pa.frac1 != pb.frac1)) {
> +        return a_less ^ ismin ? b : a;
> +    } else if (pa.sign == pb.sign) {
> +        return pa.sign ^ a_less ^ ismin ? b : a;
> +    }
> +    return pa.sign ^ ismin ? b : a;
> +}

Otherwise it seems sane to me.
David Hildenbrand Oct. 1, 2020, 12:40 p.m. UTC | #2
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!
Alex Bennée Oct. 1, 2020, 1:15 p.m. UTC | #3
David Hildenbrand <david@redhat.com> writes:

> 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.
>> 
>> 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.

Give it a try - if it really does become harder to follow then we'll
stick with the duplication however if we can have common code you'll
know at least the nan handling and minmax behaviour for float128 will be
partially tested by the 16/32/64 float code.

>
> Thanks!
David Hildenbrand May 5, 2021, 2:54 p.m. UTC | #4
On 01.10.20 15:15, Alex Bennée wrote:
> 
> David Hildenbrand <david@redhat.com> writes:
> 
>> 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.
>>>
>>> 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.
> 
> Give it a try - if it really does become harder to follow then we'll
> stick with the duplication however if we can have common code you'll
> know at least the nan handling and minmax behaviour for float128 will be
> partially tested by the 16/32/64 float code.

(finally had time to look into this)

What about something like this:



 From dd6cf176c840fc34a095cb2a158032a994aca5ef Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Tue, 29 Sep 2020 14:36:26 +0200
Subject: [PATCH] softfloat: Implement
  float128_(min|minnum|minnummag|max|maxnum|maxnummag)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
Cc: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
  fpu/softfloat.c         | 141 ++++++++++++++++++++++++++++++++--------
  include/fpu/softfloat.h |   6 ++
  2 files changed, 120 insertions(+), 27 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index db7d3a39db..f1014f6d47 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -525,6 +525,18 @@ typedef struct {
      bool sign;
  } FloatParts128;
  
+static inline FloatParts128 floatparts64_to_128(FloatParts a)
+{
+    FloatParts128 res = {
+        .frac0 = a.frac,
+        .exp = a.exp,
+        .cls = a.cls,
+        .sign = a.sign,
+    };
+
+    return res;
+}
+
  #define DECOMPOSED_BINARY_POINT    (64 - 2)
  #define DECOMPOSED_IMPLICIT_BIT    (1ull << DECOMPOSED_BINARY_POINT)
  #define DECOMPOSED_OVERFLOW_BIT    (DECOMPOSED_IMPLICIT_BIT << 1)
@@ -621,6 +633,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)
  {
@@ -3093,6 +3107,14 @@ bfloat16 uint16_to_bfloat16(uint16_t a, float_status *status)
      return uint64_to_bfloat16_scalbn(a, 0, status);
  }
  
+typedef enum MinMaxRes {
+    MINMAX_RES_A,
+    MINMAX_RES_B,
+    MINMAX_RES_SILENCE_A,
+    MINMAX_RES_SILENCE_B,
+    MINMAX_RES_DEFAULT_NAN,
+} MinMaxRes;
+
  /* Float Min/Max */
  /* min() and max() functions. These can't be implemented as
   * 'compare and pick one input' because that would mishandle
@@ -3109,27 +3131,36 @@ bfloat16 uint16_to_bfloat16(uint16_t a, float_status *status)
   * minnummag() and maxnummag() functions correspond to minNumMag()
   * and minNumMag() from the IEEE-754 2008.
   */
-static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
-                                bool ieee, bool ismag, float_status *s)
+static MinMaxRes minmax_floats128(FloatParts128 a, FloatParts128 b, bool ismin,
+                                  bool ieee, bool ismag, float_status *s)
  {
      if (unlikely(is_nan(a.cls) || is_nan(b.cls))) {
-        if (ieee) {
-            /* Takes two floating-point values `a' and `b', one of
-             * which is a NaN, and returns the appropriate NaN
-             * result. If either `a' or `b' is a signaling NaN,
-             * the invalid exception is raised.
-             */
-            if (is_snan(a.cls) || is_snan(b.cls)) {
-                return pick_nan(a, b, s);
-            } else if (is_nan(a.cls) && !is_nan(b.cls)) {
-                return b;
+        if (ieee && !is_snan(a.cls) && !is_snan(b.cls)) {
+            if (is_nan(a.cls) && !is_nan(b.cls)) {
+                return MINMAX_RES_B;
              } else if (is_nan(b.cls) && !is_nan(a.cls)) {
-                return a;
+                return MINMAX_RES_A;
              }
          }
-        return pick_nan(a, b, s);
+
+        /* Similar logic to pick_nan(), avoiding re-packing. */
+        if (is_snan(a.cls) || is_snan(b.cls)) {
+            s->float_exception_flags |= float_flag_invalid;
+        }
+        if (s->default_nan_mode) {
+            return MINMAX_RES_DEFAULT_NAN;
+        }
+        if (pickNaN(a.cls, b.cls,
+                    a.frac0 > b.frac0 ||
+                    (a.frac0 == b.frac0 && a.frac1 > b.frac1) ||
+                    (a.frac0 == b.frac0 && a.frac1 == b.frac1 &&
+                     a.sign < b.sign), s)) {
+            return is_snan(b.cls) ? MINMAX_RES_SILENCE_B : MINMAX_RES_B;
+        }
+        return is_snan(a.cls) ? MINMAX_RES_SILENCE_A : MINMAX_RES_A;
      } else {
          int a_exp, b_exp;
+        bool a_less;
  
          switch (a.cls) {
          case float_class_normal:
@@ -3160,23 +3191,44 @@ static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
              break;
          }
  
-        if (ismag && (a_exp != b_exp || a.frac != b.frac)) {
-            bool a_less = a_exp < b_exp;
-            if (a_exp == b_exp) {
-                a_less = a.frac < b.frac;
+        a_less = a_exp < b_exp;
+        if (a_exp == b_exp) {
+            a_less = a.frac0 < b.frac0;
+            if (a.frac0 == b.frac0) {
+                a_less = a.frac1 < b.frac1;
              }
-            return a_less ^ ismin ? b : a;
          }
  
-        if (a.sign == b.sign) {
-            bool a_less = a_exp < b_exp;
-            if (a_exp == b_exp) {
-                a_less = a.frac < b.frac;
-            }
-            return a.sign ^ a_less ^ ismin ? b : a;
-        } else {
-            return a.sign ^ ismin ? b : a;
+        if (ismag &&
+            (a_exp != b_exp || a.frac0 != b.frac0 || a.frac1 != b.frac1)) {
+            return a_less ^ ismin ? MINMAX_RES_B : MINMAX_RES_A;
+        } else if (a.sign == b.sign) {
+            return a.sign ^ a_less ^ ismin ? MINMAX_RES_B : MINMAX_RES_A;
          }
+        return a.sign ^ ismin ? MINMAX_RES_B : MINMAX_RES_A;
+    }
+}
+
+static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
+                                bool ieee, bool ismag, float_status *s)
+{
+    FloatParts128 ta = floatparts64_to_128(a);
+    FloatParts128 tb = floatparts64_to_128(b);
+    MinMaxRes res = minmax_floats128(ta, tb, ismin, ieee, ismag, s);
+
+    switch(res) {
+    case MINMAX_RES_A:
+        return a;
+    case MINMAX_RES_B:
+        return b;
+    case MINMAX_RES_SILENCE_A:
+        return parts_silence_nan(a, s);
+    case MINMAX_RES_SILENCE_B:
+        return parts_silence_nan(b, s);
+    case MINMAX_RES_DEFAULT_NAN:
+        return parts_default_nan(s);
+    default:
+        g_assert_not_reached();
      }
  }
  
@@ -3233,6 +3285,41 @@ BF16_MINMAX(maxnummag, false, true, true)
  
  #undef BF16_MINMAX
  
+#define F128_MINMAX(name, ismin, isiee, ismag)                          \
+float128 float128_ ## name(float128 a, float128 b, float_status *s)     \
+{                                                                       \
+    FloatParts128 pa, pb;                                               \
+    MinMaxRes res;                                                      \
+                                                                        \
+    float128_unpack(&pa, a, s);                                         \
+    float128_unpack(&pb, b, s);                                         \
+    res = minmax_floats128(pa, pb, ismin, isiee, ismag, s);             \
+                                                                        \
+    switch(res) {                                                       \
+    case MINMAX_RES_A:                                                  \
+        return a;                                                       \
+    case MINMAX_RES_B:                                                  \
+        return b;                                                       \
+    case MINMAX_RES_SILENCE_A:                                          \
+        return float128_silence_nan(a, s);                              \
+    case MINMAX_RES_SILENCE_B:                                          \
+        return float128_silence_nan(b, s);                              \
+    case MINMAX_RES_DEFAULT_NAN:                                        \
+        return float128_default_nan(s);                                 \
+    default:                                                            \
+        g_assert_not_reached();                                         \
+    }                                                                   \
+}
+
+F128_MINMAX(min, true, false, false)
+F128_MINMAX(minnum, true, true, false)
+F128_MINMAX(minnummag, true, true, true)
+F128_MINMAX(max, false, false, false)
+F128_MINMAX(maxnum, false, true, false)
+F128_MINMAX(maxnummag, false, true, true)
+
+#undef F128_MINMAX
+
  /* Floating point compare */
  static FloatRelation compare_floats(FloatParts a, FloatParts b, bool is_quiet,
                                      float_status *s)
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index a38433deb4..4fab2ef6f4 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -1201,6 +1201,12 @@ float128 float128_muladd(float128, float128, float128, int,
  float128 float128_sqrt(float128, float_status *status);
  FloatRelation float128_compare(float128, float128, float_status *status);
  FloatRelation float128_compare_quiet(float128, float128, float_status *status);
+float128 float128_min(float128, float128, float_status *status);
+float128 float128_max(float128, float128, float_status *status);
+float128 float128_minnum(float128, float128, float_status *status);
+float128 float128_maxnum(float128, float128, float_status *status);
+float128 float128_minnummag(float128, float128, float_status *status);
+float128 float128_maxnummag(float128, float128, float_status *status);
  bool float128_is_quiet_nan(float128, float_status *status);
  bool float128_is_signaling_nan(float128, float_status *status);
  float128 float128_silence_nan(float128, float_status *status);
Alex Bennée May 10, 2021, 9:57 a.m. UTC | #5
David Hildenbrand <david@redhat.com> writes:

> On 01.10.20 15:15, Alex Bennée wrote:
>> David Hildenbrand <david@redhat.com> writes:
>> 
>>> 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.
>>>>
>>>> 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.
>> Give it a try - if it really does become harder to follow then we'll
>> stick with the duplication however if we can have common code you'll
>> know at least the nan handling and minmax behaviour for float128 will be
>> partially tested by the 16/32/64 float code.
>
> (finally had time to look into this)
>
> What about something like this:
>

I was just about to look this morning but I see Richard has posted his
mega series:

  From: Richard Henderson <richard.henderson@linaro.org>
  Subject: [PATCH 00/72] Convert floatx80 and float128 to FloatParts
  Date: Fri,  7 May 2021 18:46:50 -0700
  Message-Id: <20210508014802.892561-1-richard.henderson@linaro.org>

which I think also includes the
float128_(min|minnum|minnummag|max|maxnum|maxnummag) functions. I'm
going to have a look at that first if that's ok.

>
>
> From dd6cf176c840fc34a095cb2a158032a994aca5ef Mon Sep 17 00:00:00 2001
> From: David Hildenbrand <david@redhat.com>
> Date: Tue, 29 Sep 2020 14:36:26 +0200
> Subject: [PATCH] softfloat: Implement
>  float128_(min|minnum|minnummag|max|maxnum|maxnummag)
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> 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>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  fpu/softfloat.c         | 141 ++++++++++++++++++++++++++++++++--------
>  include/fpu/softfloat.h |   6 ++
>  2 files changed, 120 insertions(+), 27 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index db7d3a39db..f1014f6d47 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -525,6 +525,18 @@ typedef struct {
>      bool sign;
>  } FloatParts128;
>  +static inline FloatParts128 floatparts64_to_128(FloatParts a)
> +{
> +    FloatParts128 res = {
> +        .frac0 = a.frac,
> +        .exp = a.exp,
> +        .cls = a.cls,
> +        .sign = a.sign,
> +    };
> +
> +    return res;
> +}
> +
>  #define DECOMPOSED_BINARY_POINT    (64 - 2)
>  #define DECOMPOSED_IMPLICIT_BIT    (1ull << DECOMPOSED_BINARY_POINT)
>  #define DECOMPOSED_OVERFLOW_BIT    (DECOMPOSED_IMPLICIT_BIT << 1)
> @@ -621,6 +633,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)
>  {
> @@ -3093,6 +3107,14 @@ bfloat16 uint16_to_bfloat16(uint16_t a, float_status *status)
>      return uint64_to_bfloat16_scalbn(a, 0, status);
>  }
>  +typedef enum MinMaxRes {
> +    MINMAX_RES_A,
> +    MINMAX_RES_B,
> +    MINMAX_RES_SILENCE_A,
> +    MINMAX_RES_SILENCE_B,
> +    MINMAX_RES_DEFAULT_NAN,
> +} MinMaxRes;
> +
>  /* Float Min/Max */
>  /* min() and max() functions. These can't be implemented as
>   * 'compare and pick one input' because that would mishandle
> @@ -3109,27 +3131,36 @@ bfloat16 uint16_to_bfloat16(uint16_t a, float_status *status)
>   * minnummag() and maxnummag() functions correspond to minNumMag()
>   * and minNumMag() from the IEEE-754 2008.
>   */
> -static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
> -                                bool ieee, bool ismag, float_status *s)
> +static MinMaxRes minmax_floats128(FloatParts128 a, FloatParts128 b, bool ismin,
> +                                  bool ieee, bool ismag, float_status *s)
>  {
>      if (unlikely(is_nan(a.cls) || is_nan(b.cls))) {
> -        if (ieee) {
> -            /* Takes two floating-point values `a' and `b', one of
> -             * which is a NaN, and returns the appropriate NaN
> -             * result. If either `a' or `b' is a signaling NaN,
> -             * the invalid exception is raised.
> -             */
> -            if (is_snan(a.cls) || is_snan(b.cls)) {
> -                return pick_nan(a, b, s);
> -            } else if (is_nan(a.cls) && !is_nan(b.cls)) {
> -                return b;
> +        if (ieee && !is_snan(a.cls) && !is_snan(b.cls)) {
> +            if (is_nan(a.cls) && !is_nan(b.cls)) {
> +                return MINMAX_RES_B;
>              } else if (is_nan(b.cls) && !is_nan(a.cls)) {
> -                return a;
> +                return MINMAX_RES_A;
>              }
>          }
> -        return pick_nan(a, b, s);
> +
> +        /* Similar logic to pick_nan(), avoiding re-packing. */
> +        if (is_snan(a.cls) || is_snan(b.cls)) {
> +            s->float_exception_flags |= float_flag_invalid;
> +        }
> +        if (s->default_nan_mode) {
> +            return MINMAX_RES_DEFAULT_NAN;
> +        }
> +        if (pickNaN(a.cls, b.cls,
> +                    a.frac0 > b.frac0 ||
> +                    (a.frac0 == b.frac0 && a.frac1 > b.frac1) ||
> +                    (a.frac0 == b.frac0 && a.frac1 == b.frac1 &&
> +                     a.sign < b.sign), s)) {
> +            return is_snan(b.cls) ? MINMAX_RES_SILENCE_B : MINMAX_RES_B;
> +        }
> +        return is_snan(a.cls) ? MINMAX_RES_SILENCE_A : MINMAX_RES_A;
>      } else {
>          int a_exp, b_exp;
> +        bool a_less;
>            switch (a.cls) {
>          case float_class_normal:
> @@ -3160,23 +3191,44 @@ static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
>              break;
>          }
>  -        if (ismag && (a_exp != b_exp || a.frac != b.frac)) {
> -            bool a_less = a_exp < b_exp;
> -            if (a_exp == b_exp) {
> -                a_less = a.frac < b.frac;
> +        a_less = a_exp < b_exp;
> +        if (a_exp == b_exp) {
> +            a_less = a.frac0 < b.frac0;
> +            if (a.frac0 == b.frac0) {
> +                a_less = a.frac1 < b.frac1;
>              }
> -            return a_less ^ ismin ? b : a;
>          }
>  -        if (a.sign == b.sign) {
> -            bool a_less = a_exp < b_exp;
> -            if (a_exp == b_exp) {
> -                a_less = a.frac < b.frac;
> -            }
> -            return a.sign ^ a_less ^ ismin ? b : a;
> -        } else {
> -            return a.sign ^ ismin ? b : a;
> +        if (ismag &&
> +            (a_exp != b_exp || a.frac0 != b.frac0 || a.frac1 != b.frac1)) {
> +            return a_less ^ ismin ? MINMAX_RES_B : MINMAX_RES_A;
> +        } else if (a.sign == b.sign) {
> +            return a.sign ^ a_less ^ ismin ? MINMAX_RES_B : MINMAX_RES_A;
>          }
> +        return a.sign ^ ismin ? MINMAX_RES_B : MINMAX_RES_A;
> +    }
> +}
> +
> +static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
> +                                bool ieee, bool ismag, float_status *s)
> +{
> +    FloatParts128 ta = floatparts64_to_128(a);
> +    FloatParts128 tb = floatparts64_to_128(b);
> +    MinMaxRes res = minmax_floats128(ta, tb, ismin, ieee, ismag, s);
> +
> +    switch(res) {
> +    case MINMAX_RES_A:
> +        return a;
> +    case MINMAX_RES_B:
> +        return b;
> +    case MINMAX_RES_SILENCE_A:
> +        return parts_silence_nan(a, s);
> +    case MINMAX_RES_SILENCE_B:
> +        return parts_silence_nan(b, s);
> +    case MINMAX_RES_DEFAULT_NAN:
> +        return parts_default_nan(s);
> +    default:
> +        g_assert_not_reached();
>      }
>  }
>  @@ -3233,6 +3285,41 @@ BF16_MINMAX(maxnummag, false, true, true)
>    #undef BF16_MINMAX
>  +#define F128_MINMAX(name, ismin, isiee, ismag)
> \
> +float128 float128_ ## name(float128 a, float128 b, float_status *s)     \
> +{                                                                       \
> +    FloatParts128 pa, pb;                                               \
> +    MinMaxRes res;                                                      \
> +                                                                        \
> +    float128_unpack(&pa, a, s);                                         \
> +    float128_unpack(&pb, b, s);                                         \
> +    res = minmax_floats128(pa, pb, ismin, isiee, ismag, s);             \
> +                                                                        \
> +    switch(res) {                                                       \
> +    case MINMAX_RES_A:                                                  \
> +        return a;                                                       \
> +    case MINMAX_RES_B:                                                  \
> +        return b;                                                       \
> +    case MINMAX_RES_SILENCE_A:                                          \
> +        return float128_silence_nan(a, s);                              \
> +    case MINMAX_RES_SILENCE_B:                                          \
> +        return float128_silence_nan(b, s);                              \
> +    case MINMAX_RES_DEFAULT_NAN:                                        \
> +        return float128_default_nan(s);                                 \
> +    default:                                                            \
> +        g_assert_not_reached();                                         \
> +    }                                                                   \
> +}
> +
> +F128_MINMAX(min, true, false, false)
> +F128_MINMAX(minnum, true, true, false)
> +F128_MINMAX(minnummag, true, true, true)
> +F128_MINMAX(max, false, false, false)
> +F128_MINMAX(maxnum, false, true, false)
> +F128_MINMAX(maxnummag, false, true, true)
> +
> +#undef F128_MINMAX
> +
>  /* Floating point compare */
>  static FloatRelation compare_floats(FloatParts a, FloatParts b, bool is_quiet,
>                                      float_status *s)
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index a38433deb4..4fab2ef6f4 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -1201,6 +1201,12 @@ float128 float128_muladd(float128, float128, float128, int,
>  float128 float128_sqrt(float128, float_status *status);
>  FloatRelation float128_compare(float128, float128, float_status *status);
>  FloatRelation float128_compare_quiet(float128, float128, float_status *status);
> +float128 float128_min(float128, float128, float_status *status);
> +float128 float128_max(float128, float128, float_status *status);
> +float128 float128_minnum(float128, float128, float_status *status);
> +float128 float128_maxnum(float128, float128, float_status *status);
> +float128 float128_minnummag(float128, float128, float_status *status);
> +float128 float128_maxnummag(float128, float128, float_status *status);
>  bool float128_is_quiet_nan(float128, float_status *status);
>  bool float128_is_signaling_nan(float128, float_status *status);
>  float128 float128_silence_nan(float128, float_status *status);
> -- 
> 2.30.2
David Hildenbrand May 10, 2021, 10 a.m. UTC | #6
On 10.05.21 11:57, Alex Bennée wrote:
> 
> David Hildenbrand <david@redhat.com> writes:
> 
>> On 01.10.20 15:15, Alex Bennée wrote:
>>> David Hildenbrand <david@redhat.com> writes:
>>>
>>>> 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.
>>>>>
>>>>> 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.
>>> Give it a try - if it really does become harder to follow then we'll
>>> stick with the duplication however if we can have common code you'll
>>> know at least the nan handling and minmax behaviour for float128 will be
>>> partially tested by the 16/32/64 float code.
>>
>> (finally had time to look into this)
>>
>> What about something like this:
>>
> 
> I was just about to look this morning but I see Richard has posted his
> mega series:
> 
>    From: Richard Henderson <richard.henderson@linaro.org>
>    Subject: [PATCH 00/72] Convert floatx80 and float128 to FloatParts
>    Date: Fri,  7 May 2021 18:46:50 -0700
>    Message-Id: <20210508014802.892561-1-richard.henderson@linaro.org>
> 
> which I think also includes the
> float128_(min|minnum|minnummag|max|maxnum|maxnummag) functions. I'm
> going to have a look at that first if that's ok.

Sure, I have the gut feeling that it follows a similar approach :)
diff mbox series

Patch

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,
     }
 }
 
+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);
+
+    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);
+        }
+        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;
+    }
+    switch (pb.cls) {
+    case float_class_normal:
+        b_exp = pb.exp;
+        break;
+    case float_class_inf:
+        b_exp = INT_MAX;
+        break;
+    case float_class_zero:
+        b_exp = INT_MIN;
+        break;
+    default:
+        g_assert_not_reached();
+        break;
+    }
+
+    a_less = a_exp < b_exp;
+    if (a_exp == b_exp) {
+        a_less = pa.frac0 < pb.frac0;
+        if (pa.frac0 == pb.frac0) {
+            a_less = pa.frac1 < pb.frac1;
+        }
+    }
+
+    if (ismag &&
+        (a_exp != b_exp || pa.frac0 != pb.frac0 || pa.frac1 != pb.frac1)) {
+        return a_less ^ ismin ? b : a;
+    } else if (pa.sign == pb.sign) {
+        return pa.sign ^ a_less ^ ismin ? b : a;
+    }
+    return pa.sign ^ ismin ? b : a;
+}
+
 #define MINMAX(sz, name, ismin, isiee, ismag)                           \
 float ## sz float ## sz ## _ ## name(float ## sz a, float ## sz b,      \
                                      float_status *s)                   \
@@ -3214,6 +3299,21 @@  MINMAX(64, maxnummag, false, true, true)
 
 #undef MINMAX
 
+#define F128_MINMAX(name, ismin, isiee, ismag)                          \
+float128 float128_ ## name(float128 a, float128 b, float_status *s)     \
+{                                                                       \
+    return float128_minmax(a, b, ismin, isiee, ismag, s);               \
+}
+
+F128_MINMAX(min, true, false, false)
+F128_MINMAX(minnum, true, true, false)
+F128_MINMAX(minnummag, true, true, true)
+F128_MINMAX(max, false, false, false)
+F128_MINMAX(maxnum, false, true, false)
+F128_MINMAX(maxnummag, false, true, true)
+
+#undef F128_MINMAX
+
 #define BF16_MINMAX(name, ismin, isiee, ismag)                          \
 bfloat16 bfloat16_ ## name(bfloat16 a, bfloat16 b, float_status *s)     \
 {                                                                       \
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index a38433deb4..4fab2ef6f4 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -1201,6 +1201,12 @@  float128 float128_muladd(float128, float128, float128, int,
 float128 float128_sqrt(float128, float_status *status);
 FloatRelation float128_compare(float128, float128, float_status *status);
 FloatRelation float128_compare_quiet(float128, float128, float_status *status);
+float128 float128_min(float128, float128, float_status *status);
+float128 float128_max(float128, float128, float_status *status);
+float128 float128_minnum(float128, float128, float_status *status);
+float128 float128_maxnum(float128, float128, float_status *status);
+float128 float128_minnummag(float128, float128, float_status *status);
+float128 float128_maxnummag(float128, float128, float_status *status);
 bool float128_is_quiet_nan(float128, float_status *status);
 bool float128_is_signaling_nan(float128, float_status *status);
 float128 float128_silence_nan(float128, float_status *status);