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

[Jan Bobek]

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 <address@hidden>
>> ---
>>  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 <address@hidden>
> 
> 
> r~
>

signature.asc
Description: OpenPGP digital signature