Re: [Qemu-devel] [PATCH v7 4/6] softfloat: Add minNum() and maxNum() functions to softfloat.

[Peter Maydell]

On 2 December 2013 20:12, Will Newton <address@hidden> wrote:
> Add floatnn_minnum() and floatnn_maxnum() functions which are equivalent
> to the minNum() and maxNum() functions from IEEE 754-2008. They are
> similar to min() and max() but differ in the handling of QNaN arguments.

Missing Signed-off-by: line here.

> ---
>  fpu/softfloat.c         | 54 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/fpu/softfloat.h |  4 ++++
>  2 files changed, 58 insertions(+)
>
> Changes in v7:
>  - New patch
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 97bf627..9834927 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -6750,6 +6750,60 @@ float ## s float ## s ## _max(float ## s a, float ## s 
> b STATUS_PARAM)  \
>  MINMAX(32)
>  MINMAX(64)
>
> +/* 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.

I think this comment could usefully be expanded to add:
 * 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.



> + */
> +#define MINMAXNUM(s)                                                       \
> +INLINE float ## s float ## s ## _minmaxnum(float ## s a, float ## s b,     \
> +                                           int ismin STATUS_PARAM )        \
> +{                                                                          \
> +    flag aSign, bSign;                                                     \
> +    uint ## s ## _t av, bv;                                                \
> +    a = float ## s ## _squash_input_denormal(a STATUS_VAR);                \
> +    b = float ## s ## _squash_input_denormal(b STATUS_VAR);                \
> +    if (float ## s ## _is_quiet_nan(a) &&                                  \
> +        !float ## s ##_is_quiet_nan(b)) {                                  \
> +        return b;                                                          \
> +    } else if (float ## s ## _is_quiet_nan(b) &&                           \
> +               !float ## s ## _is_quiet_nan(a)) {                          \
> +        return a;                                                          \

This is incorrect if the inputs are one signalling NaN and one
quiet NaN (both according to the IEEE spec and the ARM ARM
pseudocode). Among other things we will fail to signal InvalidOp.

Also, the bulk of this macro is identical to MINMAX. That suggests
we could instead extend MINMAX to define functions taking an
extra parameter isminmaxnum indicating we want 754-2008
semantics, and then we just need to replace the current NaN-check
clause in it with this one:

    if (float ## s ## _is_any_nan(a) ||
        float ## s ## _is_any_nan(b)) {
        if (isminmax) {
            if ((float ## s ## _is_quiet_nan(a) && !float ## s ##
_is_any_nan(b)) {
                return b;
            }
             if ((float ## s ## _is_quiet_nan(b) && !float ## s ##
_is_any_nan(a)) {
                return a;
            }
       }
       return propagateFloat ## s ## NaN(a, b STATUS_VAR);
  }

(line continuation backslashes omitted for clarity)

thanks
-- PMM