[Qemu-devel] [PATCH v2 00/27] softfloat patch roundup

[Qemu-devel] [PATCH v2 00/27] softfloat patch roundup

[Richard Henderson]

This is my SNaN patch set, Alex's float-float refactor, and a couple
of other random outstanding fpu patches.  This has been reordered so
as to be bisectable, since the float-float refactor requires the snan
work to avoid breakage.

It probably doesn't matter, but this was built on top of my arm fp16
patch set simply to make it easier to test.  The whole tree is

  git://github.com/rth7680/qemu.git fpu-roundup


r~


Alex Bennée (5):
  fpu/softfloat: int_to_float ensure r fully initialised
  fpu/softfloat: re-factor float to float conversions
  fpu/softfloat: support ARM Alternative half-precision
  target/arm: convert conversion helpers to fpst/ahp_flag
  target/arm: squash FZ16 behaviour for conversions

Peter Maydell (2):
  fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)
  target/arm: Fix fp_status_f16 tininess before rounding

Richard Henderson (20):
  fpu/softfloat: Merge NO_SIGNALING_NANS definitions
  fpu/softfloat: Split floatXX_silence_nan from floatXX_maybe_silence_nan
  fpu/softfloat: Move softfloat-specialize.h below FloatParts definition
  fpu/softfloat: Canonicalize NaN fraction
  fpu/softfloat: Introduce parts_is_snan_frac
  fpu/softfloat: Replace float_class_dnan with parts_default_nan
  fpu/softfloat: Replace float_class_msnan with parts_silence_nan
  target/arm: Use floatX_silence_nan when we have already checked for SNaN
  target/arm: Remove floatX_maybe_silence_nan from conversions
  target/hppa: Remove floatX_maybe_silence_nan from conversions
  target/m68k: Use floatX_silence_nan when we have already checked for SNaN
  target/mips: Remove floatX_maybe_silence_nan from conversions
  target/riscv: Remove floatX_maybe_silence_nan from conversions
  target/s390x: Remove floatX_maybe_silence_nan from conversions
  fpu/softfloat: Use float*_silence_nan in propagateFloat*NaN
  fpu/softfloat: Remove floatX_maybe_silence_nan
  fpu/softfloat: Specialize on snan_bit_is_one
  fpu/softfloat: Make is_nan et al available to softfloat-specialize.h
  fpu/softfloat: Pass FloatClass to pickNaN
  fpu/softfloat: Pass FloatClass to pickNaNMulAdd

 fpu/softfloat-specialize.h    | 668 ++++++++++++++++++----------------
 include/fpu/softfloat-types.h |   1 +
 include/fpu/softfloat.h       |  22 +-
 target/arm/helper.h           |  10 +-
 target/arm/translate.h        |  12 +
 target/mips/cpu.h             |   4 +-
 fpu/softfloat.c               | 665 ++++++++++-----------------------
 target/arm/cpu.c              |   2 +
 target/arm/helper-a64.c       |   7 +-
 target/arm/helper.c           | 114 +++---
 target/arm/translate-a64.c    |  38 +-
 target/arm/translate.c        |  74 +++-
 target/hppa/cpu.c             |   1 -
 target/hppa/op_helper.c       |   2 -
 target/m68k/softfloat.c       |   3 +-
 target/mips/msa_helper.c      |   4 -
 target/mips/op_helper.c       |   2 -
 target/mips/translate_init.c  |   4 +-
 target/ppc/fpu_helper.c       |   1 -
 target/riscv/fpu_helper.c     |   6 +-
 target/s390x/fpu_helper.c     |  12 +-
 target/sh4/cpu.c              |   1 -
 target/unicore32/cpu.c        |   2 -
 23 files changed, 730 insertions(+), 925 deletions(-)

-- 
2.17.0

[Qemu-devel] [PATCH v2 01/27] fpu/softfloat: int_to_float ensure r fully initialised

[Richard Henderson]

From: Alex Bennée <alex.bennee@linaro.org>

Reported by Coverity (CID1390635). We ensure this for uint_to_float
later on so we might as well mirror that.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 8401b37bd4..b39c0c6fbb 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1525,7 +1525,7 @@ FLOAT_TO_UINT(64, 64)
 
 static FloatParts int_to_float(int64_t a, float_status *status)
 {
-    FloatParts r;
+    FloatParts r = {};
     if (a == 0) {
         r.cls = float_class_zero;
         r.sign = false;
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 01/27] fpu/softfloat: int_to_float ensure r fully initialised

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
>
> Reported by Coverity (CID1390635). We ensure this for uint_to_float
> later on so we might as well mirror that.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 8401b37bd4..b39c0c6fbb 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -1525,7 +1525,7 @@ FLOAT_TO_UINT(64, 64)
>
>  static FloatParts int_to_float(int64_t a, float_status *status)
>  {
> -    FloatParts r;
> +    FloatParts r = {};
>      if (a == 0) {
>          r.cls = float_class_zero;
>          r.sign = false;
> --
> 2.17.0
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 02/27] fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)

[Richard Henderson]

From: Peter Maydell <peter.maydell@linaro.org>

In float-to-integer conversion, if the floating point input
converts exactly to the largest or smallest integer that
fits in to the result type, this is not an overflow.
In this situation we were producing the correct result value,
but were incorrectly setting the Invalid flag.
For example for Arm A64, "FCVTAS w0, d0" on an input of
0x41dfffffffc00000 should produce 0x7fffffff and set no flags.

Fix the boundary case to take the right half of the if()
statements.

This fixes a regression from 2.11 introduced by the softfloat
refactoring.

Cc: qemu-stable@nongnu.org
Fixes: ab52f973a50
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 fpu/softfloat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index b39c0c6fbb..bc0f52fa54 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1368,14 +1368,14 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
             r = UINT64_MAX;
         }
         if (p.sign) {
-            if (r < -(uint64_t) min) {
+            if (r <= -(uint64_t) min) {
                 return -r;
             } else {
                 s->float_exception_flags = orig_flags | float_flag_invalid;
                 return min;
             }
         } else {
-            if (r < max) {
+            if (r <= max) {
                 return r;
             } else {
                 s->float_exception_flags = orig_flags | float_flag_invalid;
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 02/27] fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
>
> In float-to-integer conversion, if the floating point input
> converts exactly to the largest or smallest integer that
> fits in to the result type, this is not an overflow.
> In this situation we were producing the correct result value,
> but were incorrectly setting the Invalid flag.
> For example for Arm A64, "FCVTAS w0, d0" on an input of
> 0x41dfffffffc00000 should produce 0x7fffffff and set no flags.
>
> Fix the boundary case to take the right half of the if()
> statements.
>
> This fixes a regression from 2.11 introduced by the softfloat
> refactoring.
>
> Cc: qemu-stable@nongnu.org
> Fixes: ab52f973a50
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

This is missing your Signed-off-by: line (and a reviewed-by
would be nice too :-))

> ---
>  fpu/softfloat.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index b39c0c6fbb..bc0f52fa54 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -1368,14 +1368,14 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
>              r = UINT64_MAX;
>          }
>          if (p.sign) {
> -            if (r < -(uint64_t) min) {
> +            if (r <= -(uint64_t) min) {
>                  return -r;
>              } else {
>                  s->float_exception_flags = orig_flags | float_flag_invalid;
>                  return min;
>              }
>          } else {
> -            if (r < max) {
> +            if (r <= max) {
>                  return r;
>              } else {
>                  s->float_exception_flags = orig_flags | float_flag_invalid;
> --
> 2.17.0

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 02/27] fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)

[Richard Henderson]

On 05/14/2018 03:19 AM, Peter Maydell wrote:
> On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
>> From: Peter Maydell <peter.maydell@linaro.org>
>>
>> In float-to-integer conversion, if the floating point input
>> converts exactly to the largest or smallest integer that
>> fits in to the result type, this is not an overflow.
>> In this situation we were producing the correct result value,
>> but were incorrectly setting the Invalid flag.
>> For example for Arm A64, "FCVTAS w0, d0" on an input of
>> 0x41dfffffffc00000 should produce 0x7fffffff and set no flags.
>>
>> Fix the boundary case to take the right half of the if()
>> statements.
>>
>> This fixes a regression from 2.11 introduced by the softfloat
>> refactoring.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: ab52f973a50
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
> This is missing your Signed-off-by: line (and a reviewed-by
> would be nice too :-))

Dang it.  I thought I had done so for all of the patches.
Clearly missed one though.  For your target-arm.next branch:

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v2 03/27] fpu/softfloat: Merge NO_SIGNALING_NANS definitions

[Richard Henderson]

Move the ifdef inside the relevant functions instead of
duplicating the function declarations.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 100 +++++++++++++++----------------------
 1 file changed, 40 insertions(+), 60 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 27834af0de..58b05718c8 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -233,17 +233,6 @@ typedef struct {
     uint64_t high, low;
 } commonNaNT;
 
-#ifdef NO_SIGNALING_NANS
-int float16_is_quiet_nan(float16 a_, float_status *status)
-{
-    return float16_is_any_nan(a_);
-}
-
-int float16_is_signaling_nan(float16 a_, float_status *status)
-{
-    return 0;
-}
-#else
 /*----------------------------------------------------------------------------
 | Returns 1 if the half-precision floating-point value `a' is a quiet
 | NaN; otherwise returns 0.
@@ -251,12 +240,16 @@ int float16_is_signaling_nan(float16 a_, float_status *status)
 
 int float16_is_quiet_nan(float16 a_, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return float16_is_any_nan(a_);
+#else
     uint16_t a = float16_val(a_);
     if (status->snan_bit_is_one) {
         return (((a >> 9) & 0x3F) == 0x3E) && (a & 0x1FF);
     } else {
         return ((a & ~0x8000) >= 0x7C80);
     }
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -266,14 +259,17 @@ int float16_is_quiet_nan(float16 a_, float_status *status)
 
 int float16_is_signaling_nan(float16 a_, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return 0;
+#else
     uint16_t a = float16_val(a_);
     if (status->snan_bit_is_one) {
         return ((a & ~0x8000) >= 0x7C80);
     } else {
         return (((a >> 9) & 0x3F) == 0x3E) && (a & 0x1FF);
     }
-}
 #endif
+}
 
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the half-precision floating point value `a' is a
@@ -333,17 +329,6 @@ static float16 commonNaNToFloat16(commonNaNT a, float_status *status)
     }
 }
 
-#ifdef NO_SIGNALING_NANS
-int float32_is_quiet_nan(float32 a_, float_status *status)
-{
-    return float32_is_any_nan(a_);
-}
-
-int float32_is_signaling_nan(float32 a_, float_status *status)
-{
-    return 0;
-}
-#else
 /*----------------------------------------------------------------------------
 | Returns 1 if the single-precision floating-point value `a' is a quiet
 | NaN; otherwise returns 0.
@@ -351,12 +336,16 @@ int float32_is_signaling_nan(float32 a_, float_status *status)
 
 int float32_is_quiet_nan(float32 a_, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return float32_is_any_nan(a_);
+#else
     uint32_t a = float32_val(a_);
     if (status->snan_bit_is_one) {
         return (((a >> 22) & 0x1FF) == 0x1FE) && (a & 0x003FFFFF);
     } else {
         return ((uint32_t)(a << 1) >= 0xFF800000);
     }
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -366,14 +355,17 @@ int float32_is_quiet_nan(float32 a_, float_status *status)
 
 int float32_is_signaling_nan(float32 a_, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return 0;
+#else
     uint32_t a = float32_val(a_);
     if (status->snan_bit_is_one) {
         return ((uint32_t)(a << 1) >= 0xFF800000);
     } else {
         return (((a >> 22) & 0x1FF) == 0x1FE) && (a & 0x003FFFFF);
     }
-}
 #endif
+}
 
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the single-precision floating point value `a' is a
@@ -744,17 +736,6 @@ static float32 propagateFloat32NaN(float32 a, float32 b, float_status *status)
     }
 }
 
-#ifdef NO_SIGNALING_NANS
-int float64_is_quiet_nan(float64 a_, float_status *status)
-{
-    return float64_is_any_nan(a_);
-}
-
-int float64_is_signaling_nan(float64 a_, float_status *status)
-{
-    return 0;
-}
-#else
 /*----------------------------------------------------------------------------
 | Returns 1 if the double-precision floating-point value `a' is a quiet
 | NaN; otherwise returns 0.
@@ -762,6 +743,9 @@ int float64_is_signaling_nan(float64 a_, float_status *status)
 
 int float64_is_quiet_nan(float64 a_, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return float64_is_any_nan(a_);
+#else
     uint64_t a = float64_val(a_);
     if (status->snan_bit_is_one) {
         return (((a >> 51) & 0xFFF) == 0xFFE)
@@ -769,6 +753,7 @@ int float64_is_quiet_nan(float64 a_, float_status *status)
     } else {
         return ((a << 1) >= 0xFFF0000000000000ULL);
     }
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -778,6 +763,9 @@ int float64_is_quiet_nan(float64 a_, float_status *status)
 
 int float64_is_signaling_nan(float64 a_, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return 0;
+#else
     uint64_t a = float64_val(a_);
     if (status->snan_bit_is_one) {
         return ((a << 1) >= 0xFFF0000000000000ULL);
@@ -785,8 +773,8 @@ int float64_is_signaling_nan(float64 a_, float_status *status)
         return (((a >> 51) & 0xFFF) == 0xFFE)
             && (a & LIT64(0x0007FFFFFFFFFFFF));
     }
-}
 #endif
+}
 
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the double-precision floating point value `a' is a
@@ -899,17 +887,6 @@ static float64 propagateFloat64NaN(float64 a, float64 b, float_status *status)
     }
 }
 
-#ifdef NO_SIGNALING_NANS
-int floatx80_is_quiet_nan(floatx80 a_, float_status *status)
-{
-    return floatx80_is_any_nan(a_);
-}
-
-int floatx80_is_signaling_nan(floatx80 a_, float_status *status)
-{
-    return 0;
-}
-#else
 /*----------------------------------------------------------------------------
 | Returns 1 if the extended double-precision floating-point value `a' is a
 | quiet NaN; otherwise returns 0. This slightly differs from the same
@@ -918,6 +895,9 @@ int floatx80_is_signaling_nan(floatx80 a_, float_status *status)
 
 int floatx80_is_quiet_nan(floatx80 a, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return floatx80_is_any_nan(a);
+#else
     if (status->snan_bit_is_one) {
         uint64_t aLow;
 
@@ -929,6 +909,7 @@ int floatx80_is_quiet_nan(floatx80 a, float_status *status)
         return ((a.high & 0x7FFF) == 0x7FFF)
             && (LIT64(0x8000000000000000) <= ((uint64_t)(a.low << 1)));
     }
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -939,6 +920,9 @@ int floatx80_is_quiet_nan(floatx80 a, float_status *status)
 
 int floatx80_is_signaling_nan(floatx80 a, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return 0;
+#else
     if (status->snan_bit_is_one) {
         return ((a.high & 0x7FFF) == 0x7FFF)
             && ((a.low << 1) >= 0x8000000000000000ULL);
@@ -950,8 +934,8 @@ int floatx80_is_signaling_nan(floatx80 a, float_status *status)
             && (uint64_t)(aLow << 1)
             && (a.low == aLow);
     }
-}
 #endif
+}
 
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the extended double-precision floating point value
@@ -1060,17 +1044,6 @@ floatx80 propagateFloatx80NaN(floatx80 a, floatx80 b, float_status *status)
     }
 }
 
-#ifdef NO_SIGNALING_NANS
-int float128_is_quiet_nan(float128 a_, float_status *status)
-{
-    return float128_is_any_nan(a_);
-}
-
-int float128_is_signaling_nan(float128 a_, float_status *status)
-{
-    return 0;
-}
-#else
 /*----------------------------------------------------------------------------
 | Returns 1 if the quadruple-precision floating-point value `a' is a quiet
 | NaN; otherwise returns 0.
@@ -1078,6 +1051,9 @@ int float128_is_signaling_nan(float128 a_, float_status *status)
 
 int float128_is_quiet_nan(float128 a, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return float128_is_any_nan(a);
+#else
     if (status->snan_bit_is_one) {
         return (((a.high >> 47) & 0xFFFF) == 0xFFFE)
             && (a.low || (a.high & 0x00007FFFFFFFFFFFULL));
@@ -1085,6 +1061,7 @@ int float128_is_quiet_nan(float128 a, float_status *status)
         return ((a.high << 1) >= 0xFFFF000000000000ULL)
             && (a.low || (a.high & 0x0000FFFFFFFFFFFFULL));
     }
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -1094,6 +1071,9 @@ int float128_is_quiet_nan(float128 a, float_status *status)
 
 int float128_is_signaling_nan(float128 a, float_status *status)
 {
+#ifdef NO_SIGNALING_NANS
+    return 0;
+#else
     if (status->snan_bit_is_one) {
         return ((a.high << 1) >= 0xFFFF000000000000ULL)
             && (a.low || (a.high & 0x0000FFFFFFFFFFFFULL));
@@ -1101,8 +1081,8 @@ int float128_is_signaling_nan(float128 a, float_status *status)
         return (((a.high >> 47) & 0xFFFF) == 0xFFFE)
             && (a.low || (a.high & LIT64(0x00007FFFFFFFFFFF)));
     }
-}
 #endif
+}
 
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the quadruple-precision floating point value `a' is
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 03/27] fpu/softfloat: Merge NO_SIGNALING_NANS definitions

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> Move the ifdef inside the relevant functions instead of
> duplicating the function declarations.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h | 100 +++++++++++++++----------------------
>  1 file changed, 40 insertions(+), 60 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 04/27] fpu/softfloat: Split floatXX_silence_nan from floatXX_maybe_silence_nan

[Richard Henderson]

The new function assumes that the input is an SNaN and
does not double-check.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 174 +++++++++++++++++++++++++------------
 include/fpu/softfloat.h    |   5 ++
 2 files changed, 123 insertions(+), 56 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 58b05718c8..4fc9ea4ac0 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -271,22 +271,35 @@ int float16_is_signaling_nan(float16 a_, float_status *status)
 #endif
 }
 
+/*----------------------------------------------------------------------------
+| Returns a quiet NaN from a signalling NaN for the half-precision
+| floating point value `a'.
+*----------------------------------------------------------------------------*/
+
+float16 float16_silence_nan(float16 a, float_status *status)
+{
+#ifdef NO_SIGNALING_NANS
+    g_assert_not_reached();
+#else
+    if (status->snan_bit_is_one) {
+        return float16_default_nan(status);
+    } else {
+        return a | (1 << 9);
+    }
+#endif
+}
+
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the half-precision floating point value `a' is a
 | signaling NaN; otherwise returns `a'.
 *----------------------------------------------------------------------------*/
-float16 float16_maybe_silence_nan(float16 a_, float_status *status)
+
+float16 float16_maybe_silence_nan(float16 a, float_status *status)
 {
-    if (float16_is_signaling_nan(a_, status)) {
-        if (status->snan_bit_is_one) {
-            return float16_default_nan(status);
-        } else {
-            uint16_t a = float16_val(a_);
-            a |= (1 << 9);
-            return make_float16(a);
-        }
+    if (float16_is_signaling_nan(a, status)) {
+        return float16_silence_nan(a, status);
     }
-    return a_;
+    return a;
 }
 
 /*----------------------------------------------------------------------------
@@ -367,30 +380,40 @@ int float32_is_signaling_nan(float32 a_, float_status *status)
 #endif
 }
 
+/*----------------------------------------------------------------------------
+| Returns a quiet NaN from a signalling NaN for the single-precision
+| floating point value `a'.
+*----------------------------------------------------------------------------*/
+
+float32 float32_silence_nan(float32 a, float_status *status)
+{
+#ifdef NO_SIGNALING_NANS
+    g_assert_not_reached();
+#else
+    if (status->snan_bit_is_one) {
+# ifdef TARGET_HPPA
+        a &= ~0x00400000;
+        a |=  0x00200000;
+        return a;
+# else
+        return float32_default_nan(status);
+# endif
+    } else {
+        return a | (1 << 22);
+    }
+#endif
+}
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the single-precision floating point value `a' is a
 | signaling NaN; otherwise returns `a'.
 *----------------------------------------------------------------------------*/
 
-float32 float32_maybe_silence_nan(float32 a_, float_status *status)
+float32 float32_maybe_silence_nan(float32 a, float_status *status)
 {
-    if (float32_is_signaling_nan(a_, status)) {
-        if (status->snan_bit_is_one) {
-#ifdef TARGET_HPPA
-            uint32_t a = float32_val(a_);
-            a &= ~0x00400000;
-            a |=  0x00200000;
-            return make_float32(a);
-#else
-            return float32_default_nan(status);
-#endif
-        } else {
-            uint32_t a = float32_val(a_);
-            a |= (1 << 22);
-            return make_float32(a);
-        }
+    if (float32_is_signaling_nan(a, status)) {
+        return float32_silence_nan(a, status);
     }
-    return a_;
+    return a;
 }
 
 /*----------------------------------------------------------------------------
@@ -776,30 +799,41 @@ int float64_is_signaling_nan(float64 a_, float_status *status)
 #endif
 }
 
+/*----------------------------------------------------------------------------
+| Returns a quiet NaN from a signalling NaN for the double-precision
+| floating point value `a'.
+*----------------------------------------------------------------------------*/
+
+float64 float64_silence_nan(float64 a, float_status *status)
+{
+#ifdef NO_SIGNALING_NANS
+    g_assert_not_reached();
+#else
+    if (status->snan_bit_is_one) {
+# ifdef TARGET_HPPA
+        a &= ~0x0008000000000000ULL;
+        a |=  0x0004000000000000ULL;
+        return a;
+# else
+        return float64_default_nan(status);
+# endif
+    } else {
+        return a | LIT64(0x0008000000000000);
+    }
+#endif
+}
+
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the double-precision floating point value `a' is a
 | signaling NaN; otherwise returns `a'.
 *----------------------------------------------------------------------------*/
 
-float64 float64_maybe_silence_nan(float64 a_, float_status *status)
+float64 float64_maybe_silence_nan(float64 a, float_status *status)
 {
-    if (float64_is_signaling_nan(a_, status)) {
-        if (status->snan_bit_is_one) {
-#ifdef TARGET_HPPA
-            uint64_t a = float64_val(a_);
-            a &= ~0x0008000000000000ULL;
-            a |=  0x0004000000000000ULL;
-            return make_float64(a);
-#else
-            return float64_default_nan(status);
-#endif
-        } else {
-            uint64_t a = float64_val(a_);
-            a |= LIT64(0x0008000000000000);
-            return make_float64(a);
-        }
+    if (float64_is_signaling_nan(a, status)) {
+        return float64_silence_nan(a, status);
     }
-    return a_;
+    return a;
 }
 
 /*----------------------------------------------------------------------------
@@ -937,6 +971,25 @@ int floatx80_is_signaling_nan(floatx80 a, float_status *status)
 #endif
 }
 
+/*----------------------------------------------------------------------------
+| Returns a quiet NaN from a signalling NaN for the extended double-precision
+| floating point value `a'.
+*----------------------------------------------------------------------------*/
+
+floatx80 floatx80_silence_nan(floatx80 a, float_status *status)
+{
+#ifdef NO_SIGNALING_NANS
+    g_assert_not_reached();
+#else
+    if (status->snan_bit_is_one) {
+        return floatx80_default_nan(status);
+    } else {
+        a.low |= LIT64(0xC000000000000000);
+        return a;
+    }
+#endif
+}
+
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the extended double-precision floating point value
 | `a' is a signaling NaN; otherwise returns `a'.
@@ -945,12 +998,7 @@ int floatx80_is_signaling_nan(floatx80 a, float_status *status)
 floatx80 floatx80_maybe_silence_nan(floatx80 a, float_status *status)
 {
     if (floatx80_is_signaling_nan(a, status)) {
-        if (status->snan_bit_is_one) {
-            a = floatx80_default_nan(status);
-        } else {
-            a.low |= LIT64(0xC000000000000000);
-            return a;
-        }
+        return floatx80_silence_nan(a, status);
     }
     return a;
 }
@@ -1084,6 +1132,25 @@ int float128_is_signaling_nan(float128 a, float_status *status)
 #endif
 }
 
+/*----------------------------------------------------------------------------
+| Returns a quiet NaN from a signalling NaN for the quadruple-precision
+| floating point value `a'.
+*----------------------------------------------------------------------------*/
+
+float128 float128_silence_nan(float128 a, float_status *status)
+{
+#ifdef NO_SIGNALING_NANS
+    g_assert_not_reached();
+#else
+    if (status->snan_bit_is_one) {
+        return float128_default_nan(status);
+    } else {
+        a.high |= LIT64(0x0000800000000000);
+        return a;
+    }
+#endif
+}
+
 /*----------------------------------------------------------------------------
 | Returns a quiet NaN if the quadruple-precision floating point value `a' is
 | a signaling NaN; otherwise returns `a'.
@@ -1092,12 +1159,7 @@ int float128_is_signaling_nan(float128 a, float_status *status)
 float128 float128_maybe_silence_nan(float128 a, float_status *status)
 {
     if (float128_is_signaling_nan(a, status)) {
-        if (status->snan_bit_is_one) {
-            a = float128_default_nan(status);
-        } else {
-            a.high |= LIT64(0x0000800000000000);
-            return a;
-        }
+        return float128_silence_nan(a, status);
     }
     return a;
 }
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 36626a501b..43962dc3f5 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -257,6 +257,7 @@ int float16_compare_quiet(float16, float16, float_status *status);
 
 int float16_is_quiet_nan(float16, float_status *status);
 int float16_is_signaling_nan(float16, float_status *status);
+float16 float16_silence_nan(float16, float_status *status);
 float16 float16_maybe_silence_nan(float16, float_status *status);
 
 static inline int float16_is_any_nan(float16 a)
@@ -368,6 +369,7 @@ float32 float32_minnummag(float32, float32, float_status *status);
 float32 float32_maxnummag(float32, float32, float_status *status);
 int float32_is_quiet_nan(float32, float_status *status);
 int float32_is_signaling_nan(float32, float_status *status);
+float32 float32_silence_nan(float32, float_status *status);
 float32 float32_maybe_silence_nan(float32, float_status *status);
 float32 float32_scalbn(float32, int, float_status *status);
 
@@ -497,6 +499,7 @@ float64 float64_minnummag(float64, float64, float_status *status);
 float64 float64_maxnummag(float64, float64, float_status *status);
 int float64_is_quiet_nan(float64 a, float_status *status);
 int float64_is_signaling_nan(float64, float_status *status);
+float64 float64_silence_nan(float64, float_status *status);
 float64 float64_maybe_silence_nan(float64, float_status *status);
 float64 float64_scalbn(float64, int, float_status *status);
 
@@ -600,6 +603,7 @@ int floatx80_compare(floatx80, floatx80, float_status *status);
 int floatx80_compare_quiet(floatx80, floatx80, float_status *status);
 int floatx80_is_quiet_nan(floatx80, float_status *status);
 int floatx80_is_signaling_nan(floatx80, float_status *status);
+floatx80 floatx80_silence_nan(floatx80, float_status *status);
 floatx80 floatx80_maybe_silence_nan(floatx80, float_status *status);
 floatx80 floatx80_scalbn(floatx80, int, float_status *status);
 
@@ -811,6 +815,7 @@ int float128_compare(float128, float128, float_status *status);
 int float128_compare_quiet(float128, float128, float_status *status);
 int float128_is_quiet_nan(float128, float_status *status);
 int float128_is_signaling_nan(float128, float_status *status);
+float128 float128_silence_nan(float128, float_status *status);
 float128 float128_maybe_silence_nan(float128, float_status *status);
 float128 float128_scalbn(float128, int, float_status *status);
 
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 04/27] fpu/softfloat: Split floatXX_silence_nan from floatXX_maybe_silence_nan

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> The new function assumes that the input is an SNaN and
> does not double-check.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h | 174 +++++++++++++++++++++++++------------
>  include/fpu/softfloat.h    |   5 ++
>  2 files changed, 123 insertions(+), 56 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 05/27] fpu/softfloat: Move softfloat-specialize.h below FloatParts definition

[Richard Henderson]

We want to be able to specialize on the canonical representation.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index bc0f52fa54..353893aaea 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -95,16 +95,6 @@ this code that are retained.
 *----------------------------------------------------------------------------*/
 #include "fpu/softfloat-macros.h"
 
-/*----------------------------------------------------------------------------
-| Functions and definitions to determine:  (1) whether tininess for underflow
-| is detected before or after rounding by default, (2) what (if anything)
-| happens when exceptions are raised, (3) how signaling NaNs are distinguished
-| from quiet NaNs, (4) the default generated quiet NaNs, and (5) how NaNs
-| are propagated from function inputs to output.  These details are target-
-| specific.
-*----------------------------------------------------------------------------*/
-#include "softfloat-specialize.h"
-
 /*----------------------------------------------------------------------------
 | Returns the fraction bits of the half-precision floating-point value `a'.
 *----------------------------------------------------------------------------*/
@@ -247,6 +237,16 @@ typedef struct {
     uint64_t roundeven_mask;
 } FloatFmt;
 
+/*----------------------------------------------------------------------------
+| Functions and definitions to determine:  (1) whether tininess for underflow
+| is detected before or after rounding by default, (2) what (if anything)
+| happens when exceptions are raised, (3) how signaling NaNs are distinguished
+| from quiet NaNs, (4) the default generated quiet NaNs, and (5) how NaNs
+| are propagated from function inputs to output.  These details are target-
+| specific.
+*----------------------------------------------------------------------------*/
+#include "softfloat-specialize.h"
+
 /* Expand fields based on the size of exponent and fraction */
 #define FLOAT_PARAMS(E, F)                                           \
     .exp_size       = E,                                             \
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 05/27] fpu/softfloat: Move softfloat-specialize.h below FloatParts definition

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> We want to be able to specialize on the canonical representation.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index bc0f52fa54..353893aaea 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -95,16 +95,6 @@ this code that are retained.
>  *----------------------------------------------------------------------------*/
>  #include "fpu/softfloat-macros.h"
>
> -/*----------------------------------------------------------------------------
> -| Functions and definitions to determine:  (1) whether tininess for underflow
> -| is detected before or after rounding by default, (2) what (if anything)
> -| happens when exceptions are raised, (3) how signaling NaNs are distinguished
> -| from quiet NaNs, (4) the default generated quiet NaNs, and (5) how NaNs
> -| are propagated from function inputs to output.  These details are target-
> -| specific.
> -*----------------------------------------------------------------------------*/
> -#include "softfloat-specialize.h"
> -
>  /*----------------------------------------------------------------------------
>  | Returns the fraction bits of the half-precision floating-point value `a'.
>  *----------------------------------------------------------------------------*/
> @@ -247,6 +237,16 @@ typedef struct {
>      uint64_t roundeven_mask;
>  } FloatFmt;
>
> +/*----------------------------------------------------------------------------
> +| Functions and definitions to determine:  (1) whether tininess for underflow
> +| is detected before or after rounding by default, (2) what (if anything)
> +| happens when exceptions are raised, (3) how signaling NaNs are distinguished
> +| from quiet NaNs, (4) the default generated quiet NaNs, and (5) how NaNs
> +| are propagated from function inputs to output.  These details are target-
> +| specific.
> +*----------------------------------------------------------------------------*/
> +#include "softfloat-specialize.h"
> +
>  /* Expand fields based on the size of exponent and fraction */
>  #define FLOAT_PARAMS(E, F)                                           \
>      .exp_size       = E,                                             \
> --
> 2.17.0
>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 06/27] fpu/softfloat: Canonicalize NaN fraction

[Richard Henderson]

Shift the NaN fraction to a canonical position, much like we do
for the fraction of normal numbers.  Immediately, this simplifies
the float-to-float conversion.  Later, this will facilitate
manipulation of NaNs within the shared code paths.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 353893aaea..a56d3d975b 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -330,10 +330,11 @@ static FloatParts canonicalize(FloatParts part, const FloatFmt *parm,
         if (part.frac == 0) {
             part.cls = float_class_inf;
         } else {
+            part.frac <<= parm->frac_shift;
 #ifdef NO_SIGNALING_NANS
             part.cls = float_class_qnan;
 #else
-            int64_t msb = part.frac << (parm->frac_shift + 2);
+            int64_t msb = part.frac << 2;
             if ((msb < 0) == status->snan_bit_is_one) {
                 part.cls = float_class_snan;
             } else {
@@ -480,6 +481,7 @@ static FloatParts round_canonical(FloatParts p, float_status *s,
     case float_class_qnan:
     case float_class_snan:
         exp = exp_max;
+        frac >>= parm->frac_shift;
         break;
 
     default:
@@ -503,6 +505,7 @@ static float16 float16_round_pack_canonical(FloatParts p, float_status *s)
     case float_class_dnan:
         return float16_default_nan(s);
     case float_class_msnan:
+        p.frac >>= float16_params.frac_shift;
         return float16_maybe_silence_nan(float16_pack_raw(p), s);
     default:
         p = round_canonical(p, s, &float16_params);
@@ -521,6 +524,7 @@ static float32 float32_round_pack_canonical(FloatParts p, float_status *s)
     case float_class_dnan:
         return float32_default_nan(s);
     case float_class_msnan:
+        p.frac >>= float32_params.frac_shift;
         return float32_maybe_silence_nan(float32_pack_raw(p), s);
     default:
         p = round_canonical(p, s, &float32_params);
@@ -539,6 +543,7 @@ static float64 float64_round_pack_canonical(FloatParts p, float_status *s)
     case float_class_dnan:
         return float64_default_nan(s);
     case float_class_msnan:
+        p.frac >>= float64_params.frac_shift;
         return float64_maybe_silence_nan(float64_pack_raw(p), s);
     default:
         p = round_canonical(p, s, &float64_params);
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 06/27] fpu/softfloat: Canonicalize NaN fraction

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> Shift the NaN fraction to a canonical position, much like we do
> for the fraction of normal numbers.  Immediately, this simplifies
> the float-to-float conversion.  Later, this will facilitate
> manipulation of NaNs within the shared code paths.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

The commit message says it simplifies the float-to-float codepath,
but the patch doesn't touch that: it's missing the part that was in
v1 of the series that removed the
        a.frac = a.frac << (64 - srcf->frac_size) >> (64 - dstf->frac_size);
line and updated the associated comment.

Isn't that change necessary in this patch if we're changing the
canonical representation of NaNs ?

> ---
>  fpu/softfloat.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 353893aaea..a56d3d975b 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -330,10 +330,11 @@ static FloatParts canonicalize(FloatParts part, const FloatFmt *parm,
>          if (part.frac == 0) {
>              part.cls = float_class_inf;
>          } else {
> +            part.frac <<= parm->frac_shift;
>  #ifdef NO_SIGNALING_NANS
>              part.cls = float_class_qnan;
>  #else
> -            int64_t msb = part.frac << (parm->frac_shift + 2);
> +            int64_t msb = part.frac << 2;
>              if ((msb < 0) == status->snan_bit_is_one) {
>                  part.cls = float_class_snan;
>              } else {
> @@ -480,6 +481,7 @@ static FloatParts round_canonical(FloatParts p, float_status *s,
>      case float_class_qnan:
>      case float_class_snan:
>          exp = exp_max;
> +        frac >>= parm->frac_shift;
>          break;
>
>      default:
> @@ -503,6 +505,7 @@ static float16 float16_round_pack_canonical(FloatParts p, float_status *s)
>      case float_class_dnan:
>          return float16_default_nan(s);
>      case float_class_msnan:
> +        p.frac >>= float16_params.frac_shift;
>          return float16_maybe_silence_nan(float16_pack_raw(p), s);
>      default:
>          p = round_canonical(p, s, &float16_params);
> @@ -521,6 +524,7 @@ static float32 float32_round_pack_canonical(FloatParts p, float_status *s)
>      case float_class_dnan:
>          return float32_default_nan(s);
>      case float_class_msnan:
> +        p.frac >>= float32_params.frac_shift;
>          return float32_maybe_silence_nan(float32_pack_raw(p), s);
>      default:
>          p = round_canonical(p, s, &float32_params);
> @@ -539,6 +543,7 @@ static float64 float64_round_pack_canonical(FloatParts p, float_status *s)
>      case float_class_dnan:
>          return float64_default_nan(s);
>      case float_class_msnan:
> +        p.frac >>= float64_params.frac_shift;
>          return float64_maybe_silence_nan(float64_pack_raw(p), s);
>      default:
>          p = round_canonical(p, s, &float64_params);
> --
> 2.17.0
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 06/27] fpu/softfloat: Canonicalize NaN fraction

[Richard Henderson]

On 05/14/2018 03:29 AM, Peter Maydell wrote:
> On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
>> Shift the NaN fraction to a canonical position, much like we do
>> for the fraction of normal numbers.  Immediately, this simplifies
>> the float-to-float conversion.  Later, this will facilitate
>> manipulation of NaNs within the shared code paths.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> The commit message says it simplifies the float-to-float codepath,
> but the patch doesn't touch that: it's missing the part that was in
> v1 of the series that removed the
>         a.frac = a.frac << (64 - srcf->frac_size) >> (64 - dstf->frac_size);
> line and updated the associated comment.
> 
> Isn't that change necessary in this patch if we're changing the
> canonical representation of NaNs ?

With the reordering of the patches, the offending line never gets added.
So, yeah, the text should be re-worded here.


r~

[Qemu-devel] [PATCH v2 07/27] fpu/softfloat: Introduce parts_is_snan_frac

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 15 +++++++++++++++
 fpu/softfloat.c            | 12 ++----------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 4fc9ea4ac0..515cb12cfa 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -86,6 +86,21 @@ this code that are retained.
 #define NO_SIGNALING_NANS 1
 #endif
 
+/*----------------------------------------------------------------------------
+| For the deconstructed floating-point with fraction FRAC, return true
+| if the fraction represents a signalling NaN; otherwise false.
+*----------------------------------------------------------------------------*/
+
+static bool parts_is_snan_frac(uint64_t frac, float_status *status)
+{
+#ifdef NO_SIGNALING_NANS
+    return false;
+#else
+    flag msb = extract64(frac, DECOMPOSED_BINARY_POINT - 1, 1);
+    return msb == status->snan_bit_is_one;
+#endif
+}
+
 /*----------------------------------------------------------------------------
 | The pattern for a default generated half-precision NaN.
 *----------------------------------------------------------------------------*/
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index a56d3d975b..cb68f2eb20 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -331,16 +331,8 @@ static FloatParts canonicalize(FloatParts part, const FloatFmt *parm,
             part.cls = float_class_inf;
         } else {
             part.frac <<= parm->frac_shift;
-#ifdef NO_SIGNALING_NANS
-            part.cls = float_class_qnan;
-#else
-            int64_t msb = part.frac << 2;
-            if ((msb < 0) == status->snan_bit_is_one) {
-                part.cls = float_class_snan;
-            } else {
-                part.cls = float_class_qnan;
-            }
-#endif
+            part.cls = (parts_is_snan_frac(part.frac, status)
+                        ? float_class_snan : float_class_qnan);
         }
     } else if (part.exp == 0) {
         if (likely(part.frac == 0)) {
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 07/27] fpu/softfloat: Introduce parts_is_snan_frac

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h | 15 +++++++++++++++
>  fpu/softfloat.c            | 12 ++----------
>  2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
> index 4fc9ea4ac0..515cb12cfa 100644
> --- a/fpu/softfloat-specialize.h
> +++ b/fpu/softfloat-specialize.h
> @@ -86,6 +86,21 @@ this code that are retained.
>  #define NO_SIGNALING_NANS 1
>  #endif
>
> +/*----------------------------------------------------------------------------
> +| For the deconstructed floating-point with fraction FRAC, return true
> +| if the fraction represents a signalling NaN; otherwise false.
> +*----------------------------------------------------------------------------*/
> +
> +static bool parts_is_snan_frac(uint64_t frac, float_status *status)
> +{
> +#ifdef NO_SIGNALING_NANS
> +    return false;
> +#else
> +    flag msb = extract64(frac, DECOMPOSED_BINARY_POINT - 1, 1);
> +    return msb == status->snan_bit_is_one;
> +#endif
> +}
> +
>  /*----------------------------------------------------------------------------
>  | The pattern for a default generated half-precision NaN.
>  *----------------------------------------------------------------------------*/
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index a56d3d975b..cb68f2eb20 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -331,16 +331,8 @@ static FloatParts canonicalize(FloatParts part, const FloatFmt *parm,
>              part.cls = float_class_inf;
>          } else {
>              part.frac <<= parm->frac_shift;
> -#ifdef NO_SIGNALING_NANS
> -            part.cls = float_class_qnan;
> -#else
> -            int64_t msb = part.frac << 2;
> -            if ((msb < 0) == status->snan_bit_is_one) {
> -                part.cls = float_class_snan;
> -            } else {
> -                part.cls = float_class_qnan;
> -            }
> -#endif
> +            part.cls = (parts_is_snan_frac(part.frac, status)
> +                        ? float_class_snan : float_class_qnan);
>          }
>      } else if (part.exp == 0) {
>          if (likely(part.frac == 0)) {
> --
> 2.17.0

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 08/27] fpu/softfloat: Replace float_class_dnan with parts_default_nan

[Richard Henderson]

With a canonical representation of NaNs, we can return the
default nan directly rather than delay the expansion until
the final format is known.

Note one case where we uselessly assigned to a.sign, which was
overwritten/ignored later when expanding float_class_dnan.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 37 +++++++++++++++++++++++++++++++++++++
 fpu/softfloat.c            | 38 +++++++++++---------------------------
 2 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 515cb12cfa..0d3d81a52b 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -101,6 +101,43 @@ static bool parts_is_snan_frac(uint64_t frac, float_status *status)
 #endif
 }
 
+/*----------------------------------------------------------------------------
+| The pattern for a default generated deconstructed floating-point NaN.
+*----------------------------------------------------------------------------*/
+
+static FloatParts parts_default_nan(float_status *status)
+{
+    bool sign = 0;
+    uint64_t frac;
+
+#if defined(TARGET_SPARC) || defined(TARGET_M68K)
+    frac = (1ULL << DECOMPOSED_BINARY_POINT) - 1;
+#elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA) || \
+      defined(TARGET_S390X) || defined(TARGET_RISCV)
+    frac = 1ULL << (DECOMPOSED_BINARY_POINT - 1);
+#elif defined(TARGET_HPPA)
+    frac = 1ULL << (DECOMPOSED_BINARY_POINT - 2);
+#else
+    if (status->snan_bit_is_one) {
+        frac = (1ULL << (DECOMPOSED_BINARY_POINT - 1)) - 1;
+    } else {
+#if defined(TARGET_MIPS)
+        frac = 1ULL << (DECOMPOSED_BINARY_POINT - 1);
+#else
+        frac = 1ULL << (DECOMPOSED_BINARY_POINT - 1);
+        sign = 1;
+#endif
+    }
+#endif
+
+    return (FloatParts) {
+        .cls = float_class_qnan,
+        .sign = sign,
+        .exp = INT_MAX,
+        .frac = frac
+    };
+}
+
 /*----------------------------------------------------------------------------
 | The pattern for a default generated half-precision NaN.
 *----------------------------------------------------------------------------*/
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index cb68f2eb20..d16b11a85b 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -188,7 +188,6 @@ typedef enum __attribute__ ((__packed__)) {
     float_class_inf,
     float_class_qnan,  /* all NaNs from here */
     float_class_snan,
-    float_class_dnan,
     float_class_msnan, /* maybe silenced */
 } FloatClass;
 
@@ -494,8 +493,6 @@ static FloatParts float16_unpack_canonical(float16 f, float_status *s)
 static float16 float16_round_pack_canonical(FloatParts p, float_status *s)
 {
     switch (p.cls) {
-    case float_class_dnan:
-        return float16_default_nan(s);
     case float_class_msnan:
         p.frac >>= float16_params.frac_shift;
         return float16_maybe_silence_nan(float16_pack_raw(p), s);
@@ -513,8 +510,6 @@ static FloatParts float32_unpack_canonical(float32 f, float_status *s)
 static float32 float32_round_pack_canonical(FloatParts p, float_status *s)
 {
     switch (p.cls) {
-    case float_class_dnan:
-        return float32_default_nan(s);
     case float_class_msnan:
         p.frac >>= float32_params.frac_shift;
         return float32_maybe_silence_nan(float32_pack_raw(p), s);
@@ -532,8 +527,6 @@ static FloatParts float64_unpack_canonical(float64 f, float_status *s)
 static float64 float64_round_pack_canonical(FloatParts p, float_status *s)
 {
     switch (p.cls) {
-    case float_class_dnan:
-        return float64_default_nan(s);
     case float_class_msnan:
         p.frac >>= float64_params.frac_shift;
         return float64_maybe_silence_nan(float64_pack_raw(p), s);
@@ -566,7 +559,7 @@ static FloatParts return_nan(FloatParts a, float_status *s)
         /* fall through */
     case float_class_qnan:
         if (s->default_nan_mode) {
-            a.cls = float_class_dnan;
+            return parts_default_nan(s);
         }
         break;
 
@@ -583,7 +576,7 @@ static FloatParts pick_nan(FloatParts a, FloatParts b, float_status *s)
     }
 
     if (s->default_nan_mode) {
-        a.cls = float_class_dnan;
+        return parts_default_nan(s);
     } else {
         if (pickNaN(is_qnan(a.cls), is_snan(a.cls),
                     is_qnan(b.cls), is_snan(b.cls),
@@ -614,8 +607,7 @@ static FloatParts pick_nan_muladd(FloatParts a, FloatParts b, FloatParts c,
         /* Note that this check is after pickNaNMulAdd so that function
          * has an opportunity to set the Invalid flag.
          */
-        a.cls = float_class_dnan;
-        return a;
+        which = 3;
     }
 
     switch (which) {
@@ -628,8 +620,7 @@ static FloatParts pick_nan_muladd(FloatParts a, FloatParts b, FloatParts c,
         a = c;
         break;
     case 3:
-        a.cls = float_class_dnan;
-        return a;
+        return parts_default_nan(s);
     default:
         g_assert_not_reached();
     }
@@ -682,7 +673,7 @@ static FloatParts addsub_floats(FloatParts a, FloatParts b, bool subtract,
         if (a.cls == float_class_inf) {
             if (b.cls == float_class_inf) {
                 float_raise(float_flag_invalid, s);
-                a.cls = float_class_dnan;
+                return parts_default_nan(s);
             }
             return a;
         }
@@ -828,9 +819,7 @@ static FloatParts mul_floats(FloatParts a, FloatParts b, float_status *s)
     if ((a.cls == float_class_inf && b.cls == float_class_zero) ||
         (a.cls == float_class_zero && b.cls == float_class_inf)) {
         s->float_exception_flags |= float_flag_invalid;
-        a.cls = float_class_dnan;
-        a.sign = sign;
-        return a;
+        return parts_default_nan(s);
     }
     /* Multiply by 0 or Inf */
     if (a.cls == float_class_inf || a.cls == float_class_zero) {
@@ -908,8 +897,7 @@ static FloatParts muladd_floats(FloatParts a, FloatParts b, FloatParts c,
 
     if (inf_zero) {
         s->float_exception_flags |= float_flag_invalid;
-        a.cls = float_class_dnan;
-        return a;
+        return parts_default_nan(s);
     }
 
     if (flags & float_muladd_negate_c) {
@@ -933,12 +921,12 @@ static FloatParts muladd_floats(FloatParts a, FloatParts b, FloatParts c,
     if (c.cls == float_class_inf) {
         if (p_class == float_class_inf && p_sign != c.sign) {
             s->float_exception_flags |= float_flag_invalid;
-            a.cls = float_class_dnan;
+            return parts_default_nan(s);
         } else {
             a.cls = float_class_inf;
             a.sign = c.sign ^ sign_flip;
+            return a;
         }
-        return a;
     }
 
     if (p_class == float_class_inf) {
@@ -1148,8 +1136,7 @@ static FloatParts div_floats(FloatParts a, FloatParts b, float_status *s)
         &&
         (a.cls == float_class_inf || a.cls == float_class_zero)) {
         s->float_exception_flags |= float_flag_invalid;
-        a.cls = float_class_dnan;
-        return a;
+        return parts_default_nan(s);
     }
     /* Inf / x or 0 / x */
     if (a.cls == float_class_inf || a.cls == float_class_zero) {
@@ -1347,7 +1334,6 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
     switch (p.cls) {
     case float_class_snan:
     case float_class_qnan:
-    case float_class_dnan:
     case float_class_msnan:
         s->float_exception_flags = orig_flags | float_flag_invalid;
         return max;
@@ -1439,7 +1425,6 @@ static uint64_t round_to_uint_and_pack(FloatParts in, int rmode, uint64_t max,
     switch (p.cls) {
     case float_class_snan:
     case float_class_qnan:
-    case float_class_dnan:
     case float_class_msnan:
         s->float_exception_flags = orig_flags | float_flag_invalid;
         return max;
@@ -1940,8 +1925,7 @@ static FloatParts sqrt_float(FloatParts a, float_status *s, const FloatFmt *p)
     }
     if (a.sign) {
         s->float_exception_flags |= float_flag_invalid;
-        a.cls = float_class_dnan;
-        return a;
+        return parts_default_nan(s);
     }
     if (a.cls == float_class_inf) {
         return a;  /* sqrt(+inf) = +inf */
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 08/27] fpu/softfloat: Replace float_class_dnan with parts_default_nan

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> With a canonical representation of NaNs, we can return the
> default nan directly rather than delay the expansion until
> the final format is known.
>
> Note one case where we uselessly assigned to a.sign, which was
> overwritten/ignored later when expanding float_class_dnan.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h | 37 +++++++++++++++++++++++++++++++++++++
>  fpu/softfloat.c            | 38 +++++++++++---------------------------
>  2 files changed, 48 insertions(+), 27 deletions(-)
>
> diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
> index 515cb12cfa..0d3d81a52b 100644
> --- a/fpu/softfloat-specialize.h
> +++ b/fpu/softfloat-specialize.h
> @@ -101,6 +101,43 @@ static bool parts_is_snan_frac(uint64_t frac, float_status *status)
>  #endif
>  }
>
> +/*----------------------------------------------------------------------------
> +| The pattern for a default generated deconstructed floating-point NaN.
> +*----------------------------------------------------------------------------*/
> +
> +static FloatParts parts_default_nan(float_status *status)
> +{
> +    bool sign = 0;
> +    uint64_t frac;
> +
> +#if defined(TARGET_SPARC) || defined(TARGET_M68K)
> +    frac = (1ULL << DECOMPOSED_BINARY_POINT) - 1;
> +#elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA) || \
> +      defined(TARGET_S390X) || defined(TARGET_RISCV)
> +    frac = 1ULL << (DECOMPOSED_BINARY_POINT - 1);
> +#elif defined(TARGET_HPPA)
> +    frac = 1ULL << (DECOMPOSED_BINARY_POINT - 2);
> +#else
> +    if (status->snan_bit_is_one) {
> +        frac = (1ULL << (DECOMPOSED_BINARY_POINT - 1)) - 1;
> +    } else {
> +#if defined(TARGET_MIPS)
> +        frac = 1ULL << (DECOMPOSED_BINARY_POINT - 1);
> +#else
> +        frac = 1ULL << (DECOMPOSED_BINARY_POINT - 1);
> +        sign = 1;
> +#endif
> +    }
> +#endif

We can probably clean this logic up later (for instance the MIPS
case is the same as the PPC/ARM/Alpha/S390/RISCV case, and it
would be helpful to specifically indicate who's using the
weird set-the-sign-bit case rather than having that be the
default), but this matches the current logic in float*_default_nan()
so it's easier to review this way.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 09/27] fpu/softfloat: Replace float_class_msnan with parts_silence_nan

[Richard Henderson]

With a canonical representation of NaNs, we can silence an SNaN
immediately rather than delay until the final format is known.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 23 ++++++++++++++++++++++
 fpu/softfloat.c            | 40 ++++++++++----------------------------
 2 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 0d3d81a52b..571d1df378 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -138,6 +138,29 @@ static FloatParts parts_default_nan(float_status *status)
     };
 }
 
+/*----------------------------------------------------------------------------
+| Returns a quiet NaN from a signalling NaN for the deconstructed
+| floating-point parts.
+*----------------------------------------------------------------------------*/
+
+static FloatParts parts_silence_nan(FloatParts a, float_status *status)
+{
+#ifdef NO_SIGNALING_NANS
+    g_assert_not_reached();
+#elif defined(TARGET_HPPA)
+    a.frac &= ~(1ULL << (DECOMPOSED_BINARY_POINT - 1));
+    a.frac |= 1ULL << (DECOMPOSED_BINARY_POINT - 2);
+#else
+    if (status->snan_bit_is_one) {
+        return parts_default_nan(status);
+    } else {
+        a.frac |= 1ULL << (DECOMPOSED_BINARY_POINT - 1);
+    }
+#endif
+    a.cls = float_class_qnan;
+    return a;
+}
+
 /*----------------------------------------------------------------------------
 | The pattern for a default generated half-precision NaN.
 *----------------------------------------------------------------------------*/
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index d16b11a85b..c8f683963f 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -188,7 +188,6 @@ typedef enum __attribute__ ((__packed__)) {
     float_class_inf,
     float_class_qnan,  /* all NaNs from here */
     float_class_snan,
-    float_class_msnan, /* maybe silenced */
 } FloatClass;
 
 /*
@@ -492,14 +491,7 @@ static FloatParts float16_unpack_canonical(float16 f, float_status *s)
 
 static float16 float16_round_pack_canonical(FloatParts p, float_status *s)
 {
-    switch (p.cls) {
-    case float_class_msnan:
-        p.frac >>= float16_params.frac_shift;
-        return float16_maybe_silence_nan(float16_pack_raw(p), s);
-    default:
-        p = round_canonical(p, s, &float16_params);
-        return float16_pack_raw(p);
-    }
+    return float16_pack_raw(round_canonical(p, s, &float16_params));
 }
 
 static FloatParts float32_unpack_canonical(float32 f, float_status *s)
@@ -509,14 +501,7 @@ static FloatParts float32_unpack_canonical(float32 f, float_status *s)
 
 static float32 float32_round_pack_canonical(FloatParts p, float_status *s)
 {
-    switch (p.cls) {
-    case float_class_msnan:
-        p.frac >>= float32_params.frac_shift;
-        return float32_maybe_silence_nan(float32_pack_raw(p), s);
-    default:
-        p = round_canonical(p, s, &float32_params);
-        return float32_pack_raw(p);
-    }
+    return float32_pack_raw(round_canonical(p, s, &float32_params));
 }
 
 static FloatParts float64_unpack_canonical(float64 f, float_status *s)
@@ -526,14 +511,7 @@ static FloatParts float64_unpack_canonical(float64 f, float_status *s)
 
 static float64 float64_round_pack_canonical(FloatParts p, float_status *s)
 {
-    switch (p.cls) {
-    case float_class_msnan:
-        p.frac >>= float64_params.frac_shift;
-        return float64_maybe_silence_nan(float64_pack_raw(p), s);
-    default:
-        p = round_canonical(p, s, &float64_params);
-        return float64_pack_raw(p);
-    }
+    return float64_pack_raw(round_canonical(p, s, &float64_params));
 }
 
 /* Simple helpers for checking if what NaN we have */
@@ -555,7 +533,7 @@ static FloatParts return_nan(FloatParts a, float_status *s)
     switch (a.cls) {
     case float_class_snan:
         s->float_exception_flags |= float_flag_invalid;
-        a.cls = float_class_msnan;
+        a = parts_silence_nan(a, s);
         /* fall through */
     case float_class_qnan:
         if (s->default_nan_mode) {
@@ -584,7 +562,9 @@ static FloatParts pick_nan(FloatParts a, FloatParts b, float_status *s)
                     (a.frac == b.frac && a.sign < b.sign))) {
             a = b;
         }
-        a.cls = float_class_msnan;
+        if (is_snan(a.cls)) {
+            return parts_silence_nan(a, s);
+        }
     }
     return a;
 }
@@ -624,8 +604,10 @@ static FloatParts pick_nan_muladd(FloatParts a, FloatParts b, FloatParts c,
     default:
         g_assert_not_reached();
     }
-    a.cls = float_class_msnan;
 
+    if (is_snan(a.cls)) {
+        return parts_silence_nan(a, s);
+    }
     return a;
 }
 
@@ -1334,7 +1316,6 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
     switch (p.cls) {
     case float_class_snan:
     case float_class_qnan:
-    case float_class_msnan:
         s->float_exception_flags = orig_flags | float_flag_invalid;
         return max;
     case float_class_inf:
@@ -1425,7 +1406,6 @@ static uint64_t round_to_uint_and_pack(FloatParts in, int rmode, uint64_t max,
     switch (p.cls) {
     case float_class_snan:
     case float_class_qnan:
-    case float_class_msnan:
         s->float_exception_flags = orig_flags | float_flag_invalid;
         return max;
     case float_class_inf:
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 09/27] fpu/softfloat: Replace float_class_msnan with parts_silence_nan

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> With a canonical representation of NaNs, we can silence an SNaN
> immediately rather than delay until the final format is known.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h | 23 ++++++++++++++++++++++
>  fpu/softfloat.c            | 40 ++++++++++----------------------------
>  2 files changed, 33 insertions(+), 30 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 10/27] fpu/softfloat: re-factor float to float conversions

[Richard Henderson]

From: Alex Bennée <alex.bennee@linaro.org>

This allows us to delete a lot of additional boilerplate code which is
no longer needed. Currently the ieee flag is ignored (everything is
assumed to be ieee). Handling for ARM AHP will be in the next patch.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
v2
  - pass FloatFmt to float_to_float instead of sizes
  - split AHP handling to another patch
  - use rth's suggested re-packing (+ setting .exp)
v3
  - also rm extractFloat16Sign
v4
  - update for canonical nan handling
---
 fpu/softfloat-specialize.h |  40 ----
 include/fpu/softfloat.h    |   8 +-
 fpu/softfloat.c            | 437 ++++++-------------------------------
 3 files changed, 73 insertions(+), 412 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 571d1df378..995a0132c6 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -377,46 +377,6 @@ float16 float16_maybe_silence_nan(float16 a, float_status *status)
     return a;
 }
 
-/*----------------------------------------------------------------------------
-| Returns the result of converting the half-precision floating-point NaN
-| `a' to the canonical NaN format.  If `a' is a signaling NaN, the invalid
-| exception is raised.
-*----------------------------------------------------------------------------*/
-
-static commonNaNT float16ToCommonNaN(float16 a, float_status *status)
-{
-    commonNaNT z;
-
-    if (float16_is_signaling_nan(a, status)) {
-        float_raise(float_flag_invalid, status);
-    }
-    z.sign = float16_val(a) >> 15;
-    z.low = 0;
-    z.high = ((uint64_t) float16_val(a)) << 54;
-    return z;
-}
-
-/*----------------------------------------------------------------------------
-| Returns the result of converting the canonical NaN `a' to the half-
-| precision floating-point format.
-*----------------------------------------------------------------------------*/
-
-static float16 commonNaNToFloat16(commonNaNT a, float_status *status)
-{
-    uint16_t mantissa = a.high >> 54;
-
-    if (status->default_nan_mode) {
-        return float16_default_nan(status);
-    }
-
-    if (mantissa) {
-        return make_float16(((((uint16_t) a.sign) << 15)
-                             | (0x1F << 10) | mantissa));
-    } else {
-        return float16_default_nan(status);
-    }
-}
-
 /*----------------------------------------------------------------------------
 | Returns 1 if the single-precision floating-point value `a' is a quiet
 | NaN; otherwise returns 0.
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 43962dc3f5..a6860e858d 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -211,10 +211,10 @@ float128 uint64_to_float128(uint64_t, float_status *status);
 /*----------------------------------------------------------------------------
 | Software half-precision conversion routines.
 *----------------------------------------------------------------------------*/
-float16 float32_to_float16(float32, flag, float_status *status);
-float32 float16_to_float32(float16, flag, float_status *status);
-float16 float64_to_float16(float64 a, flag ieee, float_status *status);
-float64 float16_to_float64(float16 a, flag ieee, float_status *status);
+float16 float32_to_float16(float32, bool ieee, float_status *status);
+float32 float16_to_float32(float16, bool ieee, float_status *status);
+float16 float64_to_float16(float64 a, bool ieee, float_status *status);
+float64 float16_to_float64(float16 a, bool ieee, float_status *status);
 int16_t float16_to_int16(float16, float_status *status);
 uint16_t float16_to_uint16(float16 a, float_status *status);
 int16_t float16_to_int16_round_to_zero(float16, float_status *status);
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index c8f683963f..aa219223ff 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -113,15 +113,6 @@ static inline int extractFloat16Exp(float16 a)
     return (float16_val(a) >> 10) & 0x1f;
 }
 
-/*----------------------------------------------------------------------------
-| Returns the sign bit of the single-precision floating-point value `a'.
-*----------------------------------------------------------------------------*/
-
-static inline flag extractFloat16Sign(float16 a)
-{
-    return float16_val(a)>>15;
-}
-
 /*----------------------------------------------------------------------------
 | Returns the fraction bits of the single-precision floating-point value `a'.
 *----------------------------------------------------------------------------*/
@@ -1168,6 +1159,75 @@ float64 float64_div(float64 a, float64 b, float_status *status)
     return float64_round_pack_canonical(pr, status);
 }
 
+/*
+ * Float to Float conversions
+ *
+ * Returns the result of converting one float format to another. The
+ * conversion is performed according to the IEC/IEEE Standard for
+ * Binary Floating-Point Arithmetic.
+ *
+ * The float_to_float helper only needs to take care of raising
+ * invalid exceptions and handling the conversion on NaNs.
+ */
+
+static FloatParts float_to_float(FloatParts a,
+                                 const FloatFmt *srcf, const FloatFmt *dstf,
+                                 float_status *s)
+{
+    if (is_nan(a.cls)) {
+        if (is_snan(a.cls)) {
+            s->float_exception_flags |= float_flag_invalid;
+            a = parts_silence_nan(a, s);
+        }
+        if (s->default_nan_mode) {
+            return parts_default_nan(s);
+        }
+    }
+    return a;
+}
+
+float32 float16_to_float32(float16 a, bool ieee, float_status *s)
+{
+    FloatParts p = float16_unpack_canonical(a, s);
+    FloatParts pr = float_to_float(p, &float16_params, &float32_params, s);
+    return float32_round_pack_canonical(pr, s);
+}
+
+float64 float16_to_float64(float16 a, bool ieee, float_status *s)
+{
+    FloatParts p = float16_unpack_canonical(a, s);
+    FloatParts pr = float_to_float(p, &float16_params, &float64_params, s);
+    return float64_round_pack_canonical(pr, s);
+}
+
+float16 float32_to_float16(float32 a, bool ieee, float_status *s)
+{
+    FloatParts p = float32_unpack_canonical(a, s);
+    FloatParts pr = float_to_float(p, &float32_params, &float16_params, s);
+    return float16_round_pack_canonical(pr, s);
+}
+
+float64 float32_to_float64(float32 a, float_status *s)
+{
+    FloatParts p = float32_unpack_canonical(a, s);
+    FloatParts pr = float_to_float(p, &float32_params, &float64_params, s);
+    return float64_round_pack_canonical(pr, s);
+}
+
+float16 float64_to_float16(float64 a, bool ieee, float_status *s)
+{
+    FloatParts p = float64_unpack_canonical(a, s);
+    FloatParts pr = float_to_float(p, &float64_params, &float16_params, s);
+    return float16_round_pack_canonical(pr, s);
+}
+
+float32 float64_to_float32(float64 a, float_status *s)
+{
+    FloatParts p = float64_unpack_canonical(a, s);
+    FloatParts pr = float_to_float(p, &float64_params, &float32_params, s);
+    return float32_round_pack_canonical(pr, s);
+}
+
 /*
  * Rounds the floating-point value `a' to an integer, and returns the
  * result as a floating-point value. The operation is performed
@@ -3111,41 +3171,6 @@ float128 uint64_to_float128(uint64_t a, float_status *status)
     return normalizeRoundAndPackFloat128(0, 0x406E, a, 0, status);
 }
 
-
-
-
-/*----------------------------------------------------------------------------
-| Returns the result of converting the single-precision floating-point value
-| `a' to the double-precision floating-point format.  The conversion is
-| performed according to the IEC/IEEE Standard for Binary Floating-Point
-| Arithmetic.
-*----------------------------------------------------------------------------*/
-
-float64 float32_to_float64(float32 a, float_status *status)
-{
-    flag aSign;
-    int aExp;
-    uint32_t aSig;
-    a = float32_squash_input_denormal(a, status);
-
-    aSig = extractFloat32Frac( a );
-    aExp = extractFloat32Exp( a );
-    aSign = extractFloat32Sign( a );
-    if ( aExp == 0xFF ) {
-        if (aSig) {
-            return commonNaNToFloat64(float32ToCommonNaN(a, status), status);
-        }
-        return packFloat64( aSign, 0x7FF, 0 );
-    }
-    if ( aExp == 0 ) {
-        if ( aSig == 0 ) return packFloat64( aSign, 0, 0 );
-        normalizeFloat32Subnormal( aSig, &aExp, &aSig );
-        --aExp;
-    }
-    return packFloat64( aSign, aExp + 0x380, ( (uint64_t) aSig )<<29 );
-
-}
-
 /*----------------------------------------------------------------------------
 | Returns the result of converting the single-precision floating-point value
 | `a' to the extended double-precision floating-point format.  The conversion
@@ -3664,173 +3689,6 @@ int float32_unordered_quiet(float32 a, float32 b, float_status *status)
     return 0;
 }
 
-
-/*----------------------------------------------------------------------------
-| Returns the result of converting the double-precision floating-point value
-| `a' to the single-precision floating-point format.  The conversion is
-| performed according to the IEC/IEEE Standard for Binary Floating-Point
-| Arithmetic.
-*----------------------------------------------------------------------------*/
-
-float32 float64_to_float32(float64 a, float_status *status)
-{
-    flag aSign;
-    int aExp;
-    uint64_t aSig;
-    uint32_t zSig;
-    a = float64_squash_input_denormal(a, status);
-
-    aSig = extractFloat64Frac( a );
-    aExp = extractFloat64Exp( a );
-    aSign = extractFloat64Sign( a );
-    if ( aExp == 0x7FF ) {
-        if (aSig) {
-            return commonNaNToFloat32(float64ToCommonNaN(a, status), status);
-        }
-        return packFloat32( aSign, 0xFF, 0 );
-    }
-    shift64RightJamming( aSig, 22, &aSig );
-    zSig = aSig;
-    if ( aExp || zSig ) {
-        zSig |= 0x40000000;
-        aExp -= 0x381;
-    }
-    return roundAndPackFloat32(aSign, aExp, zSig, status);
-
-}
-
-
-/*----------------------------------------------------------------------------
-| Packs the sign `zSign', exponent `zExp', and significand `zSig' into a
-| half-precision floating-point value, returning the result.  After being
-| shifted into the proper positions, the three fields are simply added
-| together to form the result.  This means that any integer portion of `zSig'
-| will be added into the exponent.  Since a properly normalized significand
-| will have an integer portion equal to 1, the `zExp' input should be 1 less
-| than the desired result exponent whenever `zSig' is a complete, normalized
-| significand.
-*----------------------------------------------------------------------------*/
-static float16 packFloat16(flag zSign, int zExp, uint16_t zSig)
-{
-    return make_float16(
-        (((uint32_t)zSign) << 15) + (((uint32_t)zExp) << 10) + zSig);
-}
-
-/*----------------------------------------------------------------------------
-| Takes an abstract floating-point value having sign `zSign', exponent `zExp',
-| and significand `zSig', and returns the proper half-precision floating-
-| point value corresponding to the abstract input.  Ordinarily, the abstract
-| value is simply rounded and packed into the half-precision format, with
-| the inexact exception raised if the abstract input cannot be represented
-| exactly.  However, if the abstract value is too large, the overflow and
-| inexact exceptions are raised and an infinity or maximal finite value is
-| returned.  If the abstract value is too small, the input value is rounded to
-| a subnormal number, and the underflow and inexact exceptions are raised if
-| the abstract input cannot be represented exactly as a subnormal half-
-| precision floating-point number.
-| The `ieee' flag indicates whether to use IEEE standard half precision, or
-| ARM-style "alternative representation", which omits the NaN and Inf
-| encodings in order to raise the maximum representable exponent by one.
-|     The input significand `zSig' has its binary point between bits 22
-| and 23, which is 13 bits to the left of the usual location.  This shifted
-| significand must be normalized or smaller.  If `zSig' is not normalized,
-| `zExp' must be 0; in that case, the result returned is a subnormal number,
-| and it must not require rounding.  In the usual case that `zSig' is
-| normalized, `zExp' must be 1 less than the ``true'' floating-point exponent.
-| Note the slightly odd position of the binary point in zSig compared with the
-| other roundAndPackFloat functions. This should probably be fixed if we
-| need to implement more float16 routines than just conversion.
-| The handling of underflow and overflow follows the IEC/IEEE Standard for
-| Binary Floating-Point Arithmetic.
-*----------------------------------------------------------------------------*/
-
-static float16 roundAndPackFloat16(flag zSign, int zExp,
-                                   uint32_t zSig, flag ieee,
-                                   float_status *status)
-{
-    int maxexp = ieee ? 29 : 30;
-    uint32_t mask;
-    uint32_t increment;
-    bool rounding_bumps_exp;
-    bool is_tiny = false;
-
-    /* Calculate the mask of bits of the mantissa which are not
-     * representable in half-precision and will be lost.
-     */
-    if (zExp < 1) {
-        /* Will be denormal in halfprec */
-        mask = 0x00ffffff;
-        if (zExp >= -11) {
-            mask >>= 11 + zExp;
-        }
-    } else {
-        /* Normal number in halfprec */
-        mask = 0x00001fff;
-    }
-
-    switch (status->float_rounding_mode) {
-    case float_round_nearest_even:
-        increment = (mask + 1) >> 1;
-        if ((zSig & mask) == increment) {
-            increment = zSig & (increment << 1);
-        }
-        break;
-    case float_round_ties_away:
-        increment = (mask + 1) >> 1;
-        break;
-    case float_round_up:
-        increment = zSign ? 0 : mask;
-        break;
-    case float_round_down:
-        increment = zSign ? mask : 0;
-        break;
-    default: /* round_to_zero */
-        increment = 0;
-        break;
-    }
-
-    rounding_bumps_exp = (zSig + increment >= 0x01000000);
-
-    if (zExp > maxexp || (zExp == maxexp && rounding_bumps_exp)) {
-        if (ieee) {
-            float_raise(float_flag_overflow | float_flag_inexact, status);
-            return packFloat16(zSign, 0x1f, 0);
-        } else {
-            float_raise(float_flag_invalid, status);
-            return packFloat16(zSign, 0x1f, 0x3ff);
-        }
-    }
-
-    if (zExp < 0) {
-        /* Note that flush-to-zero does not affect half-precision results */
-        is_tiny =
-            (status->float_detect_tininess == float_tininess_before_rounding)
-            || (zExp < -1)
-            || (!rounding_bumps_exp);
-    }
-    if (zSig & mask) {
-        float_raise(float_flag_inexact, status);
-        if (is_tiny) {
-            float_raise(float_flag_underflow, status);
-        }
-    }
-
-    zSig += increment;
-    if (rounding_bumps_exp) {
-        zSig >>= 1;
-        zExp++;
-    }
-
-    if (zExp < -10) {
-        return packFloat16(zSign, 0, 0);
-    }
-    if (zExp < 0) {
-        zSig >>= -zExp;
-        zExp = 0;
-    }
-    return packFloat16(zSign, zExp, zSig >> 13);
-}
-
 /*----------------------------------------------------------------------------
 | If `a' is denormal and we are in flush-to-zero mode then set the
 | input-denormal exception and return zero. Otherwise just return the value.
@@ -3846,163 +3704,6 @@ float16 float16_squash_input_denormal(float16 a, float_status *status)
     return a;
 }
 
-static void normalizeFloat16Subnormal(uint32_t aSig, int *zExpPtr,
-                                      uint32_t *zSigPtr)
-{
-    int8_t shiftCount = countLeadingZeros32(aSig) - 21;
-    *zSigPtr = aSig << shiftCount;
-    *zExpPtr = 1 - shiftCount;
-}
-
-/* Half precision floats come in two formats: standard IEEE and "ARM" format.
-   The latter gains extra exponent range by omitting the NaN/Inf encodings.  */
-
-float32 float16_to_float32(float16 a, flag ieee, float_status *status)
-{
-    flag aSign;
-    int aExp;
-    uint32_t aSig;
-
-    aSign = extractFloat16Sign(a);
-    aExp = extractFloat16Exp(a);
-    aSig = extractFloat16Frac(a);
-
-    if (aExp == 0x1f && ieee) {
-        if (aSig) {
-            return commonNaNToFloat32(float16ToCommonNaN(a, status), status);
-        }
-        return packFloat32(aSign, 0xff, 0);
-    }
-    if (aExp == 0) {
-        if (aSig == 0) {
-            return packFloat32(aSign, 0, 0);
-        }
-
-        normalizeFloat16Subnormal(aSig, &aExp, &aSig);
-        aExp--;
-    }
-    return packFloat32( aSign, aExp + 0x70, aSig << 13);
-}
-
-float16 float32_to_float16(float32 a, flag ieee, float_status *status)
-{
-    flag aSign;
-    int aExp;
-    uint32_t aSig;
-
-    a = float32_squash_input_denormal(a, status);
-
-    aSig = extractFloat32Frac( a );
-    aExp = extractFloat32Exp( a );
-    aSign = extractFloat32Sign( a );
-    if ( aExp == 0xFF ) {
-        if (aSig) {
-            /* Input is a NaN */
-            if (!ieee) {
-                float_raise(float_flag_invalid, status);
-                return packFloat16(aSign, 0, 0);
-            }
-            return commonNaNToFloat16(
-                float32ToCommonNaN(a, status), status);
-        }
-        /* Infinity */
-        if (!ieee) {
-            float_raise(float_flag_invalid, status);
-            return packFloat16(aSign, 0x1f, 0x3ff);
-        }
-        return packFloat16(aSign, 0x1f, 0);
-    }
-    if (aExp == 0 && aSig == 0) {
-        return packFloat16(aSign, 0, 0);
-    }
-    /* Decimal point between bits 22 and 23. Note that we add the 1 bit
-     * even if the input is denormal; however this is harmless because
-     * the largest possible single-precision denormal is still smaller
-     * than the smallest representable half-precision denormal, and so we
-     * will end up ignoring aSig and returning via the "always return zero"
-     * codepath.
-     */
-    aSig |= 0x00800000;
-    aExp -= 0x71;
-
-    return roundAndPackFloat16(aSign, aExp, aSig, ieee, status);
-}
-
-float64 float16_to_float64(float16 a, flag ieee, float_status *status)
-{
-    flag aSign;
-    int aExp;
-    uint32_t aSig;
-
-    aSign = extractFloat16Sign(a);
-    aExp = extractFloat16Exp(a);
-    aSig = extractFloat16Frac(a);
-
-    if (aExp == 0x1f && ieee) {
-        if (aSig) {
-            return commonNaNToFloat64(
-                float16ToCommonNaN(a, status), status);
-        }
-        return packFloat64(aSign, 0x7ff, 0);
-    }
-    if (aExp == 0) {
-        if (aSig == 0) {
-            return packFloat64(aSign, 0, 0);
-        }
-
-        normalizeFloat16Subnormal(aSig, &aExp, &aSig);
-        aExp--;
-    }
-    return packFloat64(aSign, aExp + 0x3f0, ((uint64_t)aSig) << 42);
-}
-
-float16 float64_to_float16(float64 a, flag ieee, float_status *status)
-{
-    flag aSign;
-    int aExp;
-    uint64_t aSig;
-    uint32_t zSig;
-
-    a = float64_squash_input_denormal(a, status);
-
-    aSig = extractFloat64Frac(a);
-    aExp = extractFloat64Exp(a);
-    aSign = extractFloat64Sign(a);
-    if (aExp == 0x7FF) {
-        if (aSig) {
-            /* Input is a NaN */
-            if (!ieee) {
-                float_raise(float_flag_invalid, status);
-                return packFloat16(aSign, 0, 0);
-            }
-            return commonNaNToFloat16(
-                float64ToCommonNaN(a, status), status);
-        }
-        /* Infinity */
-        if (!ieee) {
-            float_raise(float_flag_invalid, status);
-            return packFloat16(aSign, 0x1f, 0x3ff);
-        }
-        return packFloat16(aSign, 0x1f, 0);
-    }
-    shift64RightJamming(aSig, 29, &aSig);
-    zSig = aSig;
-    if (aExp == 0 && zSig == 0) {
-        return packFloat16(aSign, 0, 0);
-    }
-    /* Decimal point between bits 22 and 23. Note that we add the 1 bit
-     * even if the input is denormal; however this is harmless because
-     * the largest possible single-precision denormal is still smaller
-     * than the smallest representable half-precision denormal, and so we
-     * will end up ignoring aSig and returning via the "always return zero"
-     * codepath.
-     */
-    zSig |= 0x00800000;
-    aExp -= 0x3F1;
-
-    return roundAndPackFloat16(aSign, aExp, zSig, ieee, status);
-}
-
 /*----------------------------------------------------------------------------
 | Returns the result of converting the double-precision floating-point value
 | `a' to the extended double-precision floating-point format.  The conversion
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 10/27] fpu/softfloat: re-factor float to float conversions

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
>
> This allows us to delete a lot of additional boilerplate code which is
> no longer needed. Currently the ieee flag is ignored (everything is
> assumed to be ieee). Handling for ARM AHP will be in the next patch.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> ---
> v2
>   - pass FloatFmt to float_to_float instead of sizes
>   - split AHP handling to another patch
>   - use rth's suggested re-packing (+ setting .exp)
> v3
>   - also rm extractFloat16Sign
> v4
>   - update for canonical nan handling
> ---
>  fpu/softfloat-specialize.h |  40 ----
>  include/fpu/softfloat.h    |   8 +-
>  fpu/softfloat.c            | 437 ++++++-------------------------------
>  3 files changed, 73 insertions(+), 412 deletions(-)

This is a regression if you put it at this point in the patch series,
because we don't yet have the patches which pass the fp status
and squash FZ16 in the arm helper wrappers before calling the softfloat
code. I think it needs to go after patches 13 and 14.

thanks
-- PMM

[Qemu-devel] [PATCH v2 11/27] fpu/softfloat: support ARM Alternative half-precision

[Richard Henderson]

From: Alex Bennée <alex.bennee@linaro.org>

For float16 ARM supports an alternative half-precision format which
sacrifices the ability to represent NaN/Inf in return for a higher
dynamic range. To support this I've added an additional
FloatFmt (float16_params_ahp).

The new FloatFmt flag (arm_althp) is then used to modify the behaviour
of canonicalize and round_canonical with respect to representation and
exception raising.

Finally the float16_to_floatN and floatN_to_float16 conversion
routines select the new alternative FloatFmt when !ieee.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
v3
  - squash NaN to 0 if destination is AHP F16
v4
  - handle inf -> ahp max in float_to_float not round_canonical
  - assert no nan and inf for ahp in round_canonical
  - check ahp before snan in float_to_float
---
 fpu/softfloat.c | 95 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 81 insertions(+), 14 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index aa219223ff..15a272759d 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -211,8 +211,10 @@ typedef struct {
  *   frac_shift: shift to normalise the fraction with DECOMPOSED_BINARY_POINT
  * The following are computed based the size of fraction
  *   frac_lsb: least significant bit of fraction
- *   fram_lsbm1: the bit bellow the least significant bit (for rounding)
+ *   frac_lsbm1: the bit bellow the least significant bit (for rounding)
  *   round_mask/roundeven_mask: masks used for rounding
+ * The following optional modifiers are available:
+ *   arm_althp: handle ARM Alternative Half Precision
  */
 typedef struct {
     int exp_size;
@@ -224,6 +226,7 @@ typedef struct {
     uint64_t frac_lsbm1;
     uint64_t round_mask;
     uint64_t roundeven_mask;
+    bool arm_althp;
 } FloatFmt;
 
 /*----------------------------------------------------------------------------
@@ -252,6 +255,11 @@ static const FloatFmt float16_params = {
     FLOAT_PARAMS(5, 10)
 };
 
+static const FloatFmt float16_params_ahp = {
+    FLOAT_PARAMS(5, 10),
+    .arm_althp = true
+};
+
 static const FloatFmt float32_params = {
     FLOAT_PARAMS(8, 23)
 };
@@ -315,7 +323,7 @@ static inline float64 float64_pack_raw(FloatParts p)
 static FloatParts canonicalize(FloatParts part, const FloatFmt *parm,
                                float_status *status)
 {
-    if (part.exp == parm->exp_max) {
+    if (part.exp == parm->exp_max && !parm->arm_althp) {
         if (part.frac == 0) {
             part.cls = float_class_inf;
         } else {
@@ -404,7 +412,15 @@ static FloatParts round_canonical(FloatParts p, float_status *s,
             }
             frac >>= frac_shift;
 
-            if (unlikely(exp >= exp_max)) {
+            if (parm->arm_althp) {
+                /* ARM Alt HP eschews Inf and NaN for a wider exponent.  */
+                if (unlikely(exp > exp_max)) {
+                    /* Overflow.  Return the maximum normal.  */
+                    flags = float_flag_invalid;
+                    exp = exp_max;
+                    frac = -1;
+                }
+            } else if (unlikely(exp >= exp_max)) {
                 flags |= float_flag_overflow | float_flag_inexact;
                 if (overflow_norm) {
                     exp = exp_max - 1;
@@ -455,12 +471,14 @@ static FloatParts round_canonical(FloatParts p, float_status *s,
 
     case float_class_inf:
     do_inf:
+        assert(!parm->arm_althp);
         exp = exp_max;
         frac = 0;
         break;
 
     case float_class_qnan:
     case float_class_snan:
+        assert(!parm->arm_althp);
         exp = exp_max;
         frac >>= parm->frac_shift;
         break;
@@ -475,14 +493,27 @@ static FloatParts round_canonical(FloatParts p, float_status *s,
     return p;
 }
 
+/* Explicit FloatFmt version */
+static FloatParts float16a_unpack_canonical(float16 f, float_status *s,
+                                            const FloatFmt *params)
+{
+    return canonicalize(float16_unpack_raw(f), params, s);
+}
+
 static FloatParts float16_unpack_canonical(float16 f, float_status *s)
 {
-    return canonicalize(float16_unpack_raw(f), &float16_params, s);
+    return float16a_unpack_canonical(f, s, &float16_params);
+}
+
+static float16 float16a_round_pack_canonical(FloatParts p, float_status *s,
+                                             const FloatFmt *params)
+{
+    return float16_pack_raw(round_canonical(p, s, params));
 }
 
 static float16 float16_round_pack_canonical(FloatParts p, float_status *s)
 {
-    return float16_pack_raw(round_canonical(p, s, &float16_params));
+    return float16a_round_pack_canonical(p, s, &float16_params);
 }
 
 static FloatParts float32_unpack_canonical(float32 f, float_status *s)
@@ -1174,7 +1205,33 @@ static FloatParts float_to_float(FloatParts a,
                                  const FloatFmt *srcf, const FloatFmt *dstf,
                                  float_status *s)
 {
-    if (is_nan(a.cls)) {
+    if (dstf->arm_althp) {
+        switch (a.cls) {
+        case float_class_qnan:
+        case float_class_snan:
+            /* There is no NaN in the destination format.  Raise Invalid
+             * and return a zero with the sign of the input NaN.
+             */
+            s->float_exception_flags |= float_flag_invalid;
+            a.cls = float_class_zero;
+            a.frac = 0;
+            a.exp = 0;
+            break;
+
+        case float_class_inf:
+            /* There is no Inf in the destination format.  Raise Invalid
+             * and return the maximum normal with the correct sign.
+             */
+            s->float_exception_flags |= float_flag_invalid;
+            a.cls = float_class_normal;
+            a.exp = dstf->exp_max;
+            a.frac = ((1ull << dstf->frac_size) - 1) << dstf->frac_shift;
+            break;
+
+        default:
+            break;
+        }
+    } else if (is_nan(a.cls)) {
         if (is_snan(a.cls)) {
             s->float_exception_flags |= float_flag_invalid;
             a = parts_silence_nan(a, s);
@@ -1186,25 +1243,34 @@ static FloatParts float_to_float(FloatParts a,
     return a;
 }
 
+/*
+ * Currently non-ieee implies ARM Alternative Half Precision handling
+ * for float16 values. If more are needed we'll need to expand the API
+ * into softfloat.
+ */
+
 float32 float16_to_float32(float16 a, bool ieee, float_status *s)
 {
-    FloatParts p = float16_unpack_canonical(a, s);
-    FloatParts pr = float_to_float(p, &float16_params, &float32_params, s);
+    const FloatFmt *fmt16 = ieee ? &float16_params : &float16_params_ahp;
+    FloatParts p = float16a_unpack_canonical(a, s, fmt16);
+    FloatParts pr = float_to_float(p, fmt16, &float32_params, s);
     return float32_round_pack_canonical(pr, s);
 }
 
 float64 float16_to_float64(float16 a, bool ieee, float_status *s)
 {
-    FloatParts p = float16_unpack_canonical(a, s);
-    FloatParts pr = float_to_float(p, &float16_params, &float64_params, s);
+    const FloatFmt *fmt16 = ieee ? &float16_params : &float16_params_ahp;
+    FloatParts p = float16a_unpack_canonical(a, s, fmt16);
+    FloatParts pr = float_to_float(p, fmt16, &float64_params, s);
     return float64_round_pack_canonical(pr, s);
 }
 
 float16 float32_to_float16(float32 a, bool ieee, float_status *s)
 {
+    const FloatFmt *fmt16 = ieee ? &float16_params : &float16_params_ahp;
     FloatParts p = float32_unpack_canonical(a, s);
-    FloatParts pr = float_to_float(p, &float32_params, &float16_params, s);
-    return float16_round_pack_canonical(pr, s);
+    FloatParts pr = float_to_float(p, &float32_params, fmt16, s);
+    return float16a_round_pack_canonical(pr, s, fmt16);
 }
 
 float64 float32_to_float64(float32 a, float_status *s)
@@ -1216,9 +1282,10 @@ float64 float32_to_float64(float32 a, float_status *s)
 
 float16 float64_to_float16(float64 a, bool ieee, float_status *s)
 {
+    const FloatFmt *fmt16 = ieee ? &float16_params : &float16_params_ahp;
     FloatParts p = float64_unpack_canonical(a, s);
-    FloatParts pr = float_to_float(p, &float64_params, &float16_params, s);
-    return float16_round_pack_canonical(pr, s);
+    FloatParts pr = float_to_float(p, &float64_params, fmt16, s);
+    return float16a_round_pack_canonical(pr, s, fmt16);
 }
 
 float32 float64_to_float32(float64 a, float_status *s)
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 11/27] fpu/softfloat: support ARM Alternative half-precision

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
>
> For float16 ARM supports an alternative half-precision format which
> sacrifices the ability to represent NaN/Inf in return for a higher
> dynamic range. To support this I've added an additional
> FloatFmt (float16_params_ahp).
>
> The new FloatFmt flag (arm_althp) is then used to modify the behaviour
> of canonicalize and round_canonical with respect to representation and
> exception raising.
>
> Finally the float16_to_floatN and floatN_to_float16 conversion
> routines select the new alternative FloatFmt when !ieee.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

If we do this like this then we have a regression for the
alternate-halfprec conversions between the patch where we change
over the conversions functions and this one.

To fix that we could put the parts of this patch that change
existing-in-master code like round_canonical() first, and then
fold the parts that fix up the conversion functions into the
patch which is currently 10/27.

> ---
> v3
>   - squash NaN to 0 if destination is AHP F16
> v4
>   - handle inf -> ahp max in float_to_float not round_canonical
>   - assert no nan and inf for ahp in round_canonical
>   - check ahp before snan in float_to_float
> ---
>  fpu/softfloat.c | 95 +++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index aa219223ff..15a272759d 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -211,8 +211,10 @@ typedef struct {
>   *   frac_shift: shift to normalise the fraction with DECOMPOSED_BINARY_POINT
>   * The following are computed based the size of fraction
>   *   frac_lsb: least significant bit of fraction
> - *   fram_lsbm1: the bit bellow the least significant bit (for rounding)
> + *   frac_lsbm1: the bit bellow the least significant bit (for rounding)

"below" (I see this typo slipped past us the first time around)

thanks
-- PMM

[Qemu-devel] [PATCH v2 12/27] target/arm: Use floatX_silence_nan when we have already checked for SNaN

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper-a64.c |  6 +++---
 target/arm/helper.c     | 12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index 4f8034c513..6f0eb83661 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -376,7 +376,7 @@ float16 HELPER(frecpx_f16)(float16 a, void *fpstp)
         float16 nan = a;
         if (float16_is_signaling_nan(a, fpst)) {
             float_raise(float_flag_invalid, fpst);
-            nan = float16_maybe_silence_nan(a, fpst);
+            nan = float16_silence_nan(a, fpst);
         }
         if (fpst->default_nan_mode) {
             nan = float16_default_nan(fpst);
@@ -405,7 +405,7 @@ float32 HELPER(frecpx_f32)(float32 a, void *fpstp)
         float32 nan = a;
         if (float32_is_signaling_nan(a, fpst)) {
             float_raise(float_flag_invalid, fpst);
-            nan = float32_maybe_silence_nan(a, fpst);
+            nan = float32_silence_nan(a, fpst);
         }
         if (fpst->default_nan_mode) {
             nan = float32_default_nan(fpst);
@@ -434,7 +434,7 @@ float64 HELPER(frecpx_f64)(float64 a, void *fpstp)
         float64 nan = a;
         if (float64_is_signaling_nan(a, fpst)) {
             float_raise(float_flag_invalid, fpst);
-            nan = float64_maybe_silence_nan(a, fpst);
+            nan = float64_silence_nan(a, fpst);
         }
         if (fpst->default_nan_mode) {
             nan = float64_default_nan(fpst);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index c6fd7f9479..3df5cf30e4 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11739,7 +11739,7 @@ float16 HELPER(recpe_f16)(float16 input, void *fpstp)
         float16 nan = f16;
         if (float16_is_signaling_nan(f16, fpst)) {
             float_raise(float_flag_invalid, fpst);
-            nan = float16_maybe_silence_nan(f16, fpst);
+            nan = float16_silence_nan(f16, fpst);
         }
         if (fpst->default_nan_mode) {
             nan =  float16_default_nan(fpst);
@@ -11787,7 +11787,7 @@ float32 HELPER(recpe_f32)(float32 input, void *fpstp)
         float32 nan = f32;
         if (float32_is_signaling_nan(f32, fpst)) {
             float_raise(float_flag_invalid, fpst);
-            nan = float32_maybe_silence_nan(f32, fpst);
+            nan = float32_silence_nan(f32, fpst);
         }
         if (fpst->default_nan_mode) {
             nan =  float32_default_nan(fpst);
@@ -11835,7 +11835,7 @@ float64 HELPER(recpe_f64)(float64 input, void *fpstp)
         float64 nan = f64;
         if (float64_is_signaling_nan(f64, fpst)) {
             float_raise(float_flag_invalid, fpst);
-            nan = float64_maybe_silence_nan(f64, fpst);
+            nan = float64_silence_nan(f64, fpst);
         }
         if (fpst->default_nan_mode) {
             nan =  float64_default_nan(fpst);
@@ -11934,7 +11934,7 @@ float16 HELPER(rsqrte_f16)(float16 input, void *fpstp)
         float16 nan = f16;
         if (float16_is_signaling_nan(f16, s)) {
             float_raise(float_flag_invalid, s);
-            nan = float16_maybe_silence_nan(f16, s);
+            nan = float16_silence_nan(f16, s);
         }
         if (s->default_nan_mode) {
             nan =  float16_default_nan(s);
@@ -11978,7 +11978,7 @@ float32 HELPER(rsqrte_f32)(float32 input, void *fpstp)
         float32 nan = f32;
         if (float32_is_signaling_nan(f32, s)) {
             float_raise(float_flag_invalid, s);
-            nan = float32_maybe_silence_nan(f32, s);
+            nan = float32_silence_nan(f32, s);
         }
         if (s->default_nan_mode) {
             nan =  float32_default_nan(s);
@@ -12021,7 +12021,7 @@ float64 HELPER(rsqrte_f64)(float64 input, void *fpstp)
         float64 nan = f64;
         if (float64_is_signaling_nan(f64, s)) {
             float_raise(float_flag_invalid, s);
-            nan = float64_maybe_silence_nan(f64, s);
+            nan = float64_silence_nan(f64, s);
         }
         if (s->default_nan_mode) {
             nan =  float64_default_nan(s);
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 12/27] target/arm: Use floatX_silence_nan when we have already checked for SNaN

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-a64.c |  6 +++---
>  target/arm/helper.c     | 12 ++++++------
>  2 files changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 13/27] target/arm: convert conversion helpers to fpst/ahp_flag

[Richard Henderson]

From: Alex Bennée <alex.bennee@linaro.org>

Instead of passing env and leaving it up to the helper to get the
right fpstatus we pass it explicitly. There was already a get_fpstatus
helper for neon for the 32 bit code. We also add an get_ahp_flag() for
passing the state of the alternative FP16 format flag. This leaves
scope for later tracking the AHP state in translation flags.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
v4
  - remove neon_fcvt_*; they are now identical to vfp_fcvt_*.
  - add flags to vfp_fcvt_* helper decls.
  - add some missing tcg_temp_free_*.
---
 target/arm/helper.h        | 10 +++---
 target/arm/translate.h     | 12 +++++++
 target/arm/helper.c        | 56 +++++------------------------
 target/arm/translate-a64.c | 38 ++++++++++++++++----
 target/arm/translate.c     | 74 +++++++++++++++++++++++++++++---------
 5 files changed, 113 insertions(+), 77 deletions(-)

diff --git a/target/arm/helper.h b/target/arm/helper.h
index ce89968b2d..047f3bc1ca 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -187,12 +187,10 @@ DEF_HELPER_3(vfp_uqtoh, f16, i64, i32, ptr)
 DEF_HELPER_FLAGS_2(set_rmode, TCG_CALL_NO_RWG, i32, i32, ptr)
 DEF_HELPER_FLAGS_2(set_neon_rmode, TCG_CALL_NO_RWG, i32, i32, env)
 
-DEF_HELPER_2(vfp_fcvt_f16_to_f32, f32, i32, env)
-DEF_HELPER_2(vfp_fcvt_f32_to_f16, i32, f32, env)
-DEF_HELPER_2(neon_fcvt_f16_to_f32, f32, i32, env)
-DEF_HELPER_2(neon_fcvt_f32_to_f16, i32, f32, env)
-DEF_HELPER_FLAGS_2(vfp_fcvt_f16_to_f64, TCG_CALL_NO_RWG, f64, i32, env)
-DEF_HELPER_FLAGS_2(vfp_fcvt_f64_to_f16, TCG_CALL_NO_RWG, i32, f64, env)
+DEF_HELPER_FLAGS_3(vfp_fcvt_f16_to_f32, TCG_CALL_NO_RWG, f32, f16, ptr, i32)
+DEF_HELPER_FLAGS_3(vfp_fcvt_f32_to_f16, TCG_CALL_NO_RWG, f16, f32, ptr, i32)
+DEF_HELPER_FLAGS_3(vfp_fcvt_f16_to_f64, TCG_CALL_NO_RWG, f64, f16, ptr, i32)
+DEF_HELPER_FLAGS_3(vfp_fcvt_f64_to_f16, TCG_CALL_NO_RWG, f16, f64, ptr, i32)
 
 DEF_HELPER_4(vfp_muladdd, f64, f64, f64, f64, ptr)
 DEF_HELPER_4(vfp_muladds, f32, f32, f32, f32, ptr)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 37a1bba056..45f04244be 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -177,4 +177,16 @@ void arm_free_cc(DisasCompare *cmp);
 void arm_jump_cc(DisasCompare *cmp, TCGLabel *label);
 void arm_gen_test_cc(int cc, TCGLabel *label);
 
+/* Return state of Alternate Half-precision flag, caller frees result */
+static inline TCGv_i32 get_ahp_flag(void)
+{
+    TCGv_i32 ret = tcg_temp_new_i32();
+
+    tcg_gen_ld_i32(ret, cpu_env,
+                   offsetof(CPUARMState, vfp.xregs[ARM_VFP_FPSCR]));
+    tcg_gen_extract_i32(ret, ret, 26, 1);
+
+    return ret;
+}
+
 #endif /* TARGET_ARM_TRANSLATE_H */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3df5cf30e4..a1c1dc5bbe 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11540,64 +11540,24 @@ uint32_t HELPER(set_neon_rmode)(uint32_t rmode, CPUARMState *env)
 }
 
 /* Half precision conversions.  */
-static float32 do_fcvt_f16_to_f32(uint32_t a, CPUARMState *env, float_status *s)
+float32 HELPER(vfp_fcvt_f16_to_f32)(float16 a, void *fpstp, uint32_t ahp_mode)
 {
-    int ieee = (env->vfp.xregs[ARM_VFP_FPSCR] & (1 << 26)) == 0;
-    float32 r = float16_to_float32(make_float16(a), ieee, s);
-    if (ieee) {
-        return float32_maybe_silence_nan(r, s);
-    }
-    return r;
+    return float16_to_float32(a, !ahp_mode, fpstp);
 }
 
-static uint32_t do_fcvt_f32_to_f16(float32 a, CPUARMState *env, float_status *s)
+float16 HELPER(vfp_fcvt_f32_to_f16)(float32 a, void *fpstp, uint32_t ahp_mode)
 {
-    int ieee = (env->vfp.xregs[ARM_VFP_FPSCR] & (1 << 26)) == 0;
-    float16 r = float32_to_float16(a, ieee, s);
-    if (ieee) {
-        r = float16_maybe_silence_nan(r, s);
-    }
-    return float16_val(r);
+    return float32_to_float16(a, !ahp_mode, fpstp);
 }
 
-float32 HELPER(neon_fcvt_f16_to_f32)(uint32_t a, CPUARMState *env)
+float64 HELPER(vfp_fcvt_f16_to_f64)(float16 a, void *fpstp, uint32_t ahp_mode)
 {
-    return do_fcvt_f16_to_f32(a, env, &env->vfp.standard_fp_status);
+    return float16_to_float64(a, !ahp_mode, fpstp);
 }
 
-uint32_t HELPER(neon_fcvt_f32_to_f16)(float32 a, CPUARMState *env)
+float16 HELPER(vfp_fcvt_f64_to_f16)(float64 a, void *fpstp, uint32_t ahp_mode)
 {
-    return do_fcvt_f32_to_f16(a, env, &env->vfp.standard_fp_status);
-}
-
-float32 HELPER(vfp_fcvt_f16_to_f32)(uint32_t a, CPUARMState *env)
-{
-    return do_fcvt_f16_to_f32(a, env, &env->vfp.fp_status);
-}
-
-uint32_t HELPER(vfp_fcvt_f32_to_f16)(float32 a, CPUARMState *env)
-{
-    return do_fcvt_f32_to_f16(a, env, &env->vfp.fp_status);
-}
-
-float64 HELPER(vfp_fcvt_f16_to_f64)(uint32_t a, CPUARMState *env)
-{
-    int ieee = (env->vfp.xregs[ARM_VFP_FPSCR] & (1 << 26)) == 0;
-    float64 r = float16_to_float64(make_float16(a), ieee, &env->vfp.fp_status);
-    if (ieee) {
-        return float64_maybe_silence_nan(r, &env->vfp.fp_status);
-    }
-    return r;
-}
-
-uint32_t HELPER(vfp_fcvt_f64_to_f16)(float64 a, CPUARMState *env)
-{
-    int ieee = (env->vfp.xregs[ARM_VFP_FPSCR] & (1 << 26)) == 0;
-    float16 r = float64_to_float16(a, ieee, &env->vfp.fp_status);
-    if (ieee) {
-        r = float16_maybe_silence_nan(r, &env->vfp.fp_status);
-    }
-    return float16_val(r);
+    return float64_to_float16(a, !ahp_mode, fpstp);
 }
 
 #define float32_two make_float32(0x40000000)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index a0b0c43d12..570791e7b5 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -5147,10 +5147,15 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
         } else {
             /* Single to half */
             TCGv_i32 tcg_rd = tcg_temp_new_i32();
-            gen_helper_vfp_fcvt_f32_to_f16(tcg_rd, tcg_rn, cpu_env);
+            TCGv_i32 ahp = get_ahp_flag();
+            TCGv_ptr fpst = get_fpstatus_ptr(true);
+
+            gen_helper_vfp_fcvt_f32_to_f16(tcg_rd, tcg_rn, fpst, ahp);
             /* write_fp_sreg is OK here because top half of tcg_rd is zero */
             write_fp_sreg(s, rd, tcg_rd);
             tcg_temp_free_i32(tcg_rd);
+            tcg_temp_free_i32(ahp);
+            tcg_temp_free_ptr(fpst);
         }
         tcg_temp_free_i32(tcg_rn);
         break;
@@ -5163,9 +5168,13 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
             /* Double to single */
             gen_helper_vfp_fcvtsd(tcg_rd, tcg_rn, cpu_env);
         } else {
+            TCGv_ptr fpst = get_fpstatus_ptr(true);
+            TCGv_i32 ahp = get_ahp_flag();
             /* Double to half */
-            gen_helper_vfp_fcvt_f64_to_f16(tcg_rd, tcg_rn, cpu_env);
+            gen_helper_vfp_fcvt_f64_to_f16(tcg_rd, tcg_rn, fpst, ahp);
             /* write_fp_sreg is OK here because top half of tcg_rd is zero */
+            tcg_temp_free_ptr(fpst);
+            tcg_temp_free_i32(ahp);
         }
         write_fp_sreg(s, rd, tcg_rd);
         tcg_temp_free_i32(tcg_rd);
@@ -5175,17 +5184,21 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
     case 0x3:
     {
         TCGv_i32 tcg_rn = read_fp_sreg(s, rn);
+        TCGv_ptr tcg_fpst = get_fpstatus_ptr(true);
+        TCGv_i32 tcg_ahp = get_ahp_flag();
         tcg_gen_ext16u_i32(tcg_rn, tcg_rn);
         if (dtype == 0) {
             /* Half to single */
             TCGv_i32 tcg_rd = tcg_temp_new_i32();
-            gen_helper_vfp_fcvt_f16_to_f32(tcg_rd, tcg_rn, cpu_env);
+            gen_helper_vfp_fcvt_f16_to_f32(tcg_rd, tcg_rn, tcg_fpst, tcg_ahp);
             write_fp_sreg(s, rd, tcg_rd);
+            tcg_temp_free_ptr(tcg_fpst);
+            tcg_temp_free_i32(tcg_ahp);
             tcg_temp_free_i32(tcg_rd);
         } else {
             /* Half to double */
             TCGv_i64 tcg_rd = tcg_temp_new_i64();
-            gen_helper_vfp_fcvt_f16_to_f64(tcg_rd, tcg_rn, cpu_env);
+            gen_helper_vfp_fcvt_f16_to_f64(tcg_rd, tcg_rn, tcg_fpst, tcg_ahp);
             write_fp_dreg(s, rd, tcg_rd);
             tcg_temp_free_i64(tcg_rd);
         }
@@ -9053,12 +9066,17 @@ static void handle_2misc_narrow(DisasContext *s, bool scalar,
             } else {
                 TCGv_i32 tcg_lo = tcg_temp_new_i32();
                 TCGv_i32 tcg_hi = tcg_temp_new_i32();
+                TCGv_ptr fpst = get_fpstatus_ptr(true);
+                TCGv_i32 ahp = get_ahp_flag();
+
                 tcg_gen_extr_i64_i32(tcg_lo, tcg_hi, tcg_op);
-                gen_helper_vfp_fcvt_f32_to_f16(tcg_lo, tcg_lo, cpu_env);
-                gen_helper_vfp_fcvt_f32_to_f16(tcg_hi, tcg_hi, cpu_env);
+                gen_helper_vfp_fcvt_f32_to_f16(tcg_lo, tcg_lo, fpst, ahp);
+                gen_helper_vfp_fcvt_f32_to_f16(tcg_hi, tcg_hi, fpst, ahp);
                 tcg_gen_deposit_i32(tcg_res[pass], tcg_lo, tcg_hi, 16, 16);
                 tcg_temp_free_i32(tcg_lo);
                 tcg_temp_free_i32(tcg_hi);
+                tcg_temp_free_ptr(fpst);
+                tcg_temp_free_i32(ahp);
             }
             break;
         case 0x56:  /* FCVTXN, FCVTXN2 */
@@ -11532,18 +11550,24 @@ static void handle_2misc_widening(DisasContext *s, int opcode, bool is_q,
         /* 16 -> 32 bit fp conversion */
         int srcelt = is_q ? 4 : 0;
         TCGv_i32 tcg_res[4];
+        TCGv_ptr fpst = get_fpstatus_ptr(true);
+        TCGv_i32 ahp = get_ahp_flag();
+
 
         for (pass = 0; pass < 4; pass++) {
             tcg_res[pass] = tcg_temp_new_i32();
 
             read_vec_element_i32(s, tcg_res[pass], rn, srcelt + pass, MO_16);
             gen_helper_vfp_fcvt_f16_to_f32(tcg_res[pass], tcg_res[pass],
-                                           cpu_env);
+                                           fpst, ahp);
         }
         for (pass = 0; pass < 4; pass++) {
             write_vec_element_i32(s, tcg_res[pass], rd, pass, MO_32);
             tcg_temp_free_i32(tcg_res[pass]);
         }
+
+        tcg_temp_free_ptr(fpst);
+        tcg_temp_free_i32(ahp);
     }
 }
 
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 731cf327a1..5ba59e0188 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -3824,38 +3824,56 @@ static int disas_vfp_insn(DisasContext *s, uint32_t insn)
                         gen_vfp_sqrt(dp);
                         break;
                     case 4: /* vcvtb.f32.f16, vcvtb.f64.f16 */
+                    {
+                        TCGv_ptr fpst = get_fpstatus_ptr(false);
+                        TCGv_i32 ahp_mode = get_ahp_flag();
                         tmp = gen_vfp_mrs();
                         tcg_gen_ext16u_i32(tmp, tmp);
                         if (dp) {
                             gen_helper_vfp_fcvt_f16_to_f64(cpu_F0d, tmp,
-                                                           cpu_env);
+                                                           fpst, ahp_mode);
                         } else {
                             gen_helper_vfp_fcvt_f16_to_f32(cpu_F0s, tmp,
-                                                           cpu_env);
+                                                           fpst, ahp_mode);
                         }
+                        tcg_temp_free_i32(ahp_mode);
+                        tcg_temp_free_ptr(fpst);
                         tcg_temp_free_i32(tmp);
                         break;
+                    }
                     case 5: /* vcvtt.f32.f16, vcvtt.f64.f16 */
+                    {
+                        TCGv_ptr fpst = get_fpstatus_ptr(false);
+                        TCGv_i32 ahp = get_ahp_flag();
                         tmp = gen_vfp_mrs();
                         tcg_gen_shri_i32(tmp, tmp, 16);
                         if (dp) {
                             gen_helper_vfp_fcvt_f16_to_f64(cpu_F0d, tmp,
-                                                           cpu_env);
+                                                           fpst, ahp);
                         } else {
                             gen_helper_vfp_fcvt_f16_to_f32(cpu_F0s, tmp,
-                                                           cpu_env);
+                                                           fpst, ahp);
                         }
                         tcg_temp_free_i32(tmp);
+                        tcg_temp_free_i32(ahp);
+                        tcg_temp_free_ptr(fpst);
                         break;
+                    }
                     case 6: /* vcvtb.f16.f32, vcvtb.f16.f64 */
+                    {
+                        TCGv_ptr fpst = get_fpstatus_ptr(false);
+                        TCGv_i32 ahp = get_ahp_flag();
                         tmp = tcg_temp_new_i32();
+
                         if (dp) {
                             gen_helper_vfp_fcvt_f64_to_f16(tmp, cpu_F0d,
-                                                           cpu_env);
+                                                           fpst, ahp);
                         } else {
                             gen_helper_vfp_fcvt_f32_to_f16(tmp, cpu_F0s,
-                                                           cpu_env);
+                                                           fpst, ahp);
                         }
+                        tcg_temp_free_i32(ahp);
+                        tcg_temp_free_ptr(fpst);
                         gen_mov_F0_vreg(0, rd);
                         tmp2 = gen_vfp_mrs();
                         tcg_gen_andi_i32(tmp2, tmp2, 0xffff0000);
@@ -3863,15 +3881,21 @@ static int disas_vfp_insn(DisasContext *s, uint32_t insn)
                         tcg_temp_free_i32(tmp2);
                         gen_vfp_msr(tmp);
                         break;
+                    }
                     case 7: /* vcvtt.f16.f32, vcvtt.f16.f64 */
+                    {
+                        TCGv_ptr fpst = get_fpstatus_ptr(true);
+                        TCGv_i32 ahp = get_ahp_flag();
                         tmp = tcg_temp_new_i32();
                         if (dp) {
                             gen_helper_vfp_fcvt_f64_to_f16(tmp, cpu_F0d,
-                                                           cpu_env);
+                                                           fpst, ahp);
                         } else {
                             gen_helper_vfp_fcvt_f32_to_f16(tmp, cpu_F0s,
-                                                           cpu_env);
+                                                           fpst, ahp);
                         }
+                        tcg_temp_free_i32(ahp);
+                        tcg_temp_free_ptr(fpst);
                         tcg_gen_shli_i32(tmp, tmp, 16);
                         gen_mov_F0_vreg(0, rd);
                         tmp2 = gen_vfp_mrs();
@@ -3880,6 +3904,7 @@ static int disas_vfp_insn(DisasContext *s, uint32_t insn)
                         tcg_temp_free_i32(tmp2);
                         gen_vfp_msr(tmp);
                         break;
+                    }
                     case 8: /* cmp */
                         gen_vfp_cmp(dp);
                         break;
@@ -7222,53 +7247,70 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
                     }
                     break;
                 case NEON_2RM_VCVT_F16_F32:
+                {
+                    TCGv_ptr fpst;
+                    TCGv_i32 ahp;
+
                     if (!arm_dc_feature(s, ARM_FEATURE_VFP_FP16) ||
                         q || (rm & 1)) {
                         return 1;
                     }
                     tmp = tcg_temp_new_i32();
                     tmp2 = tcg_temp_new_i32();
+                    fpst = get_fpstatus_ptr(true);
+                    ahp = get_ahp_flag();
                     tcg_gen_ld_f32(cpu_F0s, cpu_env, neon_reg_offset(rm, 0));
-                    gen_helper_neon_fcvt_f32_to_f16(tmp, cpu_F0s, cpu_env);
+                    gen_helper_vfp_fcvt_f32_to_f16(tmp, cpu_F0s, fpst, ahp);
                     tcg_gen_ld_f32(cpu_F0s, cpu_env, neon_reg_offset(rm, 1));
-                    gen_helper_neon_fcvt_f32_to_f16(tmp2, cpu_F0s, cpu_env);
+                    gen_helper_vfp_fcvt_f32_to_f16(tmp2, cpu_F0s, fpst, ahp);
                     tcg_gen_shli_i32(tmp2, tmp2, 16);
                     tcg_gen_or_i32(tmp2, tmp2, tmp);
                     tcg_gen_ld_f32(cpu_F0s, cpu_env, neon_reg_offset(rm, 2));
-                    gen_helper_neon_fcvt_f32_to_f16(tmp, cpu_F0s, cpu_env);
+                    gen_helper_vfp_fcvt_f32_to_f16(tmp, cpu_F0s, fpst, ahp);
                     tcg_gen_ld_f32(cpu_F0s, cpu_env, neon_reg_offset(rm, 3));
                     neon_store_reg(rd, 0, tmp2);
                     tmp2 = tcg_temp_new_i32();
-                    gen_helper_neon_fcvt_f32_to_f16(tmp2, cpu_F0s, cpu_env);
+                    gen_helper_vfp_fcvt_f32_to_f16(tmp2, cpu_F0s, fpst, ahp);
                     tcg_gen_shli_i32(tmp2, tmp2, 16);
                     tcg_gen_or_i32(tmp2, tmp2, tmp);
                     neon_store_reg(rd, 1, tmp2);
                     tcg_temp_free_i32(tmp);
+                    tcg_temp_free_i32(ahp);
+                    tcg_temp_free_ptr(fpst);
                     break;
+                }
                 case NEON_2RM_VCVT_F32_F16:
+                {
+                    TCGv_ptr fpst;
+                    TCGv_i32 ahp;
                     if (!arm_dc_feature(s, ARM_FEATURE_VFP_FP16) ||
                         q || (rd & 1)) {
                         return 1;
                     }
+                    fpst = get_fpstatus_ptr(true);
+                    ahp = get_ahp_flag();
                     tmp3 = tcg_temp_new_i32();
                     tmp = neon_load_reg(rm, 0);
                     tmp2 = neon_load_reg(rm, 1);
                     tcg_gen_ext16u_i32(tmp3, tmp);
-                    gen_helper_neon_fcvt_f16_to_f32(cpu_F0s, tmp3, cpu_env);
+                    gen_helper_vfp_fcvt_f16_to_f32(cpu_F0s, tmp3, fpst, ahp);
                     tcg_gen_st_f32(cpu_F0s, cpu_env, neon_reg_offset(rd, 0));
                     tcg_gen_shri_i32(tmp3, tmp, 16);
-                    gen_helper_neon_fcvt_f16_to_f32(cpu_F0s, tmp3, cpu_env);
+                    gen_helper_vfp_fcvt_f16_to_f32(cpu_F0s, tmp3, fpst, ahp);
                     tcg_gen_st_f32(cpu_F0s, cpu_env, neon_reg_offset(rd, 1));
                     tcg_temp_free_i32(tmp);
                     tcg_gen_ext16u_i32(tmp3, tmp2);
-                    gen_helper_neon_fcvt_f16_to_f32(cpu_F0s, tmp3, cpu_env);
+                    gen_helper_vfp_fcvt_f16_to_f32(cpu_F0s, tmp3, fpst, ahp);
                     tcg_gen_st_f32(cpu_F0s, cpu_env, neon_reg_offset(rd, 2));
                     tcg_gen_shri_i32(tmp3, tmp2, 16);
-                    gen_helper_neon_fcvt_f16_to_f32(cpu_F0s, tmp3, cpu_env);
+                    gen_helper_vfp_fcvt_f16_to_f32(cpu_F0s, tmp3, fpst, ahp);
                     tcg_gen_st_f32(cpu_F0s, cpu_env, neon_reg_offset(rd, 3));
                     tcg_temp_free_i32(tmp2);
                     tcg_temp_free_i32(tmp3);
+                    tcg_temp_free_i32(ahp);
+                    tcg_temp_free_ptr(fpst);
                     break;
+                }
                 case NEON_2RM_AESE: case NEON_2RM_AESMC:
                     if (!arm_dc_feature(s, ARM_FEATURE_V8_AES)
                         || ((rm | rd) & 1)) {
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 13/27] target/arm: convert conversion helpers to fpst/ahp_flag

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
>
> Instead of passing env and leaving it up to the helper to get the
> right fpstatus we pass it explicitly. There was already a get_fpstatus
> helper for neon for the 32 bit code. We also add an get_ahp_flag() for
> passing the state of the alternative FP16 format flag. This leaves
> scope for later tracking the AHP state in translation flags.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index a0b0c43d12..570791e7b5 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -5147,10 +5147,15 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
>          } else {
>              /* Single to half */
>              TCGv_i32 tcg_rd = tcg_temp_new_i32();
> -            gen_helper_vfp_fcvt_f32_to_f16(tcg_rd, tcg_rn, cpu_env);
> +            TCGv_i32 ahp = get_ahp_flag();
> +            TCGv_ptr fpst = get_fpstatus_ptr(true);

This should be passing in 'false' to get_fpstatus_ptr(), as should
all the other uses where we get an fpst to pass to an f*_to_f16 or
f16_to_f* conversion helper. This is because conversions between
f16 and other sizes ignore FPCR.FZ16 for the 16-bit end, but they
still honour FPCR.FZ for the other end. So you need to pass in the
fpstatus pointer which has the flush-denormals status flags set
to match FPCR.FZ, not the fp16-specific one.

Otherwise you'll generate the wrong exception flags for cases like
 FPCR.FZ = 1; FPCR.FZ16 = 0; s0 = 0x00000001; fcvt h1, s0
(which should give h1 = 0x0000 and set InputDenormal).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 13/27] target/arm: convert conversion helpers to fpst/ahp_flag

[Richard Henderson]

On 05/14/2018 06:41 AM, Peter Maydell wrote:
> On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
>> From: Alex Bennée <alex.bennee@linaro.org>
>>
>> Instead of passing env and leaving it up to the helper to get the
>> right fpstatus we pass it explicitly. There was already a get_fpstatus
>> helper for neon for the 32 bit code. We also add an get_ahp_flag() for
>> passing the state of the alternative FP16 format flag. This leaves
>> scope for later tracking the AHP state in translation flags.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
>> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
>> index a0b0c43d12..570791e7b5 100644
>> --- a/target/arm/translate-a64.c
>> +++ b/target/arm/translate-a64.c
>> @@ -5147,10 +5147,15 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
>>          } else {
>>              /* Single to half */
>>              TCGv_i32 tcg_rd = tcg_temp_new_i32();
>> -            gen_helper_vfp_fcvt_f32_to_f16(tcg_rd, tcg_rn, cpu_env);
>> +            TCGv_i32 ahp = get_ahp_flag();
>> +            TCGv_ptr fpst = get_fpstatus_ptr(true);
> 
> This should be passing in 'false' to get_fpstatus_ptr(), as should
> all the other uses where we get an fpst to pass to an f*_to_f16 or
> f16_to_f* conversion helper. This is because conversions between
> f16 and other sizes ignore FPCR.FZ16 for the 16-bit end, but they
> still honour FPCR.FZ for the other end. So you need to pass in the
> fpstatus pointer which has the flush-denormals status flags set
> to match FPCR.FZ, not the fp16-specific one.
> 
> Otherwise you'll generate the wrong exception flags for cases like
>  FPCR.FZ = 1; FPCR.FZ16 = 0; s0 = 0x00000001; fcvt h1, s0
> (which should give h1 = 0x0000 and set InputDenormal).

Good catch, thanks.


r~

[Qemu-devel] [PATCH v2 14/27] target/arm: squash FZ16 behaviour for conversions

[Richard Henderson]

From: Alex Bennée <alex.bennee@linaro.org>

The ARM ARM specifies FZ16 is suppressed for conversions. Rather than
pushing this logic into the softfloat code we can simply save the FZ
state and temporarily disable it for the softfloat call.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
v4
  - float16_to_floatX squished the wrong softfloat bit for FZ16;
    need to adjust input denormals in this case.
---
 target/arm/helper.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index a1c1dc5bbe..e05c7230d4 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11542,22 +11542,54 @@ uint32_t HELPER(set_neon_rmode)(uint32_t rmode, CPUARMState *env)
 /* Half precision conversions.  */
 float32 HELPER(vfp_fcvt_f16_to_f32)(float16 a, void *fpstp, uint32_t ahp_mode)
 {
-    return float16_to_float32(a, !ahp_mode, fpstp);
+    /* Squash FZ16 to 0 for the duration of conversion.  In this case,
+     * it would affect flushing input denormals.
+     */
+    float_status *fpst = fpstp;
+    flag save = get_flush_inputs_to_zero(fpst);
+    set_flush_inputs_to_zero(false, fpst);
+    float32 r = float16_to_float32(a, !ahp_mode, fpst);
+    set_flush_inputs_to_zero(save, fpst);
+    return r;
 }
 
 float16 HELPER(vfp_fcvt_f32_to_f16)(float32 a, void *fpstp, uint32_t ahp_mode)
 {
-    return float32_to_float16(a, !ahp_mode, fpstp);
+    /* Squash FZ16 to 0 for the duration of conversion.  In this case,
+     * it would affect flushing output denormals.
+     */
+    float_status *fpst = fpstp;
+    flag save = get_flush_to_zero(fpst);
+    set_flush_to_zero(false, fpst);
+    float16 r = float32_to_float16(a, !ahp_mode, fpst);
+    set_flush_to_zero(save, fpst);
+    return r;
 }
 
 float64 HELPER(vfp_fcvt_f16_to_f64)(float16 a, void *fpstp, uint32_t ahp_mode)
 {
-    return float16_to_float64(a, !ahp_mode, fpstp);
+    /* Squash FZ16 to 0 for the duration of conversion.  In this case,
+     * it would affect flushing input denormals.
+     */
+    float_status *fpst = fpstp;
+    flag save = get_flush_inputs_to_zero(fpst);
+    set_flush_inputs_to_zero(false, fpst);
+    float64 r = float16_to_float64(a, !ahp_mode, fpst);
+    set_flush_inputs_to_zero(save, fpst);
+    return r;
 }
 
 float16 HELPER(vfp_fcvt_f64_to_f16)(float64 a, void *fpstp, uint32_t ahp_mode)
 {
-    return float64_to_float16(a, !ahp_mode, fpstp);
+    /* Squash FZ16 to 0 for the duration of conversion.  In this case,
+     * it would affect flushing output denormals.
+     */
+    float_status *fpst = fpstp;
+    flag save = get_flush_to_zero(fpst);
+    set_flush_to_zero(false, fpst);
+    float16 r = float64_to_float16(a, !ahp_mode, fpst);
+    set_flush_to_zero(save, fpst);
+    return r;
 }
 
 #define float32_two make_float32(0x40000000)
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 14/27] target/arm: squash FZ16 behaviour for conversions

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
>
> The ARM ARM specifies FZ16 is suppressed for conversions. Rather than
> pushing this logic into the softfloat code we can simply save the FZ
> state and temporarily disable it for the softfloat call.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> ---
> v4
>   - float16_to_floatX squished the wrong softfloat bit for FZ16;
>     need to adjust input denormals in this case.
> ---
>  target/arm/helper.c | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(once you've fixed up which fp status pointer we pass to these
helpers, then they do the right thing)

thanks
-- PMM

[Qemu-devel] [PATCH v2 15/27] target/arm: Fix fp_status_f16 tininess before rounding

[Richard Henderson]

From: Peter Maydell <peter.maydell@linaro.org>

In commit d81ce0ef2c4f105 we added an extra float_status field
fp_status_fp16 for Arm, but forgot to initialize it correctly
by setting it to float_tininess_before_rounding. This currently
will only cause problems for the new V8_FP16 feature, since the
float-to-float conversion code doesn't use it yet. The effect
would be that we failed to set the Underflow IEEE exception flag
in all the cases where we should.

Add the missing initialization.

Fixes: d81ce0ef2c4f105
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index d175c5e94f..7939c6b8ae 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -324,6 +324,8 @@ static void arm_cpu_reset(CPUState *s)
                               &env->vfp.fp_status);
     set_float_detect_tininess(float_tininess_before_rounding,
                               &env->vfp.standard_fp_status);
+    set_float_detect_tininess(float_tininess_before_rounding,
+                              &env->vfp.fp_status_f16);
 #ifndef CONFIG_USER_ONLY
     if (kvm_enabled()) {
         kvm_arm_reset_vcpu(cpu);
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 15/27] target/arm: Fix fp_status_f16 tininess before rounding

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
>
> In commit d81ce0ef2c4f105 we added an extra float_status field
> fp_status_fp16 for Arm, but forgot to initialize it correctly
> by setting it to float_tininess_before_rounding. This currently
> will only cause problems for the new V8_FP16 feature, since the
> float-to-float conversion code doesn't use it yet. The effect
> would be that we failed to set the Underflow IEEE exception flag
> in all the cases where we should.
>
> Add the missing initialization.
>
> Fixes: d81ce0ef2c4f105
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

NB that there are reviewed-by: tags from you and Alex that need
to be added to this commit message.

Thanks for fixing up the subject line for me.

thanks
-- PMM

[Qemu-devel] [PATCH v2 16/27] target/arm: Remove floatX_maybe_silence_nan from conversions

[Richard Henderson]

This is now handled properly by the generic softfloat code.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper-a64.c |  1 -
 target/arm/helper.c     | 12 ++----------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index 6f0eb83661..f92bdea732 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -466,7 +466,6 @@ float32 HELPER(fcvtx_f64_to_f32)(float64 a, CPUARMState *env)
     set_float_rounding_mode(float_round_to_zero, &tstat);
     set_float_exception_flags(0, &tstat);
     r = float64_to_float32(a, &tstat);
-    r = float32_maybe_silence_nan(r, &tstat);
     exflags = get_float_exception_flags(&tstat);
     if (exflags & float_flag_inexact) {
         r = make_float32(float32_val(r) | 1);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index e05c7230d4..db8bbe52a6 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11348,20 +11348,12 @@ FLOAT_CONVS(ui, d, 64, u)
 /* floating point conversion */
 float64 VFP_HELPER(fcvtd, s)(float32 x, CPUARMState *env)
 {
-    float64 r = float32_to_float64(x, &env->vfp.fp_status);
-    /* ARM requires that S<->D conversion of any kind of NaN generates
-     * a quiet NaN by forcing the most significant frac bit to 1.
-     */
-    return float64_maybe_silence_nan(r, &env->vfp.fp_status);
+    return float32_to_float64(x, &env->vfp.fp_status);
 }
 
 float32 VFP_HELPER(fcvts, d)(float64 x, CPUARMState *env)
 {
-    float32 r =  float64_to_float32(x, &env->vfp.fp_status);
-    /* ARM requires that S<->D conversion of any kind of NaN generates
-     * a quiet NaN by forcing the most significant frac bit to 1.
-     */
-    return float32_maybe_silence_nan(r, &env->vfp.fp_status);
+    return float64_to_float32(x, &env->vfp.fp_status);
 }
 
 /* VFP3 fixed point conversion.  */
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 16/27] target/arm: Remove floatX_maybe_silence_nan from conversions

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> This is now handled properly by the generic softfloat code.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-a64.c |  1 -
>  target/arm/helper.c     | 12 ++----------
>  2 files changed, 2 insertions(+), 11 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 17/27] target/hppa: Remove floatX_maybe_silence_nan from conversions

[Richard Henderson]

This is now handled properly by the generic softfloat code.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/op_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
index a3af62daf7..912e8d5be4 100644
--- a/target/hppa/op_helper.c
+++ b/target/hppa/op_helper.c
@@ -341,7 +341,6 @@ float64 HELPER(fdiv_d)(CPUHPPAState *env, float64 a, float64 b)
 float64 HELPER(fcnv_s_d)(CPUHPPAState *env, float32 arg)
 {
     float64 ret = float32_to_float64(arg, &env->fp_status);
-    ret = float64_maybe_silence_nan(ret, &env->fp_status);
     update_fr0_op(env, GETPC());
     return ret;
 }
@@ -349,7 +348,6 @@ float64 HELPER(fcnv_s_d)(CPUHPPAState *env, float32 arg)
 float32 HELPER(fcnv_d_s)(CPUHPPAState *env, float64 arg)
 {
     float32 ret = float64_to_float32(arg, &env->fp_status);
-    ret = float32_maybe_silence_nan(ret, &env->fp_status);
     update_fr0_op(env, GETPC());
     return ret;
 }
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 17/27] target/hppa: Remove floatX_maybe_silence_nan from conversions

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> This is now handled properly by the generic softfloat code.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/hppa/op_helper.c | 2 --
>  1 file changed, 2 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 18/27] target/m68k: Use floatX_silence_nan when we have already checked for SNaN

[Richard Henderson]

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/softfloat.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/m68k/softfloat.c b/target/m68k/softfloat.c
index e41b07d042..6ec227e20f 100644
--- a/target/m68k/softfloat.c
+++ b/target/m68k/softfloat.c
@@ -31,13 +31,14 @@ static floatx80 propagateFloatx80NaNOneArg(floatx80 a, float_status *status)
 {
     if (floatx80_is_signaling_nan(a, status)) {
         float_raise(float_flag_invalid, status);
+        a = floatx80_silence_nan(a, status);
     }
 
     if (status->default_nan_mode) {
         return floatx80_default_nan(status);
     }
 
-    return floatx80_maybe_silence_nan(a, status);
+    return a;
 }
 
 /*----------------------------------------------------------------------------
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 18/27] target/m68k: Use floatX_silence_nan when we have already checked for SNaN

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/m68k/softfloat.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/m68k/softfloat.c b/target/m68k/softfloat.c
> index e41b07d042..6ec227e20f 100644
> --- a/target/m68k/softfloat.c
> +++ b/target/m68k/softfloat.c
> @@ -31,13 +31,14 @@ static floatx80 propagateFloatx80NaNOneArg(floatx80 a, float_status *status)
>  {
>      if (floatx80_is_signaling_nan(a, status)) {
>          float_raise(float_flag_invalid, status);
> +        a = floatx80_silence_nan(a, status);
>      }
>
>      if (status->default_nan_mode) {
>          return floatx80_default_nan(status);
>      }
>
> -    return floatx80_maybe_silence_nan(a, status);
> +    return a;
>  }
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 19/27] target/mips: Remove floatX_maybe_silence_nan from conversions

[Richard Henderson]

This is now handled properly by the generic softfloat code.

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@mips.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/msa_helper.c | 4 ----
 target/mips/op_helper.c  | 2 --
 2 files changed, 6 deletions(-)

diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index 8fb7a369ca..c74e3cdc65 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -1615,7 +1615,6 @@ static inline float16 float16_from_float32(int32_t a, flag ieee,
       float16 f_val;
 
       f_val = float32_to_float16((float32)a, ieee, status);
-      f_val = float16_maybe_silence_nan(f_val, status);
 
       return a < 0 ? (f_val | (1 << 15)) : f_val;
 }
@@ -1625,7 +1624,6 @@ static inline float32 float32_from_float64(int64_t a, float_status *status)
       float32 f_val;
 
       f_val = float64_to_float32((float64)a, status);
-      f_val = float32_maybe_silence_nan(f_val, status);
 
       return a < 0 ? (f_val | (1 << 31)) : f_val;
 }
@@ -1636,7 +1634,6 @@ static inline float32 float32_from_float16(int16_t a, flag ieee,
       float32 f_val;
 
       f_val = float16_to_float32((float16)a, ieee, status);
-      f_val = float32_maybe_silence_nan(f_val, status);
 
       return a < 0 ? (f_val | (1 << 31)) : f_val;
 }
@@ -1646,7 +1643,6 @@ static inline float64 float64_from_float32(int32_t a, float_status *status)
       float64 f_val;
 
       f_val = float32_to_float64((float64)a, status);
-      f_val = float64_maybe_silence_nan(f_val, status);
 
       return a < 0 ? (f_val | (1ULL << 63)) : f_val;
 }
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 798cdad030..9025f42366 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -2700,7 +2700,6 @@ uint64_t helper_float_cvtd_s(CPUMIPSState *env, uint32_t fst0)
     uint64_t fdt2;
 
     fdt2 = float32_to_float64(fst0, &env->active_fpu.fp_status);
-    fdt2 = float64_maybe_silence_nan(fdt2, &env->active_fpu.fp_status);
     update_fcr31(env, GETPC());
     return fdt2;
 }
@@ -2790,7 +2789,6 @@ uint32_t helper_float_cvts_d(CPUMIPSState *env, uint64_t fdt0)
     uint32_t fst2;
 
     fst2 = float64_to_float32(fdt0, &env->active_fpu.fp_status);
-    fst2 = float32_maybe_silence_nan(fst2, &env->active_fpu.fp_status);
     update_fcr31(env, GETPC());
     return fst2;
 }
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 19/27] target/mips: Remove floatX_maybe_silence_nan from conversions

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> This is now handled properly by the generic softfloat code.
>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@mips.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 20/27] target/riscv: Remove floatX_maybe_silence_nan from conversions

[Richard Henderson]

This is now handled properly by the generic softfloat code.

Cc: Michael Clark <mjc@sifive.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/fpu_helper.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
index abbadead5c..fdb87d8d82 100644
--- a/target/riscv/fpu_helper.c
+++ b/target/riscv/fpu_helper.c
@@ -279,14 +279,12 @@ uint64_t helper_fmax_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
 
 uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)
 {
-    rs1 = float64_to_float32(rs1, &env->fp_status);
-    return float32_maybe_silence_nan(rs1, &env->fp_status);
+    return float64_to_float32(rs1, &env->fp_status);
 }
 
 uint64_t helper_fcvt_d_s(CPURISCVState *env, uint64_t rs1)
 {
-    rs1 = float32_to_float64(rs1, &env->fp_status);
-    return float64_maybe_silence_nan(rs1, &env->fp_status);
+    return float32_to_float64(rs1, &env->fp_status);
 }
 
 uint64_t helper_fsqrt_d(CPURISCVState *env, uint64_t frs1)
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 20/27] target/riscv: Remove floatX_maybe_silence_nan from conversions

[Michael Clark]

On Sat, May 12, 2018 at 12:43 PM, Richard Henderson <
richard.henderson@linaro.org> wrote:

> This is now handled properly by the generic softfloat code.
>
> Cc: Michael Clark <mjc@sifive.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>

Reviewed-by: Michael Clark <mjc@sifive.com>

---
>  target/riscv/fpu_helper.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index abbadead5c..fdb87d8d82 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -279,14 +279,12 @@ uint64_t helper_fmax_d(CPURISCVState *env, uint64_t
> frs1, uint64_t frs2)
>
>  uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)
>  {
> -    rs1 = float64_to_float32(rs1, &env->fp_status);
> -    return float32_maybe_silence_nan(rs1, &env->fp_status);
> +    return float64_to_float32(rs1, &env->fp_status);
>  }
>
>  uint64_t helper_fcvt_d_s(CPURISCVState *env, uint64_t rs1)
>  {
> -    rs1 = float32_to_float64(rs1, &env->fp_status);
> -    return float64_maybe_silence_nan(rs1, &env->fp_status);
> +    return float32_to_float64(rs1, &env->fp_status);
>  }
>
>  uint64_t helper_fsqrt_d(CPURISCVState *env, uint64_t frs1)
> --
> 2.17.0
>
>

[Qemu-devel] [PATCH v2 21/27] target/s390x: Remove floatX_maybe_silence_nan from conversions

[Richard Henderson]

This is now handled properly by the generic softfloat code.

Cc: Alexander Graf <agraf@suse.de>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/fpu_helper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 43f8bf1c94..5c5b451b3b 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -269,7 +269,7 @@ uint64_t HELPER(ldeb)(CPUS390XState *env, uint64_t f2)
 {
     float64 ret = float32_to_float64(f2, &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return float64_maybe_silence_nan(ret, &env->fpu_status);
+    return ret;
 }
 
 /* convert 128-bit float to 64-bit float */
@@ -277,7 +277,7 @@ uint64_t HELPER(ldxb)(CPUS390XState *env, uint64_t ah, uint64_t al)
 {
     float64 ret = float128_to_float64(make_float128(ah, al), &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return float64_maybe_silence_nan(ret, &env->fpu_status);
+    return ret;
 }
 
 /* convert 64-bit float to 128-bit float */
@@ -285,7 +285,7 @@ uint64_t HELPER(lxdb)(CPUS390XState *env, uint64_t f2)
 {
     float128 ret = float64_to_float128(f2, &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return RET128(float128_maybe_silence_nan(ret, &env->fpu_status));
+    return RET128(ret);
 }
 
 /* convert 32-bit float to 128-bit float */
@@ -293,7 +293,7 @@ uint64_t HELPER(lxeb)(CPUS390XState *env, uint64_t f2)
 {
     float128 ret = float32_to_float128(f2, &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return RET128(float128_maybe_silence_nan(ret, &env->fpu_status));
+    return RET128(ret);
 }
 
 /* convert 64-bit float to 32-bit float */
@@ -301,7 +301,7 @@ uint64_t HELPER(ledb)(CPUS390XState *env, uint64_t f2)
 {
     float32 ret = float64_to_float32(f2, &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return float32_maybe_silence_nan(ret, &env->fpu_status);
+    return ret;
 }
 
 /* convert 128-bit float to 32-bit float */
@@ -309,7 +309,7 @@ uint64_t HELPER(lexb)(CPUS390XState *env, uint64_t ah, uint64_t al)
 {
     float32 ret = float128_to_float32(make_float128(ah, al), &env->fpu_status);
     handle_exceptions(env, GETPC());
-    return float32_maybe_silence_nan(ret, &env->fpu_status);
+    return ret;
 }
 
 /* 32-bit FP compare */
-- 
2.17.0

[Qemu-devel] [PATCH v2 22/27] fpu/softfloat: Use float*_silence_nan in propagateFloat*NaN

[Richard Henderson]

We have already checked the arguments for SNaN;
we don't need to do it again.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 44 +++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 995a0132c6..4fa068a5dc 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -498,7 +498,7 @@ static float32 commonNaNToFloat32(commonNaNT a, float_status *status)
 | The routine is passed various bits of information about the
 | two NaNs and should return 0 to select NaN a and 1 for NaN b.
 | Note that signalling NaNs are always squashed to quiet NaNs
-| by the caller, by calling floatXX_maybe_silence_nan() before
+| by the caller, by calling floatXX_silence_nan() before
 | returning them.
 |
 | aIsLargerSignificand is only valid if both a and b are NaNs
@@ -536,7 +536,7 @@ static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
 {
     /* According to MIPS specifications, if one of the two operands is
      * a sNaN, a new qNaN has to be generated. This is done in
-     * floatXX_maybe_silence_nan(). For qNaN inputs the specifications
+     * floatXX_silence_nan(). For qNaN inputs the specifications
      * says: "When possible, this QNaN result is one of the operand QNaN
      * values." In practice it seems that most implementations choose
      * the first operand if both operands are qNaN. In short this gives
@@ -788,9 +788,15 @@ static float32 propagateFloat32NaN(float32 a, float32 b, float_status *status)
 
     if (pickNaN(aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
                 aIsLargerSignificand)) {
-        return float32_maybe_silence_nan(b, status);
+        if (bIsSignalingNaN) {
+            return float32_silence_nan(b, status);
+        }
+        return b;
     } else {
-        return float32_maybe_silence_nan(a, status);
+        if (aIsSignalingNaN) {
+            return float32_silence_nan(a, status);
+        }
+        return a;
     }
 }
 
@@ -950,9 +956,15 @@ static float64 propagateFloat64NaN(float64 a, float64 b, float_status *status)
 
     if (pickNaN(aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
                 aIsLargerSignificand)) {
-        return float64_maybe_silence_nan(b, status);
+        if (bIsSignalingNaN) {
+            return float64_silence_nan(b, status);
+        }
+        return b;
     } else {
-        return float64_maybe_silence_nan(a, status);
+        if (aIsSignalingNaN) {
+            return float64_silence_nan(a, status);
+        }
+        return a;
     }
 }
 
@@ -1121,9 +1133,15 @@ floatx80 propagateFloatx80NaN(floatx80 a, floatx80 b, float_status *status)
 
     if (pickNaN(aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
                 aIsLargerSignificand)) {
-        return floatx80_maybe_silence_nan(b, status);
+        if (bIsSignalingNaN) {
+            return floatx80_silence_nan(b, status);
+        }
+        return b;
     } else {
-        return floatx80_maybe_silence_nan(a, status);
+        if (aIsSignalingNaN) {
+            return floatx80_silence_nan(a, status);
+        }
+        return a;
     }
 }
 
@@ -1270,8 +1288,14 @@ static float128 propagateFloat128NaN(float128 a, float128 b,
 
     if (pickNaN(aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
                 aIsLargerSignificand)) {
-        return float128_maybe_silence_nan(b, status);
+        if (bIsSignalingNaN) {
+            return float128_silence_nan(b, status);
+        }
+        return b;
     } else {
-        return float128_maybe_silence_nan(a, status);
+        if (aIsSignalingNaN) {
+            return float128_silence_nan(a, status);
+        }
+        return a;
     }
 }
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 22/27] fpu/softfloat: Use float*_silence_nan in propagateFloat*NaN

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> We have already checked the arguments for SNaN;
> we don't need to do it again.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 23/27] fpu/softfloat: Remove floatX_maybe_silence_nan

[Richard Henderson]

These functions are now unused.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 63 --------------------------------------
 include/fpu/softfloat.h    |  5 ---
 2 files changed, 68 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 4fa068a5dc..d7033b7757 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -364,19 +364,6 @@ float16 float16_silence_nan(float16 a, float_status *status)
 #endif
 }
 
-/*----------------------------------------------------------------------------
-| Returns a quiet NaN if the half-precision floating point value `a' is a
-| signaling NaN; otherwise returns `a'.
-*----------------------------------------------------------------------------*/
-
-float16 float16_maybe_silence_nan(float16 a, float_status *status)
-{
-    if (float16_is_signaling_nan(a, status)) {
-        return float16_silence_nan(a, status);
-    }
-    return a;
-}
-
 /*----------------------------------------------------------------------------
 | Returns 1 if the single-precision floating-point value `a' is a quiet
 | NaN; otherwise returns 0.
@@ -438,18 +425,6 @@ float32 float32_silence_nan(float32 a, float_status *status)
     }
 #endif
 }
-/*----------------------------------------------------------------------------
-| Returns a quiet NaN if the single-precision floating point value `a' is a
-| signaling NaN; otherwise returns `a'.
-*----------------------------------------------------------------------------*/
-
-float32 float32_maybe_silence_nan(float32 a, float_status *status)
-{
-    if (float32_is_signaling_nan(a, status)) {
-        return float32_silence_nan(a, status);
-    }
-    return a;
-}
 
 /*----------------------------------------------------------------------------
 | Returns the result of converting the single-precision floating-point NaN
@@ -864,18 +839,6 @@ float64 float64_silence_nan(float64 a, float_status *status)
 #endif
 }
 
-/*----------------------------------------------------------------------------
-| Returns a quiet NaN if the double-precision floating point value `a' is a
-| signaling NaN; otherwise returns `a'.
-*----------------------------------------------------------------------------*/
-
-float64 float64_maybe_silence_nan(float64 a, float_status *status)
-{
-    if (float64_is_signaling_nan(a, status)) {
-        return float64_silence_nan(a, status);
-    }
-    return a;
-}
 
 /*----------------------------------------------------------------------------
 | Returns the result of converting the double-precision floating-point NaN
@@ -1037,19 +1000,6 @@ floatx80 floatx80_silence_nan(floatx80 a, float_status *status)
 #endif
 }
 
-/*----------------------------------------------------------------------------
-| Returns a quiet NaN if the extended double-precision floating point value
-| `a' is a signaling NaN; otherwise returns `a'.
-*----------------------------------------------------------------------------*/
-
-floatx80 floatx80_maybe_silence_nan(floatx80 a, float_status *status)
-{
-    if (floatx80_is_signaling_nan(a, status)) {
-        return floatx80_silence_nan(a, status);
-    }
-    return a;
-}
-
 /*----------------------------------------------------------------------------
 | Returns the result of converting the extended double-precision floating-
 | point NaN `a' to the canonical NaN format.  If `a' is a signaling NaN, the
@@ -1204,19 +1154,6 @@ float128 float128_silence_nan(float128 a, float_status *status)
 #endif
 }
 
-/*----------------------------------------------------------------------------
-| Returns a quiet NaN if the quadruple-precision floating point value `a' is
-| a signaling NaN; otherwise returns `a'.
-*----------------------------------------------------------------------------*/
-
-float128 float128_maybe_silence_nan(float128 a, float_status *status)
-{
-    if (float128_is_signaling_nan(a, status)) {
-        return float128_silence_nan(a, status);
-    }
-    return a;
-}
-
 /*----------------------------------------------------------------------------
 | Returns the result of converting the quadruple-precision floating-point NaN
 | `a' to the canonical NaN format.  If `a' is a signaling NaN, the invalid
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index a6860e858d..69f4dbc4db 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -258,7 +258,6 @@ int float16_compare_quiet(float16, float16, float_status *status);
 int float16_is_quiet_nan(float16, float_status *status);
 int float16_is_signaling_nan(float16, float_status *status);
 float16 float16_silence_nan(float16, float_status *status);
-float16 float16_maybe_silence_nan(float16, float_status *status);
 
 static inline int float16_is_any_nan(float16 a)
 {
@@ -370,7 +369,6 @@ float32 float32_maxnummag(float32, float32, float_status *status);
 int float32_is_quiet_nan(float32, float_status *status);
 int float32_is_signaling_nan(float32, float_status *status);
 float32 float32_silence_nan(float32, float_status *status);
-float32 float32_maybe_silence_nan(float32, float_status *status);
 float32 float32_scalbn(float32, int, float_status *status);
 
 static inline float32 float32_abs(float32 a)
@@ -500,7 +498,6 @@ float64 float64_maxnummag(float64, float64, float_status *status);
 int float64_is_quiet_nan(float64 a, float_status *status);
 int float64_is_signaling_nan(float64, float_status *status);
 float64 float64_silence_nan(float64, float_status *status);
-float64 float64_maybe_silence_nan(float64, float_status *status);
 float64 float64_scalbn(float64, int, float_status *status);
 
 static inline float64 float64_abs(float64 a)
@@ -604,7 +601,6 @@ int floatx80_compare_quiet(floatx80, floatx80, float_status *status);
 int floatx80_is_quiet_nan(floatx80, float_status *status);
 int floatx80_is_signaling_nan(floatx80, float_status *status);
 floatx80 floatx80_silence_nan(floatx80, float_status *status);
-floatx80 floatx80_maybe_silence_nan(floatx80, float_status *status);
 floatx80 floatx80_scalbn(floatx80, int, float_status *status);
 
 static inline floatx80 floatx80_abs(floatx80 a)
@@ -816,7 +812,6 @@ int float128_compare_quiet(float128, float128, float_status *status);
 int float128_is_quiet_nan(float128, float_status *status);
 int float128_is_signaling_nan(float128, float_status *status);
 float128 float128_silence_nan(float128, float_status *status);
-float128 float128_maybe_silence_nan(float128, float_status *status);
 float128 float128_scalbn(float128, int, float_status *status);
 
 static inline float128 float128_abs(float128 a)
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 23/27] fpu/softfloat: Remove floatX_maybe_silence_nan

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> These functions are now unused.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h | 63 --------------------------------------
>  include/fpu/softfloat.h    |  5 ---
>  2 files changed, 68 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 24/27] fpu/softfloat: Specialize on snan_bit_is_one

[Richard Henderson]

Only MIPS requires snan_bit_is_one to be variable.  While we are
specializing softfloat behaviour, allow other targets to eliminate
this runtime check.

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@mips.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Alexander Graf <agraf@suse.de>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h    | 68 ++++++++++++++++++++++-------------
 include/fpu/softfloat-types.h |  1 +
 include/fpu/softfloat.h       |  4 ---
 target/mips/cpu.h             |  4 +--
 target/hppa/cpu.c             |  1 -
 target/mips/translate_init.c  |  4 +--
 target/ppc/fpu_helper.c       |  1 -
 target/sh4/cpu.c              |  1 -
 target/unicore32/cpu.c        |  2 --
 9 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index d7033b7757..5cd9c2eedc 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -79,13 +79,31 @@ this code that are retained.
  * version 2 or later. See the COPYING file in the top-level directory.
  */
 
-#if defined(TARGET_XTENSA)
 /* Define for architectures which deviate from IEEE in not supporting
  * signaling NaNs (so all NaNs are treated as quiet).
  */
+#if defined(TARGET_XTENSA)
 #define NO_SIGNALING_NANS 1
 #endif
 
+/* Define how the architecture discriminates signaling NaNs.
+ * This done with the most significant bit of the fraction.
+ * In the original IEEE754 spec this was implementation defined;
+ * in the 2008 revision the msb is zero.  MIPS is (so far) unique
+ * in supporting both the 2008 revision and backward compatibility
+ * with their original choice and thus must make the choice at runtime.
+ */
+static inline flag snan_bit_is_one(float_status *status)
+{
+#if defined(TARGET_MIPS)
+    return status->snan_bit_is_one;
+#elif defined(TARGET_HPPA) || defined(TARGET_UNICORE32) || defined(TARGET_SH4)
+    return 1;
+#else
+    return 0;
+#endif
+}
+
 /*----------------------------------------------------------------------------
 | For the deconstructed floating-point with fraction FRAC, return true
 | if the fraction represents a signalling NaN; otherwise false.
@@ -97,7 +115,7 @@ static bool parts_is_snan_frac(uint64_t frac, float_status *status)
     return false;
 #else
     flag msb = extract64(frac, DECOMPOSED_BINARY_POINT - 1, 1);
-    return msb == status->snan_bit_is_one;
+    return msb == snan_bit_is_one(status);
 #endif
 }
 
@@ -118,7 +136,7 @@ static FloatParts parts_default_nan(float_status *status)
 #elif defined(TARGET_HPPA)
     frac = 1ULL << (DECOMPOSED_BINARY_POINT - 2);
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         frac = (1ULL << (DECOMPOSED_BINARY_POINT - 1)) - 1;
     } else {
 #if defined(TARGET_MIPS)
@@ -151,7 +169,7 @@ static FloatParts parts_silence_nan(FloatParts a, float_status *status)
     a.frac &= ~(1ULL << (DECOMPOSED_BINARY_POINT - 1));
     a.frac |= 1ULL << (DECOMPOSED_BINARY_POINT - 2);
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return parts_default_nan(status);
     } else {
         a.frac |= 1ULL << (DECOMPOSED_BINARY_POINT - 1);
@@ -169,7 +187,7 @@ float16 float16_default_nan(float_status *status)
 #if defined(TARGET_ARM)
     return const_float16(0x7E00);
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return const_float16(0x7DFF);
     } else {
 #if defined(TARGET_MIPS)
@@ -195,7 +213,7 @@ float32 float32_default_nan(float_status *status)
 #elif defined(TARGET_HPPA)
     return const_float32(0x7FA00000);
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return const_float32(0x7FBFFFFF);
     } else {
 #if defined(TARGET_MIPS)
@@ -220,7 +238,7 @@ float64 float64_default_nan(float_status *status)
 #elif defined(TARGET_HPPA)
     return const_float64(LIT64(0x7FF4000000000000));
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return const_float64(LIT64(0x7FF7FFFFFFFFFFFF));
     } else {
 #if defined(TARGET_MIPS)
@@ -242,7 +260,7 @@ floatx80 floatx80_default_nan(float_status *status)
     r.low = LIT64(0xFFFFFFFFFFFFFFFF);
     r.high = 0x7FFF;
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         r.low = LIT64(0xBFFFFFFFFFFFFFFF);
         r.high = 0x7FFF;
     } else {
@@ -274,7 +292,7 @@ float128 float128_default_nan(float_status *status)
 {
     float128 r;
 
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         r.low = LIT64(0xFFFFFFFFFFFFFFFF);
         r.high = LIT64(0x7FFF7FFFFFFFFFFF);
     } else {
@@ -319,7 +337,7 @@ int float16_is_quiet_nan(float16 a_, float_status *status)
     return float16_is_any_nan(a_);
 #else
     uint16_t a = float16_val(a_);
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return (((a >> 9) & 0x3F) == 0x3E) && (a & 0x1FF);
     } else {
         return ((a & ~0x8000) >= 0x7C80);
@@ -338,7 +356,7 @@ int float16_is_signaling_nan(float16 a_, float_status *status)
     return 0;
 #else
     uint16_t a = float16_val(a_);
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return ((a & ~0x8000) >= 0x7C80);
     } else {
         return (((a >> 9) & 0x3F) == 0x3E) && (a & 0x1FF);
@@ -356,7 +374,7 @@ float16 float16_silence_nan(float16 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     g_assert_not_reached();
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return float16_default_nan(status);
     } else {
         return a | (1 << 9);
@@ -375,7 +393,7 @@ int float32_is_quiet_nan(float32 a_, float_status *status)
     return float32_is_any_nan(a_);
 #else
     uint32_t a = float32_val(a_);
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return (((a >> 22) & 0x1FF) == 0x1FE) && (a & 0x003FFFFF);
     } else {
         return ((uint32_t)(a << 1) >= 0xFF800000);
@@ -394,7 +412,7 @@ int float32_is_signaling_nan(float32 a_, float_status *status)
     return 0;
 #else
     uint32_t a = float32_val(a_);
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return ((uint32_t)(a << 1) >= 0xFF800000);
     } else {
         return (((a >> 22) & 0x1FF) == 0x1FE) && (a & 0x003FFFFF);
@@ -412,7 +430,7 @@ float32 float32_silence_nan(float32 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     g_assert_not_reached();
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
 # ifdef TARGET_HPPA
         a &= ~0x00400000;
         a |=  0x00200000;
@@ -651,7 +669,7 @@ static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
         return 3;
     }
 
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         /* Prefer sNaN over qNaN, in the a, b, c order. */
         if (aIsSNaN) {
             return 0;
@@ -786,7 +804,7 @@ int float64_is_quiet_nan(float64 a_, float_status *status)
     return float64_is_any_nan(a_);
 #else
     uint64_t a = float64_val(a_);
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return (((a >> 51) & 0xFFF) == 0xFFE)
             && (a & 0x0007FFFFFFFFFFFFULL);
     } else {
@@ -806,7 +824,7 @@ int float64_is_signaling_nan(float64 a_, float_status *status)
     return 0;
 #else
     uint64_t a = float64_val(a_);
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return ((a << 1) >= 0xFFF0000000000000ULL);
     } else {
         return (((a >> 51) & 0xFFF) == 0xFFE)
@@ -825,7 +843,7 @@ float64 float64_silence_nan(float64 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     g_assert_not_reached();
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
 # ifdef TARGET_HPPA
         a &= ~0x0008000000000000ULL;
         a |=  0x0004000000000000ULL;
@@ -942,7 +960,7 @@ int floatx80_is_quiet_nan(floatx80 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     return floatx80_is_any_nan(a);
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         uint64_t aLow;
 
         aLow = a.low & ~0x4000000000000000ULL;
@@ -967,7 +985,7 @@ int floatx80_is_signaling_nan(floatx80 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     return 0;
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return ((a.high & 0x7FFF) == 0x7FFF)
             && ((a.low << 1) >= 0x8000000000000000ULL);
     } else {
@@ -991,7 +1009,7 @@ floatx80 floatx80_silence_nan(floatx80 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     g_assert_not_reached();
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return floatx80_default_nan(status);
     } else {
         a.low |= LIT64(0xC000000000000000);
@@ -1105,7 +1123,7 @@ int float128_is_quiet_nan(float128 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     return float128_is_any_nan(a);
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return (((a.high >> 47) & 0xFFFF) == 0xFFFE)
             && (a.low || (a.high & 0x00007FFFFFFFFFFFULL));
     } else {
@@ -1125,7 +1143,7 @@ int float128_is_signaling_nan(float128 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     return 0;
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return ((a.high << 1) >= 0xFFFF000000000000ULL)
             && (a.low || (a.high & 0x0000FFFFFFFFFFFFULL));
     } else {
@@ -1145,7 +1163,7 @@ float128 float128_silence_nan(float128 a, float_status *status)
 #ifdef NO_SIGNALING_NANS
     g_assert_not_reached();
 #else
-    if (status->snan_bit_is_one) {
+    if (snan_bit_is_one(status)) {
         return float128_default_nan(status);
     } else {
         a.high |= LIT64(0x0000800000000000);
diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 4e378cb612..b5207d4537 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -173,6 +173,7 @@ typedef struct float_status {
     /* should denormalised inputs go to zero and set the input_denormal flag? */
     flag flush_inputs_to_zero;
     flag default_nan_mode;
+    /* not always used -- see SNAN_BIT_IS_ONE in softfloat-specialize.h */
     flag snan_bit_is_one;
 } float_status;
 
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 69f4dbc4db..e72cc9525d 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -125,10 +125,6 @@ static inline void set_default_nan_mode(flag val, float_status *status)
 {
     status->default_nan_mode = val;
 }
-static inline void set_snan_bit_is_one(flag val, float_status *status)
-{
-    status->snan_bit_is_one = val;
-}
 static inline int get_float_detect_tininess(float_status *status)
 {
     return status->float_detect_tininess;
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index cfe1735e0e..2abce47ea3 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -755,8 +755,8 @@ target_ulong exception_resume_pc (CPUMIPSState *env);
 
 static inline void restore_snan_bit_mode(CPUMIPSState *env)
 {
-    set_snan_bit_is_one((env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) == 0,
-                        &env->active_fpu.fp_status);
+    env->active_fpu.fp_status.snan_bit_is_one
+        = (env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) == 0;
 }
 
 static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc,
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index c261b6b090..00bf444620 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -141,7 +141,6 @@ static void hppa_cpu_initfn(Object *obj)
     cs->env_ptr = env;
     cs->exception_index = -1;
     cpu_hppa_loaded_fr0(env);
-    set_snan_bit_is_one(true, &env->fp_status);
     cpu_hppa_put_psw(env, PSW_W);
 }
 
diff --git a/target/mips/translate_init.c b/target/mips/translate_init.c
index c7ba6ee5f9..5e40d6a198 100644
--- a/target/mips/translate_init.c
+++ b/target/mips/translate_init.c
@@ -878,6 +878,6 @@ static void msa_reset(CPUMIPSState *env)
     /* clear float_status nan mode */
     set_default_nan_mode(0, &env->active_tc.msa_fp_status);
 
-    /* set proper signanling bit meaning ("1" means "quiet") */
-    set_snan_bit_is_one(0, &env->active_tc.msa_fp_status);
+    /* set proper signaling bit meaning ("1" means "quiet") */
+    env->active_tc.msa_fp_status.snan_bit_is_one = 0;
 }
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 9ae418a577..d31a933cbb 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -3382,7 +3382,6 @@ void helper_xssqrtqp(CPUPPCState *env, uint32_t opcode)
             xt.f128 = xb.f128;
         } else if (float128_is_neg(xb.f128) && !float128_is_zero(xb.f128)) {
             float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
-            set_snan_bit_is_one(0, &env->fp_status);
             xt.f128 = float128_default_nan(&env->fp_status);
         }
     }
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 541ffc2d97..b9f393b7c7 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -71,7 +71,6 @@ static void superh_cpu_reset(CPUState *s)
     set_flush_to_zero(1, &env->fp_status);
 #endif
     set_default_nan_mode(1, &env->fp_status);
-    set_snan_bit_is_one(1, &env->fp_status);
 }
 
 static void superh_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
diff --git a/target/unicore32/cpu.c b/target/unicore32/cpu.c
index 29d160a88d..68f978d80b 100644
--- a/target/unicore32/cpu.c
+++ b/target/unicore32/cpu.c
@@ -70,7 +70,6 @@ static void unicore_ii_cpu_initfn(Object *obj)
 
     set_feature(env, UC32_HWCAP_CMOV);
     set_feature(env, UC32_HWCAP_UCF64);
-    set_snan_bit_is_one(1, &env->ucf64.fp_status);
 }
 
 static void uc32_any_cpu_initfn(Object *obj)
@@ -83,7 +82,6 @@ static void uc32_any_cpu_initfn(Object *obj)
 
     set_feature(env, UC32_HWCAP_CMOV);
     set_feature(env, UC32_HWCAP_UCF64);
-    set_snan_bit_is_one(1, &env->ucf64.fp_status);
 }
 
 static void uc32_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 24/27] fpu/softfloat: Specialize on snan_bit_is_one

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> Only MIPS requires snan_bit_is_one to be variable.  While we are
> specializing softfloat behaviour, allow other targets to eliminate
> this runtime check.
>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@mips.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h    | 68 ++++++++++++++++++++++-------------
>  include/fpu/softfloat-types.h |  1 +
>  include/fpu/softfloat.h       |  4 ---
>  target/mips/cpu.h             |  4 +--
>  target/hppa/cpu.c             |  1 -
>  target/mips/translate_init.c  |  4 +--
>  target/ppc/fpu_helper.c       |  1 -
>  target/sh4/cpu.c              |  1 -
>  target/unicore32/cpu.c        |  2 --
>  9 files changed, 48 insertions(+), 38 deletions(-)


> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index 69f4dbc4db..e72cc9525d 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -125,10 +125,6 @@ static inline void set_default_nan_mode(flag val, float_status *status)
>  {
>      status->default_nan_mode = val;
>  }
> -static inline void set_snan_bit_is_one(flag val, float_status *status)
> -{
> -    status->snan_bit_is_one = val;
> -}
>  static inline int get_float_detect_tininess(float_status *status)
>  {
>      return status->float_detect_tininess;
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index cfe1735e0e..2abce47ea3 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -755,8 +755,8 @@ target_ulong exception_resume_pc (CPUMIPSState *env);
>
>  static inline void restore_snan_bit_mode(CPUMIPSState *env)
>  {
> -    set_snan_bit_is_one((env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) == 0,
> -                        &env->active_fpu.fp_status);
> +    env->active_fpu.fp_status.snan_bit_is_one
> +        = (env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) == 0;
>  }
>
>  static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc,
>
> diff --git a/target/mips/translate_init.c b/target/mips/translate_init.c
> index c7ba6ee5f9..5e40d6a198 100644
> --- a/target/mips/translate_init.c
> +++ b/target/mips/translate_init.c
> @@ -878,6 +878,6 @@ static void msa_reset(CPUMIPSState *env)
>      /* clear float_status nan mode */
>      set_default_nan_mode(0, &env->active_tc.msa_fp_status);
>
> -    /* set proper signanling bit meaning ("1" means "quiet") */
> -    set_snan_bit_is_one(0, &env->active_tc.msa_fp_status);
> +    /* set proper signaling bit meaning ("1" means "quiet") */
> +    env->active_tc.msa_fp_status.snan_bit_is_one = 0;
>  }

Why remove the set_snan_bit_is_one() helper and require mips
to manually look inside the float_status struct?

> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index 9ae418a577..d31a933cbb 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -3382,7 +3382,6 @@ void helper_xssqrtqp(CPUPPCState *env, uint32_t opcode)
>              xt.f128 = xb.f128;
>          } else if (float128_is_neg(xb.f128) && !float128_is_zero(xb.f128)) {
>              float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSQRT, 1);
> -            set_snan_bit_is_one(0, &env->fp_status);
>              xt.f128 = float128_default_nan(&env->fp_status);
>          }
>      }

This one's weird -- why were we setting the snan_bit_is_one field
in the middle of handling a square root instruction? Is this a
bug where this was intended to be doing something else? David,
do you remember?  (change added in commit a4a68476def684f2f).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 24/27] fpu/softfloat: Specialize on snan_bit_is_one

[Richard Henderson]

On 05/14/2018 07:44 AM, Peter Maydell wrote:
> On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
>> Only MIPS requires snan_bit_is_one to be variable.  While we are
>> specializing softfloat behaviour, allow other targets to eliminate
>> this runtime check.
>>
>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>> Cc: Yongbok Kim <yongbok.kim@mips.com>
>> Cc: David Gibson <david@gibson.dropbear.id.au>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  fpu/softfloat-specialize.h    | 68 ++++++++++++++++++++++-------------
>>  include/fpu/softfloat-types.h |  1 +
>>  include/fpu/softfloat.h       |  4 ---
>>  target/mips/cpu.h             |  4 +--
>>  target/hppa/cpu.c             |  1 -
>>  target/mips/translate_init.c  |  4 +--
>>  target/ppc/fpu_helper.c       |  1 -
>>  target/sh4/cpu.c              |  1 -
>>  target/unicore32/cpu.c        |  2 --
>>  9 files changed, 48 insertions(+), 38 deletions(-)
> 
> 
>> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
>> index 69f4dbc4db..e72cc9525d 100644
>> --- a/include/fpu/softfloat.h
>> +++ b/include/fpu/softfloat.h
>> @@ -125,10 +125,6 @@ static inline void set_default_nan_mode(flag val, float_status *status)
>>  {
>>      status->default_nan_mode = val;
>>  }
>> -static inline void set_snan_bit_is_one(flag val, float_status *status)
>> -{
>> -    status->snan_bit_is_one = val;
>> -}
>>  static inline int get_float_detect_tininess(float_status *status)
>>  {
>>      return status->float_detect_tininess;
>> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
>> index cfe1735e0e..2abce47ea3 100644
>> --- a/target/mips/cpu.h
>> +++ b/target/mips/cpu.h
>> @@ -755,8 +755,8 @@ target_ulong exception_resume_pc (CPUMIPSState *env);
>>
>>  static inline void restore_snan_bit_mode(CPUMIPSState *env)
>>  {
>> -    set_snan_bit_is_one((env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) == 0,
>> -                        &env->active_fpu.fp_status);
>> +    env->active_fpu.fp_status.snan_bit_is_one
>> +        = (env->active_fpu.fcr31 & (1 << FCR31_NAN2008)) == 0;
>>  }
>>
>>  static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc,
>>
>> diff --git a/target/mips/translate_init.c b/target/mips/translate_init.c
>> index c7ba6ee5f9..5e40d6a198 100644
>> --- a/target/mips/translate_init.c
>> +++ b/target/mips/translate_init.c
>> @@ -878,6 +878,6 @@ static void msa_reset(CPUMIPSState *env)
>>      /* clear float_status nan mode */
>>      set_default_nan_mode(0, &env->active_tc.msa_fp_status);
>>
>> -    /* set proper signanling bit meaning ("1" means "quiet") */
>> -    set_snan_bit_is_one(0, &env->active_tc.msa_fp_status);
>> +    /* set proper signaling bit meaning ("1" means "quiet") */
>> +    env->active_tc.msa_fp_status.snan_bit_is_one = 0;
>>  }
> 
> Why remove the set_snan_bit_is_one() helper and require mips
> to manually look inside the float_status struct?

What I really wanted was to #ifdef TARGET_MIPS around the function and the
field, but it's poisoned at that point.  But you're right, I could have left
the function.


r~

[Qemu-devel] [PATCH v2 25/27] fpu/softfloat: Make is_nan et al available to softfloat-specialize.h

[Richard Henderson]

We will need these helpers within softfloat-specialize.h, so move
the definitions above the include.  After specialization, they will
not always be used so mark them to avoid the Werror.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 15a272759d..bafb3b17ec 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -181,6 +181,22 @@ typedef enum __attribute__ ((__packed__)) {
     float_class_snan,
 } FloatClass;
 
+/* Simple helpers for checking if what NaN we have */
+static inline __attribute__((unused)) bool is_nan(FloatClass c)
+{
+    return unlikely(c >= float_class_qnan);
+}
+
+static inline __attribute__((unused)) bool is_snan(FloatClass c)
+{
+    return c == float_class_snan;
+}
+
+static inline __attribute__((unused)) bool is_qnan(FloatClass c)
+{
+    return c == float_class_qnan;
+}
+
 /*
  * Structure holding all of the decomposed parts of a float. The
  * exponent is unbiased and the fraction is normalized. All
@@ -536,20 +552,6 @@ static float64 float64_round_pack_canonical(FloatParts p, float_status *s)
     return float64_pack_raw(round_canonical(p, s, &float64_params));
 }
 
-/* Simple helpers for checking if what NaN we have */
-static bool is_nan(FloatClass c)
-{
-    return unlikely(c >= float_class_qnan);
-}
-static bool is_snan(FloatClass c)
-{
-    return c == float_class_snan;
-}
-static bool is_qnan(FloatClass c)
-{
-    return c == float_class_qnan;
-}
-
 static FloatParts return_nan(FloatParts a, float_status *s)
 {
     switch (a.cls) {
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 25/27] fpu/softfloat: Make is_nan et al available to softfloat-specialize.h

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> We will need these helpers within softfloat-specialize.h, so move
> the definitions above the include.  After specialization, they will
> not always be used so mark them to avoid the Werror.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 15a272759d..bafb3b17ec 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -181,6 +181,22 @@ typedef enum __attribute__ ((__packed__)) {
>      float_class_snan,
>  } FloatClass;
>
> +/* Simple helpers for checking if what NaN we have */

"if/what" (ditto below).

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 26/27] fpu/softfloat: Pass FloatClass to pickNaN

[Richard Henderson]

For each operand, pass a single enumeration instead of a pair of booleans.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 168 ++++++++++++++++++-------------------
 fpu/softfloat.c            |   3 +-
 2 files changed, 82 insertions(+), 89 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 5cd9c2eedc..47c67caec6 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -501,10 +501,10 @@ static float32 commonNaNToFloat32(commonNaNT a, float_status *status)
 | tie-break rule.
 *----------------------------------------------------------------------------*/
 
-#if defined(TARGET_ARM)
-static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
+static int pickNaN(FloatClass a_cls, FloatClass b_cls,
                    flag aIsLargerSignificand)
 {
+#if defined(TARGET_ARM) || defined(TARGET_MIPS) || defined(TARGET_HPPA)
     /* ARM mandated NaN propagation rules (see FPProcessNaNs()), take
      * the first of:
      *  1. A if it is signaling
@@ -513,20 +513,6 @@ static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
      *  4. B (quiet)
      * A signaling NaN is always quietened before returning it.
      */
-    if (aIsSNaN) {
-        return 0;
-    } else if (bIsSNaN) {
-        return 1;
-    } else if (aIsQNaN) {
-        return 0;
-    } else {
-        return 1;
-    }
-}
-#elif defined(TARGET_MIPS) || defined(TARGET_HPPA)
-static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-                    flag aIsLargerSignificand)
-{
     /* According to MIPS specifications, if one of the two operands is
      * a sNaN, a new qNaN has to be generated. This is done in
      * floatXX_silence_nan(). For qNaN inputs the specifications
@@ -540,35 +526,21 @@ static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
      *  4. B (quiet)
      * A signaling NaN is always silenced before returning it.
      */
-    if (aIsSNaN) {
+    if (is_snan(a_cls)) {
         return 0;
-    } else if (bIsSNaN) {
+    } else if (is_snan(b_cls)) {
         return 1;
-    } else if (aIsQNaN) {
+    } else if (is_qnan(a_cls)) {
         return 0;
     } else {
         return 1;
     }
-}
-#elif defined(TARGET_PPC) || defined(TARGET_XTENSA)
-static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-                   flag aIsLargerSignificand)
-{
+#elif defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
     /* PowerPC propagation rules:
      *  1. A if it sNaN or qNaN
      *  2. B if it sNaN or qNaN
      * A signaling NaN is always silenced before returning it.
      */
-    if (aIsSNaN || aIsQNaN) {
-        return 0;
-    } else {
-        return 1;
-    }
-}
-#elif defined(TARGET_M68K)
-static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-                   flag aIsLargerSignificand)
-{
     /* M68000 FAMILY PROGRAMMER'S REFERENCE MANUAL
      * 3.4 FLOATING-POINT INSTRUCTION DETAILS
      * If either operand, but not both operands, of an operation is a
@@ -583,16 +555,12 @@ static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
      * a nonsignaling NaN. The operation then continues as described in the
      * preceding paragraph for nonsignaling NaNs.
      */
-    if (aIsQNaN || aIsSNaN) { /* a is the destination operand */
-        return 0; /* return the destination operand */
+    if (is_nan(a_cls)) {
+        return 0;
     } else {
-        return 1; /* return b */
+        return 1;
     }
-}
 #else
-static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-                    flag aIsLargerSignificand)
-{
     /* This implements x87 NaN propagation rules:
      * SNaN + QNaN => return the QNaN
      * two SNaNs => return the one with the larger significand, silenced
@@ -603,13 +571,13 @@ static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
      * If we get down to comparing significands and they are the same,
      * return the NaN with the positive sign bit (if any).
      */
-    if (aIsSNaN) {
-        if (bIsSNaN) {
+    if (is_snan(a_cls)) {
+        if (is_snan(b_cls)) {
             return aIsLargerSignificand ? 0 : 1;
         }
-        return bIsQNaN ? 1 : 0;
-    } else if (aIsQNaN) {
-        if (bIsSNaN || !bIsQNaN) {
+        return is_qnan(b_cls) ? 1 : 0;
+    } else if (is_qnan(a_cls)) {
+        if (is_snan(b_cls) || !is_qnan(b_cls)) {
             return 0;
         } else {
             return aIsLargerSignificand ? 0 : 1;
@@ -617,8 +585,8 @@ static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
     } else {
         return 1;
     }
-}
 #endif
+}
 
 /*----------------------------------------------------------------------------
 | Select which NaN to propagate for a three-input operation.
@@ -752,18 +720,26 @@ static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
 
 static float32 propagateFloat32NaN(float32 a, float32 b, float_status *status)
 {
-    flag aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN;
     flag aIsLargerSignificand;
     uint32_t av, bv;
+    FloatClass a_cls, b_cls;
+
+    /* This is not complete, but is good enough for pickNaN.  */
+    a_cls = (!float32_is_any_nan(a)
+             ? float_class_normal
+             : float32_is_signaling_nan(a, status)
+             ? float_class_snan
+             : float_class_qnan);
+    b_cls = (!float32_is_any_nan(b)
+             ? float_class_normal
+             : float32_is_signaling_nan(b, status)
+             ? float_class_snan
+             : float_class_qnan);
 
-    aIsQuietNaN = float32_is_quiet_nan(a, status);
-    aIsSignalingNaN = float32_is_signaling_nan(a, status);
-    bIsQuietNaN = float32_is_quiet_nan(b, status);
-    bIsSignalingNaN = float32_is_signaling_nan(b, status);
     av = float32_val(a);
     bv = float32_val(b);
 
-    if (aIsSignalingNaN | bIsSignalingNaN) {
+    if (is_snan(a_cls) || is_snan(b_cls)) {
         float_raise(float_flag_invalid, status);
     }
 
@@ -779,14 +755,13 @@ static float32 propagateFloat32NaN(float32 a, float32 b, float_status *status)
         aIsLargerSignificand = (av < bv) ? 1 : 0;
     }
 
-    if (pickNaN(aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
-                aIsLargerSignificand)) {
-        if (bIsSignalingNaN) {
+    if (pickNaN(a_cls, b_cls, aIsLargerSignificand)) {
+        if (is_snan(b_cls)) {
             return float32_silence_nan(b, status);
         }
         return b;
     } else {
-        if (aIsSignalingNaN) {
+        if (is_snan(a_cls)) {
             return float32_silence_nan(a, status);
         }
         return a;
@@ -908,18 +883,26 @@ static float64 commonNaNToFloat64(commonNaNT a, float_status *status)
 
 static float64 propagateFloat64NaN(float64 a, float64 b, float_status *status)
 {
-    flag aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN;
     flag aIsLargerSignificand;
     uint64_t av, bv;
+    FloatClass a_cls, b_cls;
+
+    /* This is not complete, but is good enough for pickNaN.  */
+    a_cls = (!float64_is_any_nan(a)
+             ? float_class_normal
+             : float64_is_signaling_nan(a, status)
+             ? float_class_snan
+             : float_class_qnan);
+    b_cls = (!float64_is_any_nan(b)
+             ? float_class_normal
+             : float64_is_signaling_nan(b, status)
+             ? float_class_snan
+             : float_class_qnan);
 
-    aIsQuietNaN = float64_is_quiet_nan(a, status);
-    aIsSignalingNaN = float64_is_signaling_nan(a, status);
-    bIsQuietNaN = float64_is_quiet_nan(b, status);
-    bIsSignalingNaN = float64_is_signaling_nan(b, status);
     av = float64_val(a);
     bv = float64_val(b);
 
-    if (aIsSignalingNaN | bIsSignalingNaN) {
+    if (is_snan(a_cls) || is_snan(b_cls)) {
         float_raise(float_flag_invalid, status);
     }
 
@@ -935,14 +918,13 @@ static float64 propagateFloat64NaN(float64 a, float64 b, float_status *status)
         aIsLargerSignificand = (av < bv) ? 1 : 0;
     }
 
-    if (pickNaN(aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
-                aIsLargerSignificand)) {
-        if (bIsSignalingNaN) {
+    if (pickNaN(a_cls, b_cls, aIsLargerSignificand)) {
+        if (is_snan(b_cls)) {
             return float64_silence_nan(b, status);
         }
         return b;
     } else {
-        if (aIsSignalingNaN) {
+        if (is_snan(a_cls)) {
             return float64_silence_nan(a, status);
         }
         return a;
@@ -1075,15 +1057,22 @@ static floatx80 commonNaNToFloatx80(commonNaNT a, float_status *status)
 
 floatx80 propagateFloatx80NaN(floatx80 a, floatx80 b, float_status *status)
 {
-    flag aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN;
     flag aIsLargerSignificand;
+    FloatClass a_cls, b_cls;
 
-    aIsQuietNaN = floatx80_is_quiet_nan(a, status);
-    aIsSignalingNaN = floatx80_is_signaling_nan(a, status);
-    bIsQuietNaN = floatx80_is_quiet_nan(b, status);
-    bIsSignalingNaN = floatx80_is_signaling_nan(b, status);
+    /* This is not complete, but is good enough for pickNaN.  */
+    a_cls = (!floatx80_is_any_nan(a)
+             ? float_class_normal
+             : floatx80_is_signaling_nan(a, status)
+             ? float_class_snan
+             : float_class_qnan);
+    b_cls = (!floatx80_is_any_nan(b)
+             ? float_class_normal
+             : floatx80_is_signaling_nan(b, status)
+             ? float_class_snan
+             : float_class_qnan);
 
-    if (aIsSignalingNaN | bIsSignalingNaN) {
+    if (is_snan(a_cls) || is_snan(b_cls)) {
         float_raise(float_flag_invalid, status);
     }
 
@@ -1099,14 +1088,13 @@ floatx80 propagateFloatx80NaN(floatx80 a, floatx80 b, float_status *status)
         aIsLargerSignificand = (a.high < b.high) ? 1 : 0;
     }
 
-    if (pickNaN(aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
-                aIsLargerSignificand)) {
-        if (bIsSignalingNaN) {
+    if (pickNaN(a_cls, b_cls, aIsLargerSignificand)) {
+        if (is_snan(b_cls)) {
             return floatx80_silence_nan(b, status);
         }
         return b;
     } else {
-        if (aIsSignalingNaN) {
+        if (is_snan(a_cls)) {
             return floatx80_silence_nan(a, status);
         }
         return a;
@@ -1217,15 +1205,22 @@ static float128 commonNaNToFloat128(commonNaNT a, float_status *status)
 static float128 propagateFloat128NaN(float128 a, float128 b,
                                      float_status *status)
 {
-    flag aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN;
     flag aIsLargerSignificand;
+    FloatClass a_cls, b_cls;
 
-    aIsQuietNaN = float128_is_quiet_nan(a, status);
-    aIsSignalingNaN = float128_is_signaling_nan(a, status);
-    bIsQuietNaN = float128_is_quiet_nan(b, status);
-    bIsSignalingNaN = float128_is_signaling_nan(b, status);
+    /* This is not complete, but is good enough for pickNaN.  */
+    a_cls = (!float128_is_any_nan(a)
+             ? float_class_normal
+             : float128_is_signaling_nan(a, status)
+             ? float_class_snan
+             : float_class_qnan);
+    b_cls = (!float128_is_any_nan(b)
+             ? float_class_normal
+             : float128_is_signaling_nan(b, status)
+             ? float_class_snan
+             : float_class_qnan);
 
-    if (aIsSignalingNaN | bIsSignalingNaN) {
+    if (is_snan(a_cls) || is_snan(b_cls)) {
         float_raise(float_flag_invalid, status);
     }
 
@@ -1241,14 +1236,13 @@ static float128 propagateFloat128NaN(float128 a, float128 b,
         aIsLargerSignificand = (a.high < b.high) ? 1 : 0;
     }
 
-    if (pickNaN(aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
-                aIsLargerSignificand)) {
-        if (bIsSignalingNaN) {
+    if (pickNaN(a_cls, b_cls, aIsLargerSignificand)) {
+        if (is_snan(b_cls)) {
             return float128_silence_nan(b, status);
         }
         return b;
     } else {
-        if (aIsSignalingNaN) {
+        if (is_snan(a_cls)) {
             return float128_silence_nan(a, status);
         }
         return a;
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index bafb3b17ec..593869e5a7 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -580,8 +580,7 @@ static FloatParts pick_nan(FloatParts a, FloatParts b, float_status *s)
     if (s->default_nan_mode) {
         return parts_default_nan(s);
     } else {
-        if (pickNaN(is_qnan(a.cls), is_snan(a.cls),
-                    is_qnan(b.cls), is_snan(b.cls),
+        if (pickNaN(a.cls, b.cls,
                     a.frac > b.frac ||
                     (a.frac == b.frac && a.sign < b.sign))) {
             a = b;
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 26/27] fpu/softfloat: Pass FloatClass to pickNaN

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> For each operand, pass a single enumeration instead of a pair of booleans.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h | 168 ++++++++++++++++++-------------------
>  fpu/softfloat.c            |   3 +-
>  2 files changed, 82 insertions(+), 89 deletions(-)

The commit also merges multiple different ifdef-selected implementations
of pickNaN into a single function whose body is ifdef-selected.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v2 27/27] fpu/softfloat: Pass FloatClass to pickNaNMulAdd

[Richard Henderson]

For each operand, pass a single enumeration instead of a pair of booleans.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat-specialize.h | 70 +++++++++++++++-----------------------
 fpu/softfloat.c            |  5 +--
 2 files changed, 28 insertions(+), 47 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 47c67caec6..85c59c20d8 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -594,15 +594,14 @@ static int pickNaN(FloatClass a_cls, FloatClass b_cls,
 | information.
 | Return values : 0 : a; 1 : b; 2 : c; 3 : default-NaN
 *----------------------------------------------------------------------------*/
-#if defined(TARGET_ARM)
-static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-                         flag cIsQNaN, flag cIsSNaN, flag infzero,
-                         float_status *status)
+static int pickNaNMulAdd(FloatClass a_cls, FloatClass b_cls, FloatClass c_cls,
+                         bool infzero, float_status *status)
 {
+#if defined(TARGET_ARM)
     /* For ARM, the (inf,zero,qnan) case sets InvalidOp and returns
      * the default NaN
      */
-    if (infzero && cIsQNaN) {
+    if (infzero && is_qnan(c_cls)) {
         float_raise(float_flag_invalid, status);
         return 3;
     }
@@ -610,25 +609,20 @@ static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
     /* This looks different from the ARM ARM pseudocode, because the ARM ARM
      * puts the operands to a fused mac operation (a*b)+c in the order c,a,b.
      */
-    if (cIsSNaN) {
+    if (is_snan(c_cls)) {
         return 2;
-    } else if (aIsSNaN) {
+    } else if (is_snan(a_cls)) {
         return 0;
-    } else if (bIsSNaN) {
+    } else if (is_snan(b_cls)) {
         return 1;
-    } else if (cIsQNaN) {
+    } else if (is_qnan(c_cls)) {
         return 2;
-    } else if (aIsQNaN) {
+    } else if (is_qnan(a_cls)) {
         return 0;
     } else {
         return 1;
     }
-}
 #elif defined(TARGET_MIPS)
-static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-                         flag cIsQNaN, flag cIsSNaN, flag infzero,
-                         float_status *status)
-{
     /* For MIPS, the (inf,zero,qnan) case sets InvalidOp and returns
      * the default NaN
      */
@@ -639,41 +633,36 @@ static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
 
     if (snan_bit_is_one(status)) {
         /* Prefer sNaN over qNaN, in the a, b, c order. */
-        if (aIsSNaN) {
+        if (is_snan(a_cls)) {
             return 0;
-        } else if (bIsSNaN) {
+        } else if (is_snan(b_cls)) {
             return 1;
-        } else if (cIsSNaN) {
+        } else if (is_snan(c_cls)) {
             return 2;
-        } else if (aIsQNaN) {
+        } else if (is_qnan(a_cls)) {
             return 0;
-        } else if (bIsQNaN) {
+        } else if (is_qnan(b_cls)) {
             return 1;
         } else {
             return 2;
         }
     } else {
         /* Prefer sNaN over qNaN, in the c, a, b order. */
-        if (cIsSNaN) {
+        if (is_snan(c_cls)) {
             return 2;
-        } else if (aIsSNaN) {
+        } else if (is_snan(a_cls)) {
             return 0;
-        } else if (bIsSNaN) {
+        } else if (is_snan(b_cls)) {
             return 1;
-        } else if (cIsQNaN) {
+        } else if (is_qnan(c_cls)) {
             return 2;
-        } else if (aIsQNaN) {
+        } else if (is_qnan(a_cls)) {
             return 0;
         } else {
             return 1;
         }
     }
-}
 #elif defined(TARGET_PPC)
-static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-                         flag cIsQNaN, flag cIsSNaN, flag infzero,
-                         float_status *status)
-{
     /* For PPC, the (inf,zero,qnan) case sets InvalidOp, but we prefer
      * to return an input NaN if we have one (ie c) rather than generating
      * a default NaN
@@ -686,31 +675,26 @@ static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
     /* If fRA is a NaN return it; otherwise if fRB is a NaN return it;
      * otherwise return fRC. Note that muladd on PPC is (fRA * fRC) + frB
      */
-    if (aIsSNaN || aIsQNaN) {
+    if (is_nan(a_cls)) {
         return 0;
-    } else if (cIsSNaN || cIsQNaN) {
+    } else if (is_nan(c_cls)) {
         return 2;
     } else {
         return 1;
     }
-}
 #else
-/* A default implementation: prefer a to b to c.
- * This is unlikely to actually match any real implementation.
- */
-static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-                         flag cIsQNaN, flag cIsSNaN, flag infzero,
-                         float_status *status)
-{
-    if (aIsSNaN || aIsQNaN) {
+    /* A default implementation: prefer a to b to c.
+     * This is unlikely to actually match any real implementation.
+     */
+    if (is_nan(a_cls)) {
         return 0;
-    } else if (bIsSNaN || bIsQNaN) {
+    } else if (is_nan(b_cls)) {
         return 1;
     } else {
         return 2;
     }
-}
 #endif
+}
 
 /*----------------------------------------------------------------------------
 | Takes two single-precision floating-point values `a' and `b', one of which
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 593869e5a7..383f2f5f99 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -601,10 +601,7 @@ static FloatParts pick_nan_muladd(FloatParts a, FloatParts b, FloatParts c,
         s->float_exception_flags |= float_flag_invalid;
     }
 
-    which = pickNaNMulAdd(is_qnan(a.cls), is_snan(a.cls),
-                          is_qnan(b.cls), is_snan(b.cls),
-                          is_qnan(c.cls), is_snan(c.cls),
-                          inf_zero, s);
+    which = pickNaNMulAdd(a.cls, b.cls, c.cls, inf_zero, s);
 
     if (s->default_nan_mode) {
         /* Note that this check is after pickNaNMulAdd so that function
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 27/27] fpu/softfloat: Pass FloatClass to pickNaNMulAdd

[Peter Maydell]

On 12 May 2018 at 01:43, Richard Henderson <richard.henderson@linaro.org> wrote:
> For each operand, pass a single enumeration instead of a pair of booleans.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-specialize.h | 70 +++++++++++++++-----------------------
>  fpu/softfloat.c            |  5 +--
>  2 files changed, 28 insertions(+), 47 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 00/27] softfloat patch roundup

[Peter Maydell]

On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> This is my SNaN patch set, Alex's float-float refactor, and a couple
> of other random outstanding fpu patches.  This has been reordered so
> as to be bisectable, since the float-float refactor requires the snan
> work to avoid breakage.
>
> It probably doesn't matter, but this was built on top of my arm fp16
> patch set simply to make it easier to test.  The whole tree is
>
>   git://github.com/rth7680/qemu.git fpu-roundup

I've added patches 1, 2, and 15 to target-arm.next:
>   fpu/softfloat: int_to_float ensure r fully initialised
>   fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)
>   target/arm: Fix fp_status_f16 tininess before rounding

thanks
-- PMM