Re: [Qemu-devel] [PATCH v2 02/15] target/ppc: remove getVSR()/putVSR() from mem_helper.c

[David Gibson]

On Sun, Jun 02, 2019 at 12:08:50PM +0100, Mark Cave-Ayland wrote:
> Since commit 8a14d31b00 "target/ppc: switch fpr/vsrl registers so all VSX
> registers are in host endian order" functions getVSR() and putVSR() which used
> to convert the VSR registers into host endian order are no longer required.
> 
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> ---
>  target/ppc/mem_helper.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
> index 5b0f9ee50d..17a3c931a9 100644
> --- a/target/ppc/mem_helper.c
> +++ b/target/ppc/mem_helper.c
> @@ -417,26 +417,27 @@ STVE(stvewx, cpu_stl_data_ra, bswap32, u32)
>  void helper_##name(CPUPPCState *env, target_ulong addr,                 \
>                     target_ulong xt_num, target_ulong rb)                \
>  {                                                                       \
> -    int i;                                                              \
> -    ppc_vsr_t xt;                                                       \
> +    ppc_vsr_t *xt = &env->vsr[xt_num];                                  \
> +    ppc_vsr_t t;                                                        \
>      uint64_t nb = GET_NB(rb);                                           \
> +    int i;                                                              \
>                                                                          \
> -    xt.s128 = int128_zero();                                            \
> +    t.s128 = int128_zero();                                             \
>      if (nb) {                                                           \
>          nb = (nb >= 16) ? 16 : nb;                                      \
>          if (msr_le && !lj) {                                            \
>              for (i = 16; i > 16 - nb; i--) {                            \
> -                xt.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC());  \
> +                t.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC());   \
>                  addr = addr_add(env, addr, 1);                          \
>              }                                                           \
>          } else {                                                        \
>              for (i = 0; i < nb; i++) {                                  \
> -                xt.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC());      \
> +                t.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC());       \
>                  addr = addr_add(env, addr, 1);                          \
>              }                                                           \
>          }                                                               \
>      }                                                                   \
> -    putVSR(xt_num, &xt, env);                                           \
> +    *xt = t;                                                            \
>  }
>  
>  VSX_LXVL(lxvl, 0)
> @@ -447,26 +448,28 @@ VSX_LXVL(lxvll, 1)
>  void helper_##name(CPUPPCState *env, target_ulong addr,           \
>                     target_ulong xt_num, target_ulong rb)          \
>  {                                                                 \
> -    int i;                                                        \
> -    ppc_vsr_t xt;                                                 \
> +    ppc_vsr_t *xt = &env->vsr[xt_num];                            \
> +    ppc_vsr_t t = *xt;                                            \
>      target_ulong nb = GET_NB(rb);                                 \
> +    int i;                                                        \
>                                                                    \
>      if (!nb) {                                                    \
>          return;                                                   \
>      }                                                             \
> -    getVSR(xt_num, &xt, env);                                     \
> +                                                                  \
>      nb = (nb >= 16) ? 16 : nb;                                    \
>      if (msr_le && !lj) {                                          \
>          for (i = 16; i > 16 - nb; i--) {                          \
> -            cpu_stb_data_ra(env, addr, xt.VsrB(i - 1), GETPC());  \
> +            cpu_stb_data_ra(env, addr, t.VsrB(i - 1), GETPC());   \
>              addr = addr_add(env, addr, 1);                        \
>          }                                                         \
>      } else {                                                      \
>          for (i = 0; i < nb; i++) {                                \
> -            cpu_stb_data_ra(env, addr, xt.VsrB(i), GETPC());      \
> +            cpu_stb_data_ra(env, addr, t.VsrB(i), GETPC())  ;     \
>              addr = addr_add(env, addr, 1);                        \
>          }                                                         \
>      }                                                             \
> +    *xt = t;                                                      \

Is this correct?  AFAICT the original wasn't writing back, so why does
the new version?

>  }
>  
>  VSX_STXVL(stxvl, 0)

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

signature.asc
Description: PGP signature