Re: [PATCH v2 2/4] target/ppc: Move VSX vector with length storage access insns to decodetree.

[Chinmay Rath <rathc@linux.vnet.ibm.com>]

Hi Richard,
On 6/17/24 00:43, Richard Henderson wrote:
> On 6/13/24 02:33, Chinmay Rath wrote:
>> +/* EA <- (ra == 0) ? 0 : GPR[ra] */
>> +static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
>> +{
>> +    TCGv EA;
>> +    if (!ra) {
>> +        EA = tcg_constant_tl(0);
>> +        return EA;
>> +    }
>> +    EA = tcg_temp_new();
>> +    if (NARROW_MODE(ctx)) {
>> +        tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
>> +    } else {
>> +        tcg_gen_mov_tl(EA, cpu_gpr[ra]);
>
> Why are you making a copy, rather than just returning cpu_gpr[ra]?
> If you need to modify the resulting EA, then you also need to make a 
> copy for 0.
>
Please ignore my previous response.
I think do_ea_calc_ra should allow modification to the resulting EA, 
hence below change appears more appropriate to me :

/* EA <- (ra == 0) ? 0 : GPR[ra] */
static TCGv do_ea_calc_ra(DisasContext *ctx, int ra)
{
     TCGv EA = tcg_temp_new();
     if (!ra) {
         tcg_gen_movi_tl(EA, 0);
         return EA;
     }
     if (NARROW_MODE(ctx)) {
         tcg_gen_ext32u_tl(EA, cpu_gpr[ra]);
     } else {
         tcg_gen_mov_tl(EA, cpu_gpr[ra]);
     }
     return EA;
}

Let me know your thoughts.

Thanks & Regards,
Chinmay
>
> r~
>