David Hildenbrand <da...@redhat.com> writes:
> Implementation inspired by minmax_floats(). Unfortuantely, we don't have
> any tests we can simply adjust/unlock.
>
> Cc: Aurelien Jarno <aurel...@aurel32.net>
> Cc: Peter Maydell <peter.mayd...@linaro.org>
> Cc: "Alex Bennée" <alex.ben...@linaro.org>
> Signed-off-by: David Hildenbrand <da...@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.
--
Alex Bennée
