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

[Chinmay Rath]

Hi Richard,

My apologies for the ill formatted reply in this patch series. Just 
realized it now. The cliched 'Tab' issue with the mail client XD.
On 6/7/24 21:16, Richard Henderson wrote:
> On 6/7/24 07:49, Chinmay Rath wrote:
> > Moving the following instructions to decodetree specification:
> >
> >    lxv{b16, d2, h8, w4, ds, ws}x   : X-form
> >    stxv{b16, d2, h8, w4}x          : X-form
> >
> > The changes were verified by validating that the tcg-ops generated 
> > for those
> > instructions remain the same, which were captured using the '-d 
> > in_asm,op' flag.
> >
> > Signed-off-by: Chinmay Rath <rathc@linux.ibm.com>
> > ---
> >   target/ppc/insn32.decode            |  10 ++
> >   target/ppc/translate/vsx-impl.c.inc | 199 ++++++++++++----------------
> >   target/ppc/translate/vsx-ops.c.inc  |  12 --
> >   3 files changed, 97 insertions(+), 124 deletions(-)
>
> Because the ops are identical,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> But you really should update these to use tcg_gen_qemu_ld/st_i128 with 
> the proper atomicity flags.  This will fix an existing bug...
^
Surely Richard, I have noted this suggestion from earlier patch and plan 
to do this, and a few others which I couldn't implement earlier, along 
with some clean-ups this week.

I refrained from doing it with the decodetree movement, to take time to 
properly understand and test. Should send out those patches soon.

Thanks & Regards,
Chinmay
> > +static bool trans_LXVD2X(DisasContext *ctx, arg_LXVD2X *a)
> >   {
> >       TCGv EA;
> >       TCGv_i64 t0;
> > +
> > +    REQUIRE_VSX(ctx);
> > +    REQUIRE_INSNS_FLAGS2(ctx, VSX);
> > +
> >       t0 = tcg_temp_new_i64();
> >       gen_set_access_type(ctx, ACCESS_INT);
> > +    EA = do_ea_calc(ctx, a->ra, cpu_gpr[a->rb]);
> >       gen_qemu_ld64_i64(ctx, t0, EA);
> > +    set_cpu_vsr(a->rt, t0, true);
>
> where the vector register is partially modified ...
>
> >       tcg_gen_addi_tl(EA, EA, 8);
> >       gen_qemu_ld64_i64(ctx, t0, EA);
>
> before a fault from the second load is recognized.
> Similarly for stores leaving memory partially modified.
>
>
> r~