Re: [PATCH 02/37] target/i386: make ldo/sto operations consistent with ldq

[Richard Henderson]

On 9/12/22 00:03, Paolo Bonzini wrote:
> ldq takes a pointer to the first byte to load the 64-bit word in;
> ldo takes a pointer to the first byte of the ZMMReg.  Make them
> consistent, which will be useful in the new SSE decoder's
> load/writeback routines.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/translate.c | 44 +++++++++++++++++++------------------
>   1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 001af76663..9a85010dcd 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -2761,28 +2761,29 @@ static inline void gen_ldo_env_A0(DisasContext *s, int 
> offset)
>   {
>       int mem_index = s->mem_index;
>       tcg_gen_qemu_ld_i64(s->tmp1_i64, s->A0, mem_index, MO_LEUQ);
> -    tcg_gen_st_i64(s->tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(0)));
> +    tcg_gen_st_i64(s->tmp1_i64, cpu_env, offset + offsetof(XMMReg, XMM_Q(0)));
>       tcg_gen_addi_tl(s->tmp0, s->A0, 8);
>       tcg_gen_qemu_ld_i64(s->tmp1_i64, s->tmp0, mem_index, MO_LEUQ);
> -    tcg_gen_st_i64(s->tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(1)));
> +    tcg_gen_st_i64(s->tmp1_i64, cpu_env, offset + offsetof(XMMReg, XMM_Q(1)));
>   }
>   
>   static inline void gen_sto_env_A0(DisasContext *s, int offset)
>   {
>       int mem_index = s->mem_index;
> -    tcg_gen_ld_i64(s->tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(0)));
> +    offset -= offsetof(ZMMReg, ZMM_Q(0));
> +    tcg_gen_ld_i64(s->tmp1_i64, cpu_env, offset + offsetof(XMMReg, XMM_Q(0)));

What is this subtract?  You don't have it for ldo or movo, and it looks wrong.

The rest of it looks ok.


r~