All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Cc: alex.bennee@linaro.org
Subject: Re: [PATCH v2 04/28] softfloat: Move minmax_flags to softfloat-parts.c.inc
Date: Wed, 26 May 2021 15:45:47 +0200	[thread overview]
Message-ID: <34c1da32-3c06-47a7-a9d8-d9668a63e70d@redhat.com> (raw)
In-Reply-To: <20210525150706.294968-5-richard.henderson@linaro.org>

On 25.05.21 17:06, Richard Henderson wrote:
> Rename to parts$N_minmax.  Combine 3 bool arguments to a bitmask,
> return a tri-state value to indicate nan vs unchanged operand.
> Introduce ftype_minmax functions as a common optimization point.
> Fold bfloat16 expansions into the same macro as the other types.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   fpu/softfloat.c           | 216 ++++++++++++++++----------------------
>   fpu/softfloat-parts.c.inc |  81 ++++++++++++++
>   2 files changed, 170 insertions(+), 127 deletions(-)
> 
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index db14bd09aa..2dadded0b5 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -482,6 +482,15 @@ enum {
>       float_cmask_anynan  = float_cmask_qnan | float_cmask_snan,
>   };
>   
> +/* Flags for parts_minmax. */
> +enum {
> +    /* Set for minimum; clear for maximum. */
> +    minmax_ismin = 1,
> +    /* Set for the IEEE 754-2008 minNum() and maxNum() operations. */
> +    minmax_isnum = 2,
> +    /* Set for the IEEE 754-2008 minNumMag() and minNumMag() operations. */
> +    minmax_ismag = 4,
> +};
>   
>   /* Simple helpers for checking if, or what kind of, NaN we have */
>   static inline __attribute__((unused)) bool is_nan(FloatClass c)
> @@ -865,6 +874,14 @@ static void parts128_uint_to_float(FloatParts128 *p, uint64_t a,
>   #define parts_uint_to_float(P, I, Z, S) \
>       PARTS_GENERIC_64_128(uint_to_float, P)(P, I, Z, S)
>   
> +static int parts64_minmax(FloatParts64 *a, FloatParts64 *b,
> +                          float_status *s, int flags, const FloatFmt *fmt);
> +static int parts128_minmax(FloatParts128 *a, FloatParts128 *b,
> +                           float_status *s, int flags, const FloatFmt *fmt);
> +
> +#define parts_minmax(A, B, S, Z, F) \
> +    PARTS_GENERIC_64_128(minmax, A)(A, B, S, Z, F)
> +
>   /*
>    * Helper functions for softfloat-parts.c.inc, per-size operations.
>    */
> @@ -3258,145 +3275,90 @@ float128 uint64_to_float128(uint64_t a, float_status *status)
>       return float128_round_pack_canonical(&p, status);
>   }
>   
> -/* Float Min/Max */
> -/* min() and max() functions. These can't be implemented as
> - * 'compare and pick one input' because that would mishandle
> - * NaNs and +0 vs -0.
> - *
> - * minnum() and maxnum() functions. These are similar to the min()
> - * and max() functions but if one of the arguments is a QNaN and
> - * the other is numerical then the numerical argument is returned.
> - * SNaNs will get quietened before being returned.
> - * minnum() and maxnum correspond to the IEEE 754-2008 minNum()
> - * and maxNum() operations. min() and max() are the typical min/max
> - * semantics provided by many CPUs which predate that specification.
> - *
> - * minnummag() and maxnummag() functions correspond to minNumMag()
> - * and minNumMag() from the IEEE-754 2008.
> +/*
> + * Minimum and maximum
>    */
> -static FloatParts64 minmax_floats(FloatParts64 a, FloatParts64 b, bool ismin,
> -                                bool ieee, bool ismag, float_status *s)
> +
> +static float16 float16_minmax(float16 a, float16 b, float_status *s, int flags)
>   {
> -    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 *parts_pick_nan(&a, &b, s);
> -            } else if (is_nan(a.cls) && !is_nan(b.cls)) {
> -                return b;
> -            } else if (is_nan(b.cls) && !is_nan(a.cls)) {
> -                return a;
> -            }
> -        }
> -        return *parts_pick_nan(&a, &b, s);
> -    } else {
> -        int a_exp, b_exp;
> +    FloatParts64 pa, pb;
> +    int which;
>   
> -        switch (a.cls) {
> -        case float_class_normal:
> -            a_exp = a.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 (b.cls) {
> -        case float_class_normal:
> -            b_exp = b.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;
> -        }
> -
> -        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;
> -            }
> -            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;
> -        }
> +    float16_unpack_canonical(&pa, a, s);
> +    float16_unpack_canonical(&pb, b, s);
> +    which = parts_minmax(&pa, &pb, s, flags, &float16_params);
> +    if (unlikely(which < 0)) {
> +        /* Some sort of nan, need to repack default and silenced nans. */
> +        return float16_round_pack_canonical(&pa, s);
>       }
> +    return which ? b : a;
>   }
>   
> -#define MINMAX(sz, name, ismin, isiee, ismag)                           \
> -float ## sz float ## sz ## _ ## name(float ## sz a, float ## sz b,      \
> -                                     float_status *s)                   \
> -{                                                                       \
> -    FloatParts64 pa, pb, pr;                                            \
> -    float ## sz ## _unpack_canonical(&pa, a, s);                        \
> -    float ## sz ## _unpack_canonical(&pb, b, s);                        \
> -    pr = minmax_floats(pa, pb, ismin, isiee, ismag, s);                 \
> -    return float ## sz ## _round_pack_canonical(&pr, s);                \
> +static bfloat16 bfloat16_minmax(bfloat16 a, bfloat16 b,
> +                                float_status *s, int flags)
> +{
> +    FloatParts64 pa, pb;
> +    int which;
> +
> +    bfloat16_unpack_canonical(&pa, a, s);
> +    bfloat16_unpack_canonical(&pb, b, s);
> +    which = parts_minmax(&pa, &pb, s, flags, &float16_params);
> +    if (unlikely(which < 0)) {
> +        /* Some sort of nan, need to repack default and silenced nans. */
> +        return bfloat16_round_pack_canonical(&pa, s);
> +    }
> +    return which ? b : a;
>   }
>   
> -MINMAX(16, min, true, false, false)
> -MINMAX(16, minnum, true, true, false)
> -MINMAX(16, minnummag, true, true, true)
> -MINMAX(16, max, false, false, false)
> -MINMAX(16, maxnum, false, true, false)
> -MINMAX(16, maxnummag, false, true, true)
> +static float32 float32_minmax(float32 a, float32 b, float_status *s, int flags)
> +{
> +    FloatParts64 pa, pb;
> +    int which;
>   
> -MINMAX(32, min, true, false, false)
> -MINMAX(32, minnum, true, true, false)
> -MINMAX(32, minnummag, true, true, true)
> -MINMAX(32, max, false, false, false)
> -MINMAX(32, maxnum, false, true, false)
> -MINMAX(32, maxnummag, false, true, true)
> -
> -MINMAX(64, min, true, false, false)
> -MINMAX(64, minnum, true, true, false)
> -MINMAX(64, minnummag, true, true, true)
> -MINMAX(64, max, false, false, false)
> -MINMAX(64, maxnum, false, true, false)
> -MINMAX(64, maxnummag, false, true, true)
> -
> -#undef MINMAX
> -
> -#define BF16_MINMAX(name, ismin, isiee, ismag)                          \
> -bfloat16 bfloat16_ ## name(bfloat16 a, bfloat16 b, float_status *s)     \
> -{                                                                       \
> -    FloatParts64 pa, pb, pr;                                            \
> -    bfloat16_unpack_canonical(&pa, a, s);                               \
> -    bfloat16_unpack_canonical(&pb, b, s);                               \
> -    pr = minmax_floats(pa, pb, ismin, isiee, ismag, s);                 \
> -    return bfloat16_round_pack_canonical(&pr, s);                       \
> +    float32_unpack_canonical(&pa, a, s);
> +    float32_unpack_canonical(&pb, b, s);
> +    which = parts_minmax(&pa, &pb, s, flags, &float32_params);
> +    if (unlikely(which < 0)) {
> +        /* Some sort of nan, need to repack default and silenced nans. */
> +        return float32_round_pack_canonical(&pa, s);
> +    }
> +    return which ? b : a;
>   }
>   
> -BF16_MINMAX(min, true, false, false)
> -BF16_MINMAX(minnum, true, true, false)
> -BF16_MINMAX(minnummag, true, true, true)
> -BF16_MINMAX(max, false, false, false)
> -BF16_MINMAX(maxnum, false, true, false)
> -BF16_MINMAX(maxnummag, false, true, true)
> +static float64 float64_minmax(float64 a, float64 b, float_status *s, int flags)
> +{
> +    FloatParts64 pa, pb;
> +    int which;
>   
> -#undef BF16_MINMAX
> +    float64_unpack_canonical(&pa, a, s);
> +    float64_unpack_canonical(&pb, b, s);
> +    which = parts_minmax(&pa, &pb, s, flags, &float64_params);
> +    if (unlikely(which < 0)) {
> +        /* Some sort of nan, need to repack default and silenced nans. */
> +        return float64_round_pack_canonical(&pa, s);
> +    }
> +    return which ? b : a;
> +}
> +
> +#define MINMAX_1(type, name, flags) \
> +    type type##_##name(type a, type b, float_status *s) \
> +    { return type##_minmax(a, b, s, flags); }
> +
> +#define MINMAX_2(type) \
> +    MINMAX_1(type, max, 0)                                      \
> +    MINMAX_1(type, maxnum, minmax_isnum)                        \
> +    MINMAX_1(type, maxnummag, minmax_isnum | minmax_ismag)      \
> +    MINMAX_1(type, min, minmax_ismin)                           \
> +    MINMAX_1(type, minnum, minmax_ismin | minmax_isnum)         \
> +    MINMAX_1(type, minnummag, minmax_ismin | minmax_isnum | minmax_ismag)
> +
> +MINMAX_2(float16)
> +MINMAX_2(bfloat16)
> +MINMAX_2(float32)
> +MINMAX_2(float64)
> +
> +#undef MINMAX_1
> +#undef MINMAX_2
>   
>   /* Floating point compare */
>   static FloatRelation compare_floats(FloatParts64 a, FloatParts64 b, bool is_quiet,
> diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
> index f3c4f8c8d2..d68ab8fee0 100644
> --- a/fpu/softfloat-parts.c.inc
> +++ b/fpu/softfloat-parts.c.inc
> @@ -936,3 +936,84 @@ static void partsN(uint_to_float)(FloatPartsN *p, uint64_t a,
>           p->frac_hi = a << shift;
>       }
>   }
> +
> +/*
> + * Float min/max.
> + *
> + * Return -1 to return the chosen nan in *a;
> + * return 0 to use the a input unchanged; 1 to use the b input unchanged.
> + */
> +static int partsN(minmax)(FloatPartsN *a, FloatPartsN *b,
> +                          float_status *s, int flags, const FloatFmt *fmt)
> +{
> +    int ab_mask = float_cmask(a->cls) | float_cmask(b->cls);
> +    int a_exp, b_exp, cmp;
> +
> +    if (unlikely(ab_mask & float_cmask_anynan)) {
> +        /*
> +         * For minnum/maxnum, if one operand is a QNaN, and the other
> +         * operand is numerical, then return numerical argument.
> +         */
> +        if ((flags & minmax_isnum)
> +            && !(ab_mask & float_cmask_snan)
> +            && (ab_mask & ~float_cmask_qnan)) {
> +            return is_nan(a->cls);
> +        }
> +        *a = *parts_pick_nan(a, b, s);
> +        return -1;
> +    }
> +
> +    a_exp = a->exp;
> +    b_exp = b->exp;
> +
> +    if (unlikely(ab_mask != float_cmask_normal)) {
> +        switch (a->cls) {
> +        case float_class_normal:
> +            break;
> +        case float_class_inf:
> +            a_exp = INT16_MAX;
> +            break;
> +        case float_class_zero:
> +            a_exp = INT16_MIN;
> +            break;
> +        default:
> +            g_assert_not_reached();
> +            break;
> +        }
> +        switch (b->cls) {
> +        case float_class_normal:
> +            break;
> +        case float_class_inf:
> +            b_exp = INT16_MAX;
> +            break;
> +        case float_class_zero:
> +            b_exp = INT16_MIN;
> +            break;
> +        default:
> +            g_assert_not_reached();
> +            break;
> +        }
> +    }
> +
> +    /* Compare magnitudes. */
> +    cmp = a_exp - b_exp;
> +    if (cmp == 0) {
> +        cmp = frac_cmp(a, b);
> +    }
> +
> +    /*
> +     * Take the sign into account.
> +     * For ismag, only do this if the magnitudes are equal.
> +     */
> +    if (!(flags & minmax_ismag) || cmp == 0) {
> +        if (a->sign != b->sign) {
> +            /* For differing signs, the negative operand is less. */
> +            cmp = a->sign ? -1 : 1;
> +        } else if (a->sign) {
> +            /* For two negative operands, invert the magnitude comparison. */
> +            cmp = -cmp;
> +        }
> +    }
> +
> +    return (cmp < 0) ^ !!(flags & minmax_ismin);
> +}
> 

Looks good to me

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb



  reply	other threads:[~2021-05-26 13:47 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-25 15:06 [PATCH v2 00/28] Convert floatx80 and float128 to FloatParts Richard Henderson
2021-05-25 15:06 ` [PATCH v2 01/28] softfloat: Move round_to_uint_and_pack to softfloat-parts.c.inc Richard Henderson
2021-06-02 11:12   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 02/28] softfloat: Move int_to_float " Richard Henderson
2021-05-26 13:34   ` David Hildenbrand
2021-06-02 11:14   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 03/28] softfloat: Move uint_to_float " Richard Henderson
2021-05-26 13:36   ` David Hildenbrand
2021-06-02 11:31   ` Alex Bennée
2021-06-02 16:28     ` Richard Henderson
2021-05-25 15:06 ` [PATCH v2 04/28] softfloat: Move minmax_flags " Richard Henderson
2021-05-26 13:45   ` David Hildenbrand [this message]
2021-06-02 20:36   ` Alex Bennée
2021-06-02 22:23     ` Richard Henderson
2021-06-02 22:29   ` Richard Henderson
2021-05-25 15:06 ` [PATCH v2 05/28] softfloat: Move compare_floats " Richard Henderson
2021-06-03  9:00   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 06/28] softfloat: Move scalbn_decomposed " Richard Henderson
2021-06-03  9:01   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 07/28] softfloat: Move sqrt_float " Richard Henderson
2021-06-03  9:17   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 08/28] softfloat: Split out parts_uncanon_normal Richard Henderson
2021-06-03  9:22   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 09/28] softfloat: Reduce FloatFmt Richard Henderson
2021-06-03  9:23   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 10/28] softfloat: Introduce Floatx80RoundPrec Richard Henderson
2021-06-03  9:26   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 11/28] softfloat: Adjust parts_uncanon_normal for floatx80 Richard Henderson
2021-06-03 11:58   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 12/28] tests/fp/fp-test: Reverse order of floatx80 precision tests Richard Henderson
2021-05-25 15:06 ` [PATCH v2 13/28] softfloat: Convert floatx80_add/sub to FloatParts Richard Henderson
2021-06-03 14:23   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 14/28] softfloat: Convert floatx80_mul " Richard Henderson
2021-06-03 14:23   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 15/28] softfloat: Convert floatx80_div " Richard Henderson
2021-06-03 14:23   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 16/28] softfloat: Convert floatx80_sqrt " Richard Henderson
2021-06-03 14:24   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 17/28] softfloat: Convert floatx80_round " Richard Henderson
2021-06-03 14:25   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 18/28] softfloat: Convert floatx80_round_to_int " Richard Henderson
2021-06-03 14:26   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 19/28] softfloat: Convert integer to floatx80 " Richard Henderson
2021-06-03 14:26   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 20/28] softfloat: Convert floatx80 float conversions " Richard Henderson
2021-06-03 14:26   ` Alex Bennée
2021-05-25 15:06 ` [PATCH v2 21/28] softfloat: Convert floatx80 to integer " Richard Henderson
2021-06-03 14:27   ` Alex Bennée
2021-05-25 15:07 ` [PATCH v2 22/28] softfloat: Convert floatx80_scalbn " Richard Henderson
2021-06-03 14:34   ` Alex Bennée
2021-05-25 15:07 ` [PATCH v2 23/28] softfloat: Convert floatx80 compare " Richard Henderson
2021-06-03 14:34   ` Alex Bennée
2021-05-25 15:07 ` [PATCH v2 24/28] softfloat: Convert float32_exp2 " Richard Henderson
2021-06-03 14:44   ` Alex Bennée
2021-05-25 15:07 ` [PATCH v2 25/28] softfloat: Move floatN_log2 to softfloat-parts.c.inc Richard Henderson
2021-06-02 15:28   ` Alex Bennée
2021-05-25 15:07 ` [PATCH v2 26/28] softfloat: Convert modrem operations to FloatParts Richard Henderson
2021-06-03 14:48   ` Alex Bennée
2021-05-25 15:07 ` [PATCH v2 27/28] tests/fp: Enable more tests Richard Henderson
2021-05-25 15:07 ` [PATCH v2 28/28] softfloat: Use hard-float for {u}int64_to_float{32, 64} Richard Henderson
2021-06-03 14:57   ` [PATCH v2 28/28] softfloat: Use hard-float for {u}int64_to_float{32,64} Alex Bennée
2021-05-25 16:51 ` [PATCH v2 00/28] Convert floatx80 and float128 to FloatParts no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=34c1da32-3c06-47a7-a9d8-d9668a63e70d@redhat.com \
    --to=david@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --subject='Re: [PATCH v2 04/28] softfloat: Move minmax_flags to softfloat-parts.c.inc' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.