Re: [Qemu-devel] [PATCH v2 15/22] softfloat: add functions corresponding to IEEE-2008 min/maxNumMag

[Aurelien Jarno]

On Wed, Jun 11, 2014 at 04:19:45PM +0100, Leon Alrae wrote:
> Add abs argument to the existing softfloat minmax() function and define
> new float{32,64}_{min,max}nummag functions.
> 
> minnummag(x,y) returns x if |x| < |y|,
>                returns y if |y| < |x|,
>                otherwise minnum(x,y)
> 
> maxnummag(x,y) returns x if |x| > |y|,
>                returns y if |y| > |x|,
>                otherwise maxnum(x,y)
> 
> Signed-off-by: Leon Alrae <address@hidden>
> ---
>  fpu/softfloat.c         |   37 +++++++++++++++++++++++++++++++------
>  include/fpu/softfloat.h |    4 ++++
>  2 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index e00a6fb..7ba2de4 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -7240,13 +7240,17 @@ int float128_compare_quiet( float128 a, float128 b 
> STATUS_PARAM )
>   * 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.
>   */
>  #define MINMAX(s)                                                       \
>  INLINE float ## s float ## s ## _minmax(float ## s a, float ## s b,     \
> -                                        int ismin, int isieee STATUS_PARAM) \
> +                                        int ismin, int isieee,          \
> +                                        int abs STATUS_PARAM)           \

Maybe ismag instead of abs, to follow the convention of other parameters?

>  {                                                                       \
>      flag aSign, bSign;                                                  \
> -    uint ## s ## _t av, bv;                                             \
> +    uint ## s ## _t av, bv, aav, abv;                                   \
>      a = float ## s ## _squash_input_denormal(a STATUS_VAR);             \
>      b = float ## s ## _squash_input_denormal(b STATUS_VAR);             \
>      if (float ## s ## _is_any_nan(a) ||                                 \
> @@ -7266,6 +7270,17 @@ INLINE float ## s float ## s ## _minmax(float ## s a, 
> float ## s b,     \
>      bSign = extractFloat ## s ## Sign(b);                               \
>      av = float ## s ## _val(a);                                         \
>      bv = float ## s ## _val(b);                                         \
> +    if (abs) {                                                          \
> +        aav = float ## s ## _abs(av);                                   \
> +        abv = float ## s ## _abs(bv);                                   \
> +        if (aav != abv) {                                               \
> +            if (ismin) {                                                \
> +                return (aav < abv) ? a : b;                             \
> +            } else {                                                    \
> +                return (aav < abv) ? b : a;                             \
> +            }                                                           \
> +        }                                                               \
> +    }                                                                   \
>      if (aSign != bSign) {                                               \
>          if (ismin) {                                                    \
>              return aSign ? a : b;                                       \
> @@ -7283,22 +7298,32 @@ INLINE float ## s float ## s ## _minmax(float ## s a, 
> float ## s b,     \
>                                                                          \
>  float ## s float ## s ## _min(float ## s a, float ## s b STATUS_PARAM)  \
>  {                                                                       \
> -    return float ## s ## _minmax(a, b, 1, 0 STATUS_VAR);                \
> +    return float ## s ## _minmax(a, b, 1, 0, 0 STATUS_VAR);             \
>  }                                                                       \
>                                                                          \
>  float ## s float ## s ## _max(float ## s a, float ## s b STATUS_PARAM)  \
>  {                                                                       \
> -    return float ## s ## _minmax(a, b, 0, 0 STATUS_VAR);                \
> +    return float ## s ## _minmax(a, b, 0, 0, 0 STATUS_VAR);             \
>  }                                                                       \
>                                                                          \
>  float ## s float ## s ## _minnum(float ## s a, float ## s b STATUS_PARAM) \
>  {                                                                       \
> -    return float ## s ## _minmax(a, b, 1, 1 STATUS_VAR);                \
> +    return float ## s ## _minmax(a, b, 1, 1, 0 STATUS_VAR);             \
>  }                                                                       \
>                                                                          \
>  float ## s float ## s ## _maxnum(float ## s a, float ## s b STATUS_PARAM) \
>  {                                                                       \
> -    return float ## s ## _minmax(a, b, 0, 1 STATUS_VAR);                \
> +    return float ## s ## _minmax(a, b, 0, 1, 0 STATUS_VAR);             \
> +}                                                                       \
> +                                                                        \
> +float ## s float ## s ## _minnummag(float ## s a, float ## s b STATUS_PARAM) 
> \
> +{                                                                       \
> +    return float ## s ## _minmax(a, b, 1, 1, 1 STATUS_VAR);             \
> +}                                                                       \
> +                                                                        \
> +float ## s float ## s ## _maxnummag(float ## s a, float ## s b STATUS_PARAM) 
> \
> +{                                                                       \
> +    return float ## s ## _minmax(a, b, 0, 1, 1 STATUS_VAR);             \
>  }
>  
>  MINMAX(32)
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index 4b3090c..feb73b7 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -375,6 +375,8 @@ float32 float32_min(float32, float32 STATUS_PARAM);
>  float32 float32_max(float32, float32 STATUS_PARAM);
>  float32 float32_minnum(float32, float32 STATUS_PARAM);
>  float32 float32_maxnum(float32, float32 STATUS_PARAM);
> +float32 float32_minnummag(float32, float32 STATUS_PARAM);
> +float32 float32_maxnummag(float32, float32 STATUS_PARAM);
>  int float32_is_quiet_nan( float32 );
>  int float32_is_signaling_nan( float32 );
>  float32 float32_maybe_silence_nan( float32 );
> @@ -485,6 +487,8 @@ float64 float64_min(float64, float64 STATUS_PARAM);
>  float64 float64_max(float64, float64 STATUS_PARAM);
>  float64 float64_minnum(float64, float64 STATUS_PARAM);
>  float64 float64_maxnum(float64, float64 STATUS_PARAM);
> +float64 float64_minnummag(float64, float64 STATUS_PARAM);
> +float64 float64_maxnummag(float64, float64 STATUS_PARAM);
>  int float64_is_quiet_nan( float64 a );
>  int float64_is_signaling_nan( float64 );
>  float64 float64_maybe_silence_nan( float64 );

Besides the small nitpick above, this looks fine to me, so:

Reviewed-by: Aurelien Jarno <address@hidden>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net