Re: [PATCH v3 1/2] target/i386: Read 8 bytes from cvttps2pi/cvtps2pi memory operands

[Paolo Bonzini]

Hi, I think this is broken for big endian systems because ldq expects
a pointer to xmm_t0.L(0) while ldo expects a pointer xmm_t0.

I will fix the bug in my new AVX decoder though, where it is also
present. So thanks for the report!

Paolo

On Tue, Aug 30, 2022 at 5:48 AM Ricky Zhou <ricky@rzhou.org> wrote:
>
> Before this change, emulation of cvttps2pi and cvtps2pi instructions
> would read 16 bytes of memory instead of 8. The SDM states that
> cvttps2pi takes a 64-bit memory location. The documentation for cvtps2pi
> claims that it takes a a 128-bit memory location, but as with cvttps2pi,
> the operand is written as xmm/m64. I double-checked on real hardware
> that both of these instructions only read 8 bytes.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ricky Zhou <ricky@rzhou.org>
> ---
>  target/i386/tcg/translate.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index b7972f0ff5..3ba5f76156 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -3621,7 +3621,11 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
> int b,
>              if (mod != 3) {
>                  gen_lea_modrm(env, s, modrm);
>                  op2_offset = offsetof(CPUX86State,xmm_t0);
> -                gen_ldo_env_A0(s, op2_offset);
> +                if (b1) {
> +                    gen_ldo_env_A0(s, op2_offset);
> +                } else {
> +                    gen_ldq_env_A0(s, op2_offset);
> +                }
>              } else {
>                  rm = (modrm & 7) | REX_B(s);
>                  op2_offset = offsetof(CPUX86State,xmm_regs[rm]);
> --
> 2.37.2
>