Re: [PATCH v2 8/8] target/ppc: Clear fpstatus flags on helpers missing it

[Daniel Henrique Barboza]

On 9/6/22 09:55, Víctor Colombo wrote:

In ppc emulation, exception flags are not cleared at the end of an
instruction. Instead, the next instruction is responsible to clear
it before its emulation. However, some helpers are not doing it,
causing an issue where the previously set exception flags are being
used and leading to incorrect values being set in FPSCR.
Fix this by clearing fp_status before doing the instruction 'real' work
for the following helpers that were missing this behavior:

- VSX_CVT_INT_TO_FP_VECTOR
- VSX_CVT_FP_TO_FP
- VSX_CVT_FP_TO_INT_VECTOR
- VSX_CVT_FP_TO_INT2
- VSX_CVT_FP_TO_INT
- VSX_CVT_FP_TO_FP_HP
- VSX_CVT_FP_TO_FP_VECTOR
- VSX_CMP
- VSX_ROUND
- xscvqpdp
- xscvdpsp[n]

Signed-off-by: Víctor Colombo <victor.colo...@eldorado.org.br>
---

Reviewed-by: Daniel Henrique Barboza <danielhb...@gmail.com>

  target/ppc/fpu_helper.c | 37 ++++++++++++++++++++++++++-----------
  1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 95b22d99b3..331361234f 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2637,6 +2637,8 @@ uint32_t helper_##op(CPUPPCState *env, ppc_vsr_t *xt,     
                \
      int all_true = 1;                                                     \
      int all_false = 1;                                                    \
                                                                            \
+    helper_reset_fpstatus(env);                                           \
+                                                                          \
      for (i = 0; i < nels; i++) {                                          \
          if (unlikely(tp##_is_any_nan(xa->fld) ||                          \
                       tp##_is_any_nan(xb->fld))) {                         \
@@ -2690,6 +2692,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, 
ppc_vsr_t *xb)   \
      ppc_vsr_t t = { };                                             \
      int i;                                                         \
                                                                     \
+    helper_reset_fpstatus(env);                                    \
+                                                                   \
      for (i = 0; i < nels; i++) {                                   \
          t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status);        \
          if (unlikely(stp##_is_signaling_nan(xb->sfld,              \
@@ -2715,6 +2719,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, 
ppc_vsr_t *xb)      \
      ppc_vsr_t t = { };                                                \
      int i;                                                            \
                                                                        \
+    helper_reset_fpstatus(env);                                       \
+                                                                      \
      for (i = 0; i < nels; i++) {                                      \
          t.VsrW(2 * i) = stp##_to_##ttp(xb->VsrD(i), &env->fp_status); \
          if (unlikely(stp##_is_signaling_nan(xb->VsrD(i),              \
@@ -2752,6 +2758,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,       
              \
      ppc_vsr_t t = *xt;                                                  \
      int i;                                                              \
                                                                          \
+    helper_reset_fpstatus(env);                                         \
+                                                                        \
      for (i = 0; i < nels; i++) {                                        \
          t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status);             \
          if (unlikely(stp##_is_signaling_nan(xb->sfld,                   \
@@ -2787,6 +2795,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, 
ppc_vsr_t *xb)   \
      ppc_vsr_t t = { };                                             \
      int i;                                                         \
                                                                     \
+    helper_reset_fpstatus(env);                                    \
+                                                                   \
      for (i = 0; i < nels; i++) {                                   \
          t.tfld = stp##_to_##ttp(xb->sfld, 1, &env->fp_status);     \
          if (unlikely(stp##_is_signaling_nan(xb->sfld,              \
@@ -2834,6 +2844,8 @@ void helper_XSCVQPDP(CPUPPCState *env, uint32_t ro, 
ppc_vsr_t *xt,
      ppc_vsr_t t = { };
      float_status tstat;

+ helper_reset_fpstatus(env);

+
      tstat = env->fp_status;
      if (ro != 0) {
          tstat.float_rounding_mode = float_round_to_odd;
@@ -2855,6 +2867,7 @@ uint64_t helper_xscvdpspn(CPUPPCState *env, uint64_t xb)
  {
      uint64_t result, sign, exp, frac;

+ helper_reset_fpstatus(env);

      float_status tstat = env->fp_status;
      set_float_exception_flags(0, &tstat);

@@ -2910,22 +2923,20 @@ uint64_t helper_XSCVSPDPN(uint64_t xb)

  #define VSX_CVT_FP_TO_INT(op, nels, stp, ttp, sfld, tfld, sfi, rnan)         \
  void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb)             \
  {                                                                            \
-    int all_flags = env->fp_status.float_exception_flags, flags;             \
      ppc_vsr_t t = { };                                                       \
-    int i;                                                                   \
+    int i, flags;                                                            \
+                                                                             \
+    helper_reset_fpstatus(env);                                              \
                                                                               \
      for (i = 0; i < nels; i++) {                                             \
-        env->fp_status.float_exception_flags = 0;                            \
          t.tfld = stp##_to_##ttp##_round_to_zero(xb->sfld, &env->fp_status);  \
          flags = env->fp_status.float_exception_flags;                        \
          if (unlikely(flags & float_flag_invalid)) {                          \
              t.tfld = float_invalid_cvt(env, flags, t.tfld, rnan, 0, GETPC());\
          }                                                                    \
-        all_flags |= flags;                                                  \
      }                                                                        \
                                                                               \
      *xt = t;                                                                 \
-    env->fp_status.float_exception_flags = all_flags;                        \
      do_float_check_status(env, sfi, GETPC());                                \
  }

@@ -2977,12 +2988,12 @@ VSX_CVT_FP_TO_INT128(XSCVQPSQZ, int128, 0x8000000000000000ULL);

  #define VSX_CVT_FP_TO_INT2(op, nels, stp, ttp, sfi, rnan)                    \
  void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb)             \
  {                                                                            \
-    int all_flags = env->fp_status.float_exception_flags, flags;             \
      ppc_vsr_t t = { };                                                       \
-    int i;                                                                   \
+    int i, flags;                                                            \
+                                                                             \
+    helper_reset_fpstatus(env);                                              \
                                                                               \
      for (i = 0; i < nels; i++) {                                             \
-        env->fp_status.float_exception_flags = 0;                            \
          t.VsrW(2 * i) = stp##_to_##ttp##_round_to_zero(xb->VsrD(i),          \
                                                         &env->fp_status);     \
          flags = env->fp_status.float_exception_flags;                        \
@@ -2991,11 +3002,9 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, 
ppc_vsr_t *xb)             \
                                                rnan, 0, GETPC());             \
          }                                                                    \
          t.VsrW(2 * i + 1) = t.VsrW(2 * i);                                   \
-        all_flags |= flags;                                                  \
      }                                                                        \
                                                                               \
      *xt = t;                                                                 \
-    env->fp_status.float_exception_flags = all_flags;                        \
      do_float_check_status(env, sfi, GETPC());                                \
  }

@@ -3020,6 +3029,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode, \

      ppc_vsr_t t = { };                                                       \
      int flags;                                                               \
                                                                               \
+    helper_reset_fpstatus(env);                                              \
+                                                                             \
      t.tfld = stp##_to_##ttp##_round_to_zero(xb->sfld, &env->fp_status);      \
      flags = get_float_exception_flags(&env->fp_status);                      \
      if (flags & float_flag_invalid) {                                        \
@@ -3032,7 +3043,6 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,       
                   \

VSX_CVT_FP_TO_INT_VECTOR(xscvqpsdz, float128, int64, f128, VsrD(0), \

                    0x8000000000000000ULL)
-
  VSX_CVT_FP_TO_INT_VECTOR(xscvqpswz, float128, int32, f128, VsrD(0),          \
                    0xffffffff80000000ULL)
  VSX_CVT_FP_TO_INT_VECTOR(xscvqpudz, float128, uint64, f128, VsrD(0), 0x0ULL)
@@ -3055,6 +3065,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, 
ppc_vsr_t *xb)        \
      ppc_vsr_t t = { };                                                  \
      int i;                                                              \
                                                                          \
+    helper_reset_fpstatus(env);                                         \
+                                                                        \
      for (i = 0; i < nels; i++) {                                        \
          t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status);             \
          if (r2sp) {                                                     \
@@ -3124,6 +3136,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode,       
              \
  {                                                                       \
      ppc_vsr_t t = *xt;                                                  \
                                                                          \
+    helper_reset_fpstatus(env);                                         \
      t.tfld = stp##_to_##ttp(xb->sfld, &env->fp_status);                 \
      helper_compute_fprf_##ttp(env, t.tfld);                             \
                                                                          \
@@ -3157,6 +3170,8 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, 
ppc_vsr_t *xb)       \
      int i;                                                             \
      FloatRoundMode curr_rounding_mode;                                 \
                                                                         \
+    helper_reset_fpstatus(env);                                        \
+                                                                       \
      if (rmode != FLOAT_ROUND_CURRENT) {                                \
          curr_rounding_mode = get_float_rounding_mode(&env->fp_status); \
          set_float_rounding_mode(rmode, &env->fp_status);               \