[Qemu-devel] [PATCH] ppc: Fix xscvdpspn for SNAN

Paul A. Clarke posted 1 patch 4 years, 7 months ago
Failed in applying to current master (apply log)
target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH] ppc: Fix xscvdpspn for SNAN
Posted by Paul A. Clarke 4 years, 7 months ago
From: "Paul A. Clarke" <pc@us.ibm.com>

helper_xscvdpspn() uses float64_to_float32() to convert double-precision
floating-point to single-precision.  Unfortunately, float64_to_float32()
converts SNAN to QNAN, which should not happen with xscvdpspn.

float64_to_float32() is also used by other instruction implementations
for conversions which _should_ convert SNAN to QNAN.

Rather than trying to wedge code to preserve SNAN in float64_to_float32()
just for this this one case, I instead embed an embodiment of the
conversion code outlined in the POWER ISA for xscvdpspn.

Signed-off-by: Paul A. Clarke <pc@us.ibm.com>
---
 target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 07bc905..c8e7192 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2887,12 +2887,40 @@ void helper_xscvqpdp(CPUPPCState *env, uint32_t opcode,
 
 uint64_t helper_xscvdpspn(CPUPPCState *env, uint64_t xb)
 {
-    uint64_t result;
+    uint64_t result, sign, exp, frac;
 
     float_status tstat = env->fp_status;
     set_float_exception_flags(0, &tstat);
 
-    result = (uint64_t)float64_to_float32(xb, &tstat);
+    sign = extract64(xb, 63,  1);
+    exp  = extract64(xb, 52, 11);
+    frac = extract64(xb,  0, 52) | 0x10000000000000ULL;
+
+    if (unlikely(exp == 0 && extract64(frac, 0, 52) != 0)) {
+        /* DP denormal operand.  */
+        /* Exponent override to DP min exp.  */
+        exp = 1;
+        /* Implicit bit override to 0.  */
+        frac = deposit64(frac, 53, 1, 0);
+    }
+
+    if (unlikely(exp < 897 && frac != 0)) {
+        /* SP tiny operand.  */
+        if (897 - exp > 63) {
+            frac = 0;
+        } else {
+            /* Denormalize until exp = SP min exp.  */
+            frac >>= (897 - exp);
+        }
+        /* Exponent override to SP min exp - 1.  */
+        exp = 896;
+    }
+
+    result = sign << 31;
+    result |= extract64(exp, 10, 1) << 30;
+    result |= extract64(exp, 0, 7) << 23;
+    result |= extract64(frac, 29, 23);
+
     /* hardware replicates result to both words of the doubleword result.  */
     return (result << 32) | result;
 }
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH] ppc: Fix xscvdpspn for SNAN
Posted by David Gibson 4 years, 7 months ago
On Tue, Aug 20, 2019 at 12:26:04PM -0500, Paul A. Clarke wrote:
> From: "Paul A. Clarke" <pc@us.ibm.com>
> 
> helper_xscvdpspn() uses float64_to_float32() to convert double-precision
> floating-point to single-precision.  Unfortunately, float64_to_float32()
> converts SNAN to QNAN, which should not happen with xscvdpspn.
> 
> float64_to_float32() is also used by other instruction implementations
> for conversions which _should_ convert SNAN to QNAN.
> 
> Rather than trying to wedge code to preserve SNAN in float64_to_float32()
> just for this this one case, I instead embed an embodiment of the
> conversion code outlined in the POWER ISA for xscvdpspn.
> 
> Signed-off-by: Paul A. Clarke <pc@us.ibm.com>

Applied to ppc-for-4.2.  I used rth's description rather than the one
above, since I found it clearer.

> ---
>  target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index 07bc905..c8e7192 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -2887,12 +2887,40 @@ void helper_xscvqpdp(CPUPPCState *env, uint32_t opcode,
>  
>  uint64_t helper_xscvdpspn(CPUPPCState *env, uint64_t xb)
>  {
> -    uint64_t result;
> +    uint64_t result, sign, exp, frac;
>  
>      float_status tstat = env->fp_status;
>      set_float_exception_flags(0, &tstat);
>  
> -    result = (uint64_t)float64_to_float32(xb, &tstat);
> +    sign = extract64(xb, 63,  1);
> +    exp  = extract64(xb, 52, 11);
> +    frac = extract64(xb,  0, 52) | 0x10000000000000ULL;
> +
> +    if (unlikely(exp == 0 && extract64(frac, 0, 52) != 0)) {
> +        /* DP denormal operand.  */
> +        /* Exponent override to DP min exp.  */
> +        exp = 1;
> +        /* Implicit bit override to 0.  */
> +        frac = deposit64(frac, 53, 1, 0);
> +    }
> +
> +    if (unlikely(exp < 897 && frac != 0)) {
> +        /* SP tiny operand.  */
> +        if (897 - exp > 63) {
> +            frac = 0;
> +        } else {
> +            /* Denormalize until exp = SP min exp.  */
> +            frac >>= (897 - exp);
> +        }
> +        /* Exponent override to SP min exp - 1.  */
> +        exp = 896;
> +    }
> +
> +    result = sign << 31;
> +    result |= extract64(exp, 10, 1) << 30;
> +    result |= extract64(exp, 0, 7) << 23;
> +    result |= extract64(frac, 29, 23);
> +
>      /* hardware replicates result to both words of the doubleword result.  */
>      return (result << 32) | result;
>  }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH] ppc: Fix xscvdpspn for SNAN
Posted by Richard Henderson 4 years, 7 months ago
On 8/20/19 10:26 AM, Paul A. Clarke wrote:
> From: "Paul A. Clarke" <pc@us.ibm.com>
> 
> helper_xscvdpspn() uses float64_to_float32() to convert double-precision
> floating-point to single-precision.  Unfortunately, float64_to_float32()
> converts SNAN to QNAN, which should not happen with xscvdpspn.
> 
> float64_to_float32() is also used by other instruction implementations
> for conversions which _should_ convert SNAN to QNAN.
> 
> Rather than trying to wedge code to preserve SNAN in float64_to_float32()
> just for this this one case, I instead embed an embodiment of the
> conversion code outlined in the POWER ISA for xscvdpspn.
> 
> Signed-off-by: Paul A. Clarke <pc@us.ibm.com>
> ---
>  target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)

Perhaps a better description is:

---

The xscvdpspn instruction implements a non-arithmetic conversion.
In particular, NaNs are not silenced and rounding is not performed.

Rewrite to match the pseudocode for ConvertDPtoSP_NS() in the
Power 3.0B manual.

---

The term comes from the ieee spec, in that "arithmetic" operations (like add)
are required to notice exceptional conditions like NaN but "non-arithmetic"
operations (like abs) are not required to do so.  Thus a valid implementation
of abs merely clears the sign bit without otherwise transforming NaNs.

The code does match the pseudocode, so
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~