[Qemu-devel] [RFC PATCH v1 05/22] target/i386: introduce gen_ld_modrm_* helpers

Jan Bobek posted 22 patches 6 years, 6 months ago
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>
[Qemu-devel] [RFC PATCH v1 05/22] target/i386: introduce gen_ld_modrm_* helpers
Posted by Jan Bobek 6 years, 6 months ago
These help with decoding/loading ModR/M vector operands; the operand's
register offset is returned, which is suitable for use with gvec
infrastructure.

Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 target/i386/translate.c | 47 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 9e22eca2dc..7548677e1f 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -3040,6 +3040,53 @@ static const struct SSEOpHelper_eppi sse_op_table7[256] = {
     [0xdf] = AESNI_OP(aeskeygenassist),
 };
 
+static inline void gen_ld_modrm_PqQq(CPUX86State *env, DisasContext *s, int modrm,
+                                     uint32_t* dofs, uint32_t* aofs)
+{
+    const int mod = (modrm >> 6) & 3;
+    const int reg = (modrm >> 3) & 7; /* no REX_R */
+    *dofs = offsetof(CPUX86State, fpregs[reg].mmx);
+
+    if(mod == 3) {
+        const int rm = modrm & 7; /* no REX_B */
+
+        *aofs = offsetof(CPUX86State, fpregs[rm].mmx);
+    } else {
+        *aofs = offsetof(CPUX86State, mmx_t0);
+
+        gen_lea_modrm(env, s, modrm);
+        gen_ldq_env_A0(s, *aofs);
+    }
+}
+
+static inline void gen_ld_modrm_VxWx(CPUX86State *env, DisasContext *s, int modrm,
+                                     uint32_t* dofs, uint32_t* aofs)
+{
+    const int mod = (modrm >> 6) & 3;
+    const int reg = ((modrm >> 3) & 7) | REX_R(s);
+    *dofs = offsetof(CPUX86State, xmm_regs[reg]);
+
+    if(mod == 3) {
+        const int rm = (modrm & 7) | REX_B(s);
+
+        *aofs = offsetof(CPUX86State, xmm_regs[rm]);
+    } else {
+        *aofs = offsetof(CPUX86State, xmm_t0);
+
+        gen_lea_modrm(env, s, modrm);
+        gen_ldo_env_A0(s, *aofs); /* FIXME this needs to load 32 bytes for YMM */
+    }
+}
+
+static inline void gen_ld_modrm_VxHxWx(CPUX86State *env, DisasContext *s, int modrm,
+                                       uint32_t* dofs, uint32_t* aofs, uint32_t* bofs)
+{
+    assert(s->prefix & PREFIX_VEX);
+
+    gen_ld_modrm_VxWx(env, s, modrm, dofs, bofs);
+    *aofs = offsetof(CPUX86State, xmm_regs[s->vex_v]);
+}
+
 static void gen_sse(CPUX86State *env, DisasContext *s, int b)
 {
     int b1, op1_offset, op2_offset, is_xmm, val;
-- 
2.20.1


Re: [Qemu-devel] [RFC PATCH v1 05/22] target/i386: introduce gen_ld_modrm_* helpers
Posted by Richard Henderson 6 years, 6 months ago
On 7/31/19 10:56 AM, Jan Bobek wrote:
> These help with decoding/loading ModR/M vector operands; the operand's
> register offset is returned, which is suitable for use with gvec
> infrastructure.
> 
> Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
> ---
>  target/i386/translate.c | 47 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 9e22eca2dc..7548677e1f 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -3040,6 +3040,53 @@ static const struct SSEOpHelper_eppi sse_op_table7[256] = {
>      [0xdf] = AESNI_OP(aeskeygenassist),
>  };
>  
> +static inline void gen_ld_modrm_PqQq(CPUX86State *env, DisasContext *s, int modrm,
> +                                     uint32_t* dofs, uint32_t* aofs)

s/uint32_t* /uint32_t */

Drop the inlines; let the compiler choose.


> +{
> +    const int mod = (modrm >> 6) & 3;
> +    const int reg = (modrm >> 3) & 7; /* no REX_R */
> +    *dofs = offsetof(CPUX86State, fpregs[reg].mmx);
> +
> +    if(mod == 3) {

s/if(/if (/

Both of these errors should be caught by ./scripts/checkpatch.pl.

> +        gen_ldo_env_A0(s, *aofs); /* FIXME this needs to load 32 bytes for YMM 

Better as "TODO", since this isn't broken and in need of fixing, since we do
not yet support AVX.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [RFC PATCH v1 05/22] target/i386: introduce gen_ld_modrm_* helpers
Posted by Jan Bobek 6 years, 6 months ago
On 7/31/19 3:08 PM, Richard Henderson wrote:
> On 7/31/19 10:56 AM, Jan Bobek wrote:
>> These help with decoding/loading ModR/M vector operands; the operand's
>> register offset is returned, which is suitable for use with gvec
>> infrastructure.
>>
>> Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
>> ---
>>  target/i386/translate.c | 47 +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 47 insertions(+)
>>
>> diff --git a/target/i386/translate.c b/target/i386/translate.c
>> index 9e22eca2dc..7548677e1f 100644
>> --- a/target/i386/translate.c
>> +++ b/target/i386/translate.c
>> @@ -3040,6 +3040,53 @@ static const struct SSEOpHelper_eppi sse_op_table7[256] = {
>>      [0xdf] = AESNI_OP(aeskeygenassist),
>>  };
>>  
>> +static inline void gen_ld_modrm_PqQq(CPUX86State *env, DisasContext *s, int modrm,
>> +                                     uint32_t* dofs, uint32_t* aofs)
> 
> s/uint32_t* /uint32_t */
> 
> Drop the inlines; let the compiler choose.
> 
> 
>> +{
>> +    const int mod = (modrm >> 6) & 3;
>> +    const int reg = (modrm >> 3) & 7; /* no REX_R */
>> +    *dofs = offsetof(CPUX86State, fpregs[reg].mmx);
>> +
>> +    if(mod == 3) {
> 
> s/if(/if (/
> 
> Both of these errors should be caught by ./scripts/checkpatch.pl.

I have the script set up; I disabled it temporarily (or so I thought)
some time ago when it was preventing me from git stash'ing some
experimental hacks, and never got around to enabling it again.

Anyway, I'll make sure not to forget to run it prior to submission
next time.

>> +        gen_ldo_env_A0(s, *aofs); /* FIXME this needs to load 32 bytes for YMM 
> 
> Better as "TODO", since this isn't broken and in need of fixing, since we do
> not yet support AVX.
> 
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
>