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

[Mark Cave-Ayland]

On 05/05/2019 15:34, Richard Henderson wrote:

> On 5/5/19 2:34 AM, Mark Cave-Ayland wrote:
>>>>      EA = tcg_temp_new();                                        \
>>>> -    xt = tcg_const_tl(xT(ctx->opcode));                         \
>>>>      gen_set_access_type(ctx, ACCESS_INT);                       \
>>>>      gen_addr_register(ctx, EA);                                 \
>>>> -    gen_helper_##name(cpu_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
>>>> +    xt = tcg_const_tl(xT(ctx->opcode));                         \
>>>> +    rb = tcg_const_tl(rB(ctx->opcode));                         \
>>>> +    gen_helper_##name(cpu_env, EA, xt, rb);                     \
>>>>      tcg_temp_free(EA);                                          \
>>>>      tcg_temp_free(xt);                                          \
>>>> +    tcg_temp_free(rb);                                          \
>>>>  }
>>>
>>> Why are you adjusting the function to pass the rB register number rather 
>>> than
>>> the contents of rB?  That seems the wrong way around...
>>
>> I think what I was trying to do here was eliminate the cpu_gpr since it
>> feels to me that with the vector patchsets and your negative offset patches
>> that this should be the way to go for accessing CPUState rather than using
>> TCG globals.
> 
> Not for the integer register set.
> 
>> Looking at this again I realise the solution is really the same as is
>> currently used
>> for gen_load_spr() so I can use something like this:>
>>     static inline void gen_load_gpr(TCGv t, int reg)
>>     {
>>         tcg_gen_ld_tl(t, cpu_env, offsetof(CPUPPCState, gpr[reg]));
>>     }
>>
>> Does this seem reasonable as a solution?
> 
> No, this will fail quickly.

Okay in that case I'll leave it as-is. So just to satisfy my curiosity here: is 
the
problem here the mixing and matching of offsets and TCG globals, rather than 
the use
of offsets as done for the VMX/VSX registers?


ATB,

Mark.