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

[Chinmay Rath]

Hi Richard,

On 6/7/24 21:11, Richard Henderson wrote:
> On 6/7/24 07:49, Chinmay Rath wrote:
> > +static bool do_ld_st_vl(DisasContext *ctx, arg_X *a,
> > +                        void (*helper)(TCGv_ptr, TCGv, TCGv_ptr, TCGv))
> > +{
> > +    TCGv EA;
> > +    TCGv_ptr xt;
> > +    if (a->rt < 32) {
> > +        REQUIRE_VSX(ctx);
> > +    } else {
> > +        REQUIRE_VECTOR(ctx);
> > +    }
> > +    xt = gen_vsr_ptr(a->rt);
> > +    gen_set_access_type(ctx, ACCESS_INT);
> > +
> > +    if (a->ra) {
> > +        EA = tcg_temp_new();
> > +        tcg_gen_mov_tl(EA, cpu_gpr[a->ra]);
> > +    } else {
> > +        EA = tcg_constant_tl(0);
> > +    }
> > +    if (NARROW_MODE(ctx)) {
> > +        tcg_gen_ext32u_tl(EA, EA);
>
> ra == 0, narrow mode, will crash, due to write into constant 0.
> Obviously 0 does not need extending, so this could be
>
>     if (!a->ra) {
>         ea = constant 0;
>     } else if (narrow mode) {
>         ea = tcg_temp_new();
>         tcg_gen_ext32u_tl(ea, cpu_gpr[a->ra]);
>     } else {
>         ra = cpu_gpr[a->ra];
>     }
^ Thank you Richard, will take care in v2.
> Aren't there existing helper functions for computing this address?
> And if not, better to create one.
^
The calculation of effective address in these instructions is slightly 
different than the others,
for which helper function exist :

EA for these insns : EA ← (RA=0) ? 0 : GPR[RA]
EA for rest storage access insns : EA ← ((RA=0) ? 0 : GPR[RA]) + GPR[RB]

This is why I could not reuse that function. Also, this calculation of 
EA is limited to these
4 insns above, and only 2 others (prefixed insns), which is why I did 
not create a new function
for this, considering it won't be reused for any other insn.

Please let me know if I should create a new function in this case as well.

Thanks and Regards,
Chinmay
> r~