[Qemu-devel] [PATCH v1 0/3] target/ppc: floating point multiply-add fixes

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH v1 0/3] target/ppc: floating point multiply-add fixes
@ 2017-03-04 14:02 Nikunj A Dadhania
  2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 1/3] target/ppc: fmadd check for excp independently Nikunj A Dadhania
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Nikunj A Dadhania @ 2017-03-04 14:02 UTC (permalink / raw)
  To: qemu-ppc, david, rth; +Cc: qemu-devel, bharata, nikunj

Exception handling in fmadd/fmsub/fnmadd/fnmsub isnt correct as the 
order of checking could give wrong settings in FPSCR.

For example, (x * y) + z, if x = infinity, y = zero and z = snan. 
After the execution of instruction VXNAN and VXIMZ both should be set. For 
this correct the ordering in the float64_maddsub_update_excp() as follows:

      * If x, y or z is an SNaN, vxsnan_flag is set to 1.
      * If x is a Zero and y, is an Infinity or x is an Infinity and y is
        an Zero, vximz_flag is set to 1.
      * If the product of x and y is an Infinity and z is an Infinity of
        the opposite sign, vxisi_flag is set to 1.

Moreover, all vector multiply-add/substract and vector scalar multiply-add/sub 
had the bug. VXISI should be set only when (∞ - ∞), where as the instruction was
doing it wrong, was just checking ((a == ∞ OR b == ∞) && (c == ∞)). There are two 
issues here:

1. infinity can be +ve or -ve, i.e.  (∞ + (-∞)), should result in setting VXISI
2. Need to take care of the operation (add or sub). (∞ + ∞) should not set VXISI

Patch:
01: Fixes the order of checking and makes them independent as per 
    the ISA

02: Introduces the macro to be used by Vector Scalar and Vector Multiply-Add

03: Make Vector Scalar and Vector Multiply Add use the macro for updating 
    exception flags 

Changelog:
v0:
* Use ALL CAPS for macro arguments
* Correct the argument type in the function

Nikunj A Dadhania (3):
  target/ppc: fmadd check for excp independently
  target/ppc: fmadd: add macro for updating flags
  target/ppc: use helper for excp handling

 target/ppc/fpu_helper.c | 77 ++++++++++++++++++++-----------------------------
 1 file changed, 32 insertions(+), 45 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v1 1/3] target/ppc: fmadd check for excp independently
  2017-03-04 14:02 [Qemu-devel] [PATCH v1 0/3] target/ppc: floating point multiply-add fixes Nikunj A Dadhania
@ 2017-03-04 14:02 ` Nikunj A Dadhania
  2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 2/3] target/ppc: fmadd: add macro for updating flags Nikunj A Dadhania
  2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 3/3] target/ppc: use helper for excp handling Nikunj A Dadhania
  2 siblings, 0 replies; 4+ messages in thread
From: Nikunj A Dadhania @ 2017-03-04 14:02 UTC (permalink / raw)
  To: qemu-ppc, david, rth; +Cc: qemu-devel, bharata, nikunj

Current order of checking does not confirm with the spec
(ISA 3.0: MultiplyAddDP page-469). Change the order and make them
independent of each other.

For example: a = infinity, b = zero, c = SNaN, this should set both
VXIMZ and VXNAN

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/fpu_helper.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 0535ad0..a547f58 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -747,17 +747,21 @@ static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
                                         float64 arg2, float64 arg3,
                                         unsigned int madd_flags)
 {
+    if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
+                 float64_is_signaling_nan(arg2, &env->fp_status) ||
+                 float64_is_signaling_nan(arg3, &env->fp_status))) {
+        /* sNaN operation */
+        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
+    }
+
     if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
                  (float64_is_zero(arg1) && float64_is_infinity(arg2)))) {
         /* Multiplication of zero by infinity */
-        arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
-    } else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
-                        float64_is_signaling_nan(arg2, &env->fp_status) ||
-                        float64_is_signaling_nan(arg3, &env->fp_status))) {
-        /* sNaN operation */
-        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
-    } else if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
-               float64_is_infinity(arg3)) {
+        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
+    }
+
+    if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
+        float64_is_infinity(arg3)) {
         uint8_t aSign, bSign, cSign;
 
         aSign = float64_is_neg(arg1);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v1 2/3] target/ppc: fmadd: add macro for updating flags
  2017-03-04 14:02 [Qemu-devel] [PATCH v1 0/3] target/ppc: floating point multiply-add fixes Nikunj A Dadhania
  2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 1/3] target/ppc: fmadd check for excp independently Nikunj A Dadhania
@ 2017-03-04 14:02 ` Nikunj A Dadhania
  2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 3/3] target/ppc: use helper for excp handling Nikunj A Dadhania
  2 siblings, 0 replies; 4+ messages in thread
From: Nikunj A Dadhania @ 2017-03-04 14:02 UTC (permalink / raw)
  To: qemu-ppc, david, rth; +Cc: qemu-devel, bharata, nikunj

Adds FPU_MADDSUB_UPDATE macro, this will be used for other routines
having float32/16

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/fpu_helper.c | 61 ++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index a547f58..13bd6ce 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -743,38 +743,37 @@ uint64_t helper_frim(CPUPPCState *env, uint64_t arg)
     return do_fri(env, arg, float_round_down);
 }
 
-static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
-                                        float64 arg2, float64 arg3,
-                                        unsigned int madd_flags)
-{
-    if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
-                 float64_is_signaling_nan(arg2, &env->fp_status) ||
-                 float64_is_signaling_nan(arg3, &env->fp_status))) {
-        /* sNaN operation */
-        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
-    }
-
-    if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
-                 (float64_is_zero(arg1) && float64_is_infinity(arg2)))) {
-        /* Multiplication of zero by infinity */
-        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
-    }
-
-    if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
-        float64_is_infinity(arg3)) {
-        uint8_t aSign, bSign, cSign;
-
-        aSign = float64_is_neg(arg1);
-        bSign = float64_is_neg(arg2);
-        cSign = float64_is_neg(arg3);
-        if (madd_flags & float_muladd_negate_c) {
-            cSign ^= 1;
-        }
-        if (aSign ^ bSign ^ cSign) {
-            float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
-        }
-    }
+#define FPU_MADDSUB_UPDATE(NAME, TP)                                    \
+static void NAME(CPUPPCState *env, TP arg1, TP arg2, TP arg3,           \
+                 unsigned int madd_flags)                               \
+{                                                                       \
+    if (TP##_is_signaling_nan(arg1, &env->fp_status) ||                 \
+        TP##_is_signaling_nan(arg2, &env->fp_status) ||                 \
+        TP##_is_signaling_nan(arg3, &env->fp_status)) {                 \
+        /* sNaN operation */                                            \
+        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);          \
+    }                                                                   \
+    if ((TP##_is_infinity(arg1) && TP##_is_zero(arg2)) ||               \
+        (TP##_is_zero(arg1) && TP##_is_infinity(arg2))) {               \
+        /* Multiplication of zero by infinity */                        \
+        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);           \
+    }                                                                   \
+    if ((TP##_is_infinity(arg1) || TP##_is_infinity(arg2)) &&           \
+        TP##_is_infinity(arg3)) {                                       \
+        uint8_t aSign, bSign, cSign;                                    \
+                                                                        \
+        aSign = TP##_is_neg(arg1);                                      \
+        bSign = TP##_is_neg(arg2);                                      \
+        cSign = TP##_is_neg(arg3);                                      \
+        if (madd_flags & float_muladd_negate_c) {                       \
+            cSign ^= 1;                                                 \
+        }                                                               \
+        if (aSign ^ bSign ^ cSign) {                                    \
+            float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);       \
+        }                                                               \
+    }                                                                   \
 }
+FPU_MADDSUB_UPDATE(float64_maddsub_update_excp, float64)
 
 #define FPU_FMADD(op, madd_flags)                                       \
 uint64_t helper_##op(CPUPPCState *env, uint64_t arg1,                   \
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v1 3/3] target/ppc: use helper for excp handling
  2017-03-04 14:02 [Qemu-devel] [PATCH v1 0/3] target/ppc: floating point multiply-add fixes Nikunj A Dadhania
  2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 1/3] target/ppc: fmadd check for excp independently Nikunj A Dadhania
  2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 2/3] target/ppc: fmadd: add macro for updating flags Nikunj A Dadhania
@ 2017-03-04 14:02 ` Nikunj A Dadhania
  2 siblings, 0 replies; 4+ messages in thread
From: Nikunj A Dadhania @ 2017-03-04 14:02 UTC (permalink / raw)
  To: qemu-ppc, david, rth; +Cc: qemu-devel, bharata, nikunj

Use the helper routine float[32,64]_maddsub_update_excp() in VSX_MADD
macro.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/fpu_helper.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 13bd6ce..c4dab15 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -773,6 +773,7 @@ static void NAME(CPUPPCState *env, TP arg1, TP arg2, TP arg3,           \
         }                                                               \
     }                                                                   \
 }
+FPU_MADDSUB_UPDATE(float32_maddsub_update_excp, float32)
 FPU_MADDSUB_UPDATE(float64_maddsub_update_excp, float64)
 
 #define FPU_FMADD(op, madd_flags)                                       \
@@ -2239,24 +2240,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)                           \
         env->fp_status.float_exception_flags |= tstat.float_exception_flags;  \
                                                                               \
         if (unlikely(tstat.float_exception_flags & float_flag_invalid)) {     \
-            if (tp##_is_signaling_nan(xa.fld, &tstat) ||                      \
-                tp##_is_signaling_nan(b->fld, &tstat) ||                      \
-                tp##_is_signaling_nan(c->fld, &tstat)) {                      \
-                float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, sfprf);    \
-                tstat.float_exception_flags &= ~float_flag_invalid;           \
-            }                                                                 \
-            if ((tp##_is_infinity(xa.fld) && tp##_is_zero(b->fld)) ||         \
-                (tp##_is_zero(xa.fld) && tp##_is_infinity(b->fld))) {         \
-                xt_out.fld = float64_to_##tp(float_invalid_op_excp(env,       \
-                    POWERPC_EXCP_FP_VXIMZ, sfprf), &env->fp_status);          \
-                tstat.float_exception_flags &= ~float_flag_invalid;           \
-            }                                                                 \
-            if ((tstat.float_exception_flags & float_flag_invalid) &&         \
-                ((tp##_is_infinity(xa.fld) ||                                 \
-                  tp##_is_infinity(b->fld)) &&                                \
-                  tp##_is_infinity(c->fld))) {                                \
-                float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, sfprf);     \
-            }                                                                 \
+            tp##_maddsub_update_excp(env, xa.fld, b->fld, c->fld, maddflgs);  \
         }                                                                     \
                                                                               \
         if (r2sp) {                                                           \
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-03-04 14:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-04 14:02 [Qemu-devel] [PATCH v1 0/3] target/ppc: floating point multiply-add fixes Nikunj A Dadhania
2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 1/3] target/ppc: fmadd check for excp independently Nikunj A Dadhania
2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 2/3] target/ppc: fmadd: add macro for updating flags Nikunj A Dadhania
2017-03-04 14:02 ` [Qemu-devel] [PATCH v1 3/3] target/ppc: use helper for excp handling Nikunj A Dadhania

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).