All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] ppc/translate: Fix unordered f64/f128 comparisons
@ 2020-11-10 10:53 LemonBoy
  2020-11-10 10:53 ` [PATCH v2 1/4] " LemonBoy
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: LemonBoy @ 2020-11-10 10:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: LemonBoy, richard.henderson, qemu-ppc, david

Fix a couple of problems found in the emulation of f64/f128 comparisons plus
some minimal self-contained commits to clean-up some code.

Is it too late to ask for inclusion in the upcoming release?

Giuseppe Musacchio (4):
  ppc/translate: Fix unordered f64/f128 comparisons
  ppc/translate: Turn the helper macros into functions
  ppc/translate: Delay NaN checking after comparison
  ppc/translate: Raise exceptions after setting the cc

 target/ppc/fpu_helper.c | 212 +++++++++++++++++++++++-----------------
 1 file changed, 121 insertions(+), 91 deletions(-)

-- 
2.27.0



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

* [PATCH v2 1/4] ppc/translate: Fix unordered f64/f128 comparisons
  2020-11-10 10:53 [PATCH v2 0/4] ppc/translate: Fix unordered f64/f128 comparisons LemonBoy
@ 2020-11-10 10:53 ` LemonBoy
  2020-11-12 21:26   ` Richard Henderson
  2020-11-10 10:53 ` [PATCH v2 2/4] ppc/translate: Turn the helper macros into functions LemonBoy
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: LemonBoy @ 2020-11-10 10:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: LemonBoy, richard.henderson, qemu-ppc, david

According to the PowerISA v3.1 reference, Table 68 "Actions for xscmpudp
- Part 1: Compare Unordered", whenever one of the two operands is a NaN
the SO bit is set while the other three bits are cleared.

Apply the same change to xscmpuqp.

The respective ordered counterparts are unaffected.

Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
---
 target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 9b8c8b70b6..b07ff66375 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2479,13 +2479,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,                      \
     if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) ||        \
         float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) {        \
         vxsnan_flag = true;                                              \
-        cc = CRF_SO;                                                     \
         if (fpscr_ve == 0 && ordered) {                                  \
             vxvc_flag = true;                                            \
         }                                                                \
     } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) ||     \
                float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) {     \
-        cc = CRF_SO;                                                     \
         if (ordered) {                                                   \
             vxvc_flag = true;                                            \
         }                                                                \
@@ -2497,12 +2495,19 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,                      \
         float_invalid_op_vxvc(env, 0, GETPC());                          \
     }                                                                    \
                                                                          \
-    if (float64_lt(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {         \
+    switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {\
+    case float_relation_less:                                            \
         cc |= CRF_LT;                                                    \
-    } else if (!float64_le(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
-        cc |= CRF_GT;                                                    \
-    } else {                                                             \
+        break;                                                           \
+    case float_relation_equal:                                           \
         cc |= CRF_EQ;                                                    \
+        break;                                                           \
+    case float_relation_greater:                                         \
+        cc |= CRF_GT;                                                    \
+        break;                                                           \
+    case float_relation_unordered:                                       \
+        cc |= CRF_SO;                                                    \
+        break;                                                           \
     }                                                                    \
                                                                          \
     env->fpscr &= ~FP_FPCC;                                              \
@@ -2545,12 +2550,19 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,                     \
         float_invalid_op_vxvc(env, 0, GETPC());                         \
     }                                                                   \
                                                                         \
-    if (float128_lt(xa->f128, xb->f128, &env->fp_status)) {             \
+    switch (float128_compare(xa->f128, xb->f128, &env->fp_status)) {    \
+    case float_relation_less:                                           \
         cc |= CRF_LT;                                                   \
-    } else if (!float128_le(xa->f128, xb->f128, &env->fp_status)) {     \
-        cc |= CRF_GT;                                                   \
-    } else {                                                            \
+        break;                                                          \
+    case float_relation_equal:                                          \
         cc |= CRF_EQ;                                                   \
+        break;                                                          \
+    case float_relation_greater:                                        \
+        cc |= CRF_GT;                                                   \
+        break;                                                          \
+    case float_relation_unordered:                                      \
+        cc |= CRF_SO;                                                   \
+        break;                                                          \
     }                                                                   \
                                                                         \
     env->fpscr &= ~FP_FPCC;                                             \
-- 
2.27.0



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

* [PATCH v2 2/4] ppc/translate: Turn the helper macros into functions
  2020-11-10 10:53 [PATCH v2 0/4] ppc/translate: Fix unordered f64/f128 comparisons LemonBoy
  2020-11-10 10:53 ` [PATCH v2 1/4] " LemonBoy
@ 2020-11-10 10:53 ` LemonBoy
  2020-11-12 21:27   ` Richard Henderson
  2020-11-10 10:53 ` [PATCH v2 3/4] ppc/translate: Delay NaN checking after comparison LemonBoy
  2020-11-10 10:53 ` [PATCH v2 4/4] ppc/translate: Raise exceptions after setting the cc LemonBoy
  3 siblings, 1 reply; 9+ messages in thread
From: LemonBoy @ 2020-11-10 10:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: LemonBoy, richard.henderson, qemu-ppc, david

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
---
 target/ppc/fpu_helper.c | 220 +++++++++++++++++++++-------------------
 1 file changed, 118 insertions(+), 102 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index b07ff66375..c74c080c17 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2467,113 +2467,129 @@ void helper_xscmpexpqp(CPUPPCState *env, uint32_t opcode,
     do_float_check_status(env, GETPC());
 }
 
-#define VSX_SCALAR_CMP(op, ordered)                                      \
-void helper_##op(CPUPPCState *env, uint32_t opcode,                      \
-                 ppc_vsr_t *xa, ppc_vsr_t *xb)                           \
-{                                                                        \
-    uint32_t cc = 0;                                                     \
-    bool vxsnan_flag = false, vxvc_flag = false;                         \
-                                                                         \
-    helper_reset_fpstatus(env);                                          \
-                                                                         \
-    if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) ||        \
-        float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) {        \
-        vxsnan_flag = true;                                              \
-        if (fpscr_ve == 0 && ordered) {                                  \
-            vxvc_flag = true;                                            \
-        }                                                                \
-    } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) ||     \
-               float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) {     \
-        if (ordered) {                                                   \
-            vxvc_flag = true;                                            \
-        }                                                                \
-    }                                                                    \
-    if (vxsnan_flag) {                                                   \
-        float_invalid_op_vxsnan(env, GETPC());                           \
-    }                                                                    \
-    if (vxvc_flag) {                                                     \
-        float_invalid_op_vxvc(env, 0, GETPC());                          \
-    }                                                                    \
-                                                                         \
-    switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {\
-    case float_relation_less:                                            \
-        cc |= CRF_LT;                                                    \
-        break;                                                           \
-    case float_relation_equal:                                           \
-        cc |= CRF_EQ;                                                    \
-        break;                                                           \
-    case float_relation_greater:                                         \
-        cc |= CRF_GT;                                                    \
-        break;                                                           \
-    case float_relation_unordered:                                       \
-        cc |= CRF_SO;                                                    \
-        break;                                                           \
-    }                                                                    \
-                                                                         \
-    env->fpscr &= ~FP_FPCC;                                              \
-    env->fpscr |= cc << FPSCR_FPCC;                                      \
-    env->crf[BF(opcode)] = cc;                                           \
-                                                                         \
-    do_float_check_status(env, GETPC());                                 \
+static inline void do_scalar_cmp(CPUPPCState *env, ppc_vsr_t *xa, ppc_vsr_t *xb,
+                                 int crf_idx, bool ordered)
+{
+    uint32_t cc = 0;
+    bool vxsnan_flag = false, vxvc_flag = false;
+
+    helper_reset_fpstatus(env);
+
+    if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) ||
+        float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) {
+        vxsnan_flag = true;
+        if (fpscr_ve == 0 && ordered) {
+            vxvc_flag = true;
+        }
+    } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) ||
+               float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) {
+        if (ordered) {
+            vxvc_flag = true;
+        }
+    }
+    if (vxsnan_flag) {
+        float_invalid_op_vxsnan(env, GETPC());
+    }
+    if (vxvc_flag) {
+        float_invalid_op_vxvc(env, 0, GETPC());
+    }
+
+    switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {
+    case float_relation_less:
+        cc |= CRF_LT;
+        break;
+    case float_relation_equal:
+        cc |= CRF_EQ;
+        break;
+    case float_relation_greater:
+        cc |= CRF_GT;
+        break;
+    case float_relation_unordered:
+        cc |= CRF_SO;
+        break;
+    }
+
+    env->fpscr &= ~FP_FPCC;
+    env->fpscr |= cc << FPSCR_FPCC;
+    env->crf[crf_idx] = cc;
+
+    do_float_check_status(env, GETPC());
+}
+
+void helper_xscmpodp(CPUPPCState *env, uint32_t opcode, ppc_vsr_t *xa,
+                     ppc_vsr_t *xb)
+{
+    do_scalar_cmp(env, xa, xb, BF(opcode), true);
+}
+
+void helper_xscmpudp(CPUPPCState *env, uint32_t opcode, ppc_vsr_t *xa,
+                     ppc_vsr_t *xb)
+{
+    do_scalar_cmp(env, xa, xb, BF(opcode), false);
 }
 
-VSX_SCALAR_CMP(xscmpodp, 1)
-VSX_SCALAR_CMP(xscmpudp, 0)
+static inline void do_scalar_cmpq(CPUPPCState *env, ppc_vsr_t *xa,
+                                  ppc_vsr_t *xb, int crf_idx, bool ordered)
+{
+    uint32_t cc = 0;
+    bool vxsnan_flag = false, vxvc_flag = false;
+
+    helper_reset_fpstatus(env);
+
+    if (float128_is_signaling_nan(xa->f128, &env->fp_status) ||
+        float128_is_signaling_nan(xb->f128, &env->fp_status)) {
+        vxsnan_flag = true;
+        cc = CRF_SO;
+        if (fpscr_ve == 0 && ordered) {
+            vxvc_flag = true;
+        }
+    } else if (float128_is_quiet_nan(xa->f128, &env->fp_status) ||
+               float128_is_quiet_nan(xb->f128, &env->fp_status)) {
+        cc = CRF_SO;
+        if (ordered) {
+            vxvc_flag = true;
+        }
+    }
+    if (vxsnan_flag) {
+        float_invalid_op_vxsnan(env, GETPC());
+    }
+    if (vxvc_flag) {
+        float_invalid_op_vxvc(env, 0, GETPC());
+    }
+
+    switch (float128_compare(xa->f128, xb->f128, &env->fp_status)) {
+    case float_relation_less:
+        cc |= CRF_LT;
+        break;
+    case float_relation_equal:
+        cc |= CRF_EQ;
+        break;
+    case float_relation_greater:
+        cc |= CRF_GT;
+        break;
+    case float_relation_unordered:
+        cc |= CRF_SO;
+        break;
+    }
 
-#define VSX_SCALAR_CMPQ(op, ordered)                                    \
-void helper_##op(CPUPPCState *env, uint32_t opcode,                     \
-                 ppc_vsr_t *xa, ppc_vsr_t *xb)                          \
-{                                                                       \
-    uint32_t cc = 0;                                                    \
-    bool vxsnan_flag = false, vxvc_flag = false;                        \
-                                                                        \
-    helper_reset_fpstatus(env);                                         \
-                                                                        \
-    if (float128_is_signaling_nan(xa->f128, &env->fp_status) ||         \
-        float128_is_signaling_nan(xb->f128, &env->fp_status)) {         \
-        vxsnan_flag = true;                                             \
-        cc = CRF_SO;                                                    \
-        if (fpscr_ve == 0 && ordered) {                                 \
-            vxvc_flag = true;                                           \
-        }                                                               \
-    } else if (float128_is_quiet_nan(xa->f128, &env->fp_status) ||      \
-               float128_is_quiet_nan(xb->f128, &env->fp_status)) {      \
-        cc = CRF_SO;                                                    \
-        if (ordered) {                                                  \
-            vxvc_flag = true;                                           \
-        }                                                               \
-    }                                                                   \
-    if (vxsnan_flag) {                                                  \
-        float_invalid_op_vxsnan(env, GETPC());                          \
-    }                                                                   \
-    if (vxvc_flag) {                                                    \
-        float_invalid_op_vxvc(env, 0, GETPC());                         \
-    }                                                                   \
-                                                                        \
-    switch (float128_compare(xa->f128, xb->f128, &env->fp_status)) {    \
-    case float_relation_less:                                           \
-        cc |= CRF_LT;                                                   \
-        break;                                                          \
-    case float_relation_equal:                                          \
-        cc |= CRF_EQ;                                                   \
-        break;                                                          \
-    case float_relation_greater:                                        \
-        cc |= CRF_GT;                                                   \
-        break;                                                          \
-    case float_relation_unordered:                                      \
-        cc |= CRF_SO;                                                   \
-        break;                                                          \
-    }                                                                   \
-                                                                        \
-    env->fpscr &= ~FP_FPCC;                                             \
-    env->fpscr |= cc << FPSCR_FPCC;                                     \
-    env->crf[BF(opcode)] = cc;                                          \
-                                                                        \
-    do_float_check_status(env, GETPC());                                \
+    env->fpscr &= ~FP_FPCC;
+    env->fpscr |= cc << FPSCR_FPCC;
+    env->crf[crf_idx] = cc;
+
+    do_float_check_status(env, GETPC());
+}
+
+void helper_xscmpoqp(CPUPPCState *env, uint32_t opcode, ppc_vsr_t *xa,
+                     ppc_vsr_t *xb)
+{
+    do_scalar_cmpq(env, xa, xb, BF(opcode), true);
 }
 
-VSX_SCALAR_CMPQ(xscmpoqp, 1)
-VSX_SCALAR_CMPQ(xscmpuqp, 0)
+void helper_xscmpuqp(CPUPPCState *env, uint32_t opcode, ppc_vsr_t *xa,
+                     ppc_vsr_t *xb)
+{
+    do_scalar_cmpq(env, xa, xb, BF(opcode), false);
+}
 
 /*
  * VSX_MAX_MIN - VSX floating point maximum/minimum
-- 
2.27.0



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

* [PATCH v2 3/4] ppc/translate: Delay NaN checking after comparison
  2020-11-10 10:53 [PATCH v2 0/4] ppc/translate: Fix unordered f64/f128 comparisons LemonBoy
  2020-11-10 10:53 ` [PATCH v2 1/4] " LemonBoy
  2020-11-10 10:53 ` [PATCH v2 2/4] ppc/translate: Turn the helper macros into functions LemonBoy
@ 2020-11-10 10:53 ` LemonBoy
  2020-11-12 21:28   ` Richard Henderson
  2020-11-10 10:53 ` [PATCH v2 4/4] ppc/translate: Raise exceptions after setting the cc LemonBoy
  3 siblings, 1 reply; 9+ messages in thread
From: LemonBoy @ 2020-11-10 10:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: LemonBoy, richard.henderson, qemu-ppc, david

Since we always perform a comparison between the two operands avoid
checking for NaN unless the result states they're unordered.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
---
 target/ppc/fpu_helper.c | 82 +++++++++++++++++++++--------------------
 1 file changed, 42 insertions(+), 40 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index c74c080c17..315959f681 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2475,25 +2475,6 @@ static inline void do_scalar_cmp(CPUPPCState *env, ppc_vsr_t *xa, ppc_vsr_t *xb,
 
     helper_reset_fpstatus(env);
 
-    if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) ||
-        float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) {
-        vxsnan_flag = true;
-        if (fpscr_ve == 0 && ordered) {
-            vxvc_flag = true;
-        }
-    } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) ||
-               float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) {
-        if (ordered) {
-            vxvc_flag = true;
-        }
-    }
-    if (vxsnan_flag) {
-        float_invalid_op_vxsnan(env, GETPC());
-    }
-    if (vxvc_flag) {
-        float_invalid_op_vxvc(env, 0, GETPC());
-    }
-
     switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {
     case float_relation_less:
         cc |= CRF_LT;
@@ -2506,6 +2487,27 @@ static inline void do_scalar_cmp(CPUPPCState *env, ppc_vsr_t *xa, ppc_vsr_t *xb,
         break;
     case float_relation_unordered:
         cc |= CRF_SO;
+
+        if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) ||
+            float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) {
+            vxsnan_flag = true;
+            if (fpscr_ve == 0 && ordered) {
+                vxvc_flag = true;
+            }
+        } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) ||
+                   float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) {
+            if (ordered) {
+                vxvc_flag = true;
+            }
+        }
+
+        if (vxsnan_flag) {
+            float_invalid_op_vxsnan(env, GETPC());
+        }
+        if (vxvc_flag) {
+            float_invalid_op_vxvc(env, 0, GETPC());
+        }
+
         break;
     }
 
@@ -2536,27 +2538,6 @@ static inline void do_scalar_cmpq(CPUPPCState *env, ppc_vsr_t *xa,
 
     helper_reset_fpstatus(env);
 
-    if (float128_is_signaling_nan(xa->f128, &env->fp_status) ||
-        float128_is_signaling_nan(xb->f128, &env->fp_status)) {
-        vxsnan_flag = true;
-        cc = CRF_SO;
-        if (fpscr_ve == 0 && ordered) {
-            vxvc_flag = true;
-        }
-    } else if (float128_is_quiet_nan(xa->f128, &env->fp_status) ||
-               float128_is_quiet_nan(xb->f128, &env->fp_status)) {
-        cc = CRF_SO;
-        if (ordered) {
-            vxvc_flag = true;
-        }
-    }
-    if (vxsnan_flag) {
-        float_invalid_op_vxsnan(env, GETPC());
-    }
-    if (vxvc_flag) {
-        float_invalid_op_vxvc(env, 0, GETPC());
-    }
-
     switch (float128_compare(xa->f128, xb->f128, &env->fp_status)) {
     case float_relation_less:
         cc |= CRF_LT;
@@ -2569,6 +2550,27 @@ static inline void do_scalar_cmpq(CPUPPCState *env, ppc_vsr_t *xa,
         break;
     case float_relation_unordered:
         cc |= CRF_SO;
+
+        if (float128_is_signaling_nan(xa->f128, &env->fp_status) ||
+            float128_is_signaling_nan(xb->f128, &env->fp_status)) {
+            vxsnan_flag = true;
+            if (fpscr_ve == 0 && ordered) {
+                vxvc_flag = true;
+            }
+        } else if (float128_is_quiet_nan(xa->f128, &env->fp_status) ||
+                   float128_is_quiet_nan(xb->f128, &env->fp_status)) {
+            if (ordered) {
+                vxvc_flag = true;
+            }
+        }
+
+        if (vxsnan_flag) {
+            float_invalid_op_vxsnan(env, GETPC());
+        }
+        if (vxvc_flag) {
+            float_invalid_op_vxvc(env, 0, GETPC());
+        }
+
         break;
     }
 
-- 
2.27.0



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

* [PATCH v2 4/4] ppc/translate: Raise exceptions after setting the cc
  2020-11-10 10:53 [PATCH v2 0/4] ppc/translate: Fix unordered f64/f128 comparisons LemonBoy
                   ` (2 preceding siblings ...)
  2020-11-10 10:53 ` [PATCH v2 3/4] ppc/translate: Delay NaN checking after comparison LemonBoy
@ 2020-11-10 10:53 ` LemonBoy
  2020-11-12 21:28   ` Richard Henderson
  3 siblings, 1 reply; 9+ messages in thread
From: LemonBoy @ 2020-11-10 10:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: LemonBoy, richard.henderson, qemu-ppc, david

The PowerISA reference states that the comparison operators update the
FPCC, CR and FPSCR and, if VE=1, jump to the exception handler.

Moving the exception-triggering code after the CC update sequence solves
the problem.

Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
---
 target/ppc/fpu_helper.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 315959f681..c851ac8f5a 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2501,13 +2501,6 @@ static inline void do_scalar_cmp(CPUPPCState *env, ppc_vsr_t *xa, ppc_vsr_t *xb,
             }
         }
 
-        if (vxsnan_flag) {
-            float_invalid_op_vxsnan(env, GETPC());
-        }
-        if (vxvc_flag) {
-            float_invalid_op_vxvc(env, 0, GETPC());
-        }
-
         break;
     }
 
@@ -2515,6 +2508,13 @@ static inline void do_scalar_cmp(CPUPPCState *env, ppc_vsr_t *xa, ppc_vsr_t *xb,
     env->fpscr |= cc << FPSCR_FPCC;
     env->crf[crf_idx] = cc;
 
+    if (vxsnan_flag) {
+        float_invalid_op_vxsnan(env, GETPC());
+    }
+    if (vxvc_flag) {
+        float_invalid_op_vxvc(env, 0, GETPC());
+    }
+
     do_float_check_status(env, GETPC());
 }
 
@@ -2564,13 +2564,6 @@ static inline void do_scalar_cmpq(CPUPPCState *env, ppc_vsr_t *xa,
             }
         }
 
-        if (vxsnan_flag) {
-            float_invalid_op_vxsnan(env, GETPC());
-        }
-        if (vxvc_flag) {
-            float_invalid_op_vxvc(env, 0, GETPC());
-        }
-
         break;
     }
 
@@ -2578,6 +2571,13 @@ static inline void do_scalar_cmpq(CPUPPCState *env, ppc_vsr_t *xa,
     env->fpscr |= cc << FPSCR_FPCC;
     env->crf[crf_idx] = cc;
 
+    if (vxsnan_flag) {
+        float_invalid_op_vxsnan(env, GETPC());
+    }
+    if (vxvc_flag) {
+        float_invalid_op_vxvc(env, 0, GETPC());
+    }
+
     do_float_check_status(env, GETPC());
 }
 
-- 
2.27.0



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

* Re: [PATCH v2 1/4] ppc/translate: Fix unordered f64/f128 comparisons
  2020-11-10 10:53 ` [PATCH v2 1/4] " LemonBoy
@ 2020-11-12 21:26   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2020-11-12 21:26 UTC (permalink / raw)
  To: LemonBoy, qemu-devel; +Cc: qemu-ppc, david

On 11/10/20 2:53 AM, LemonBoy wrote:
>      if (float64_is_signaling_nan(xa->VsrD(0), &env->fp_status) ||        \
>          float64_is_signaling_nan(xb->VsrD(0), &env->fp_status)) {        \
>          vxsnan_flag = true;                                              \
> -        cc = CRF_SO;                                                     \
>          if (fpscr_ve == 0 && ordered) {                                  \
>              vxvc_flag = true;                                            \
>          }                                                                \
>      } else if (float64_is_quiet_nan(xa->VsrD(0), &env->fp_status) ||     \
>                 float64_is_quiet_nan(xb->VsrD(0), &env->fp_status)) {     \
> -        cc = CRF_SO;                                                     \
>          if (ordered) {                                                   \
>              vxvc_flag = true;                                            \
>          }                                                                \
> @@ -2497,12 +2495,19 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,                      \
>          float_invalid_op_vxvc(env, 0, GETPC());                          \
>      }                                                                    \
>                                                                           \
> -    if (float64_lt(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {         \
> +    switch (float64_compare(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) {\
> +    case float_relation_less:                                            \
>          cc |= CRF_LT;                                                    \
> -    } else if (!float64_le(xa->VsrD(0), xb->VsrD(0), &env->fp_status)) { \
> -        cc |= CRF_GT;                                                    \
> -    } else {                                                             \
> +        break;                                                           \
> +    case float_relation_equal:                                           \
>          cc |= CRF_EQ;                                                    \
> +        break;                                                           \
> +    case float_relation_greater:                                         \
> +        cc |= CRF_GT;                                                    \
> +        break;                                                           \
> +    case float_relation_unordered:                                       \
> +        cc |= CRF_SO;                                                    \
> +        break;                                                           \
>      }              

You can now drop the |= for =, since cc is always 0 beforehand.


r~


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

* Re: [PATCH v2 2/4] ppc/translate: Turn the helper macros into functions
  2020-11-10 10:53 ` [PATCH v2 2/4] ppc/translate: Turn the helper macros into functions LemonBoy
@ 2020-11-12 21:27   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2020-11-12 21:27 UTC (permalink / raw)
  To: LemonBoy, qemu-devel; +Cc: qemu-ppc, david

On 11/10/20 2:53 AM, LemonBoy wrote:
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> ---
>  target/ppc/fpu_helper.c | 220 +++++++++++++++++++++-------------------
>  1 file changed, 118 insertions(+), 102 deletions(-)

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

r~


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

* Re: [PATCH v2 3/4] ppc/translate: Delay NaN checking after comparison
  2020-11-10 10:53 ` [PATCH v2 3/4] ppc/translate: Delay NaN checking after comparison LemonBoy
@ 2020-11-12 21:28   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2020-11-12 21:28 UTC (permalink / raw)
  To: LemonBoy, qemu-devel; +Cc: qemu-ppc, david

On 11/10/20 2:53 AM, LemonBoy wrote:
> Since we always perform a comparison between the two operands avoid
> checking for NaN unless the result states they're unordered.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> ---
>  target/ppc/fpu_helper.c | 82 +++++++++++++++++++++--------------------
>  1 file changed, 42 insertions(+), 40 deletions(-)

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

r~


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

* Re: [PATCH v2 4/4] ppc/translate: Raise exceptions after setting the cc
  2020-11-10 10:53 ` [PATCH v2 4/4] ppc/translate: Raise exceptions after setting the cc LemonBoy
@ 2020-11-12 21:28   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2020-11-12 21:28 UTC (permalink / raw)
  To: LemonBoy, qemu-devel; +Cc: qemu-ppc, david

On 11/10/20 2:53 AM, LemonBoy wrote:
> The PowerISA reference states that the comparison operators update the
> FPCC, CR and FPSCR and, if VE=1, jump to the exception handler.
> 
> Moving the exception-triggering code after the CC update sequence solves
> the problem.
> 
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> ---
>  target/ppc/fpu_helper.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)

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

r~


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

end of thread, other threads:[~2020-11-12 21:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 10:53 [PATCH v2 0/4] ppc/translate: Fix unordered f64/f128 comparisons LemonBoy
2020-11-10 10:53 ` [PATCH v2 1/4] " LemonBoy
2020-11-12 21:26   ` Richard Henderson
2020-11-10 10:53 ` [PATCH v2 2/4] ppc/translate: Turn the helper macros into functions LemonBoy
2020-11-12 21:27   ` Richard Henderson
2020-11-10 10:53 ` [PATCH v2 3/4] ppc/translate: Delay NaN checking after comparison LemonBoy
2020-11-12 21:28   ` Richard Henderson
2020-11-10 10:53 ` [PATCH v2 4/4] ppc/translate: Raise exceptions after setting the cc LemonBoy
2020-11-12 21:28   ` Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.