[v2,1/4] ppc/translate: Fix unordered f64/f128 comparisons

Commit Message

Giuseppe Musacchio Nov. 10, 2020, 10:53 a.m. UTC

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 <[e-mail]>
---
 target/ppc/fpu_helper.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

Comments

Richard Henderson Nov. 12, 2020, 9:26 p.m. UTC | #1

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~

Patch

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;                                             \