Re: [PATCH for-6.2 32/34] target/arm: Implement MVE scatter-gather insns

[Peter Maydell]

On Thu, 22 Jul 2021 at 01:36, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 7/13/21 3:37 AM, Peter Maydell wrote:
> > +static bool do_ldst_sg(DisasContext *s, arg_vldst_sg *a, MVEGenLdStSGFn fn)
> > +{
> > +    TCGv_i32 addr;
> > +    TCGv_ptr qd, qm;
> > +
> > +    if (!dc_isar_feature(aa32_mve, s) ||
> > +        !mve_check_qreg_bank(s, a->qd | a->qm) ||
> > +        !fn || a->rn == 15) {
> > +        /* Rn case is UNPREDICTABLE */
> > +        return false;
> > +    }
>
> No Qd != Qm check for loads?  Given that we know in advance that it simply 
> won't work for
> VLDRD, it would be nice to diagnose the error.
>
> > +static bool trans_VLDR_S_sg(DisasContext *s, arg_vldst_sg *a)
> > +{
> > +    static MVEGenLdStSGFn * const fns[2][4][4] = { {
> > +            { NULL, F(vldrb_sg_sh), F(vldrb_sg_sw), NULL },
> > +            { NULL, NULL,           F(vldrh_sg_sw), NULL },
> > +            { NULL, NULL,           NULL,           NULL },
> > +            { NULL, NULL,           NULL,           NULL }
> > +        }, {
> > +            { NULL, NULL,              NULL,              NULL },
> > +            { NULL, NULL,              F(vldrh_sg_os_sw), NULL },
> > +            { NULL, NULL,              NULL,              NULL },
> > +            { NULL, NULL,              NULL,              NULL }
> > +        }
> > +    };
>
> A little bit unfortunate with table density here, but whatever.

Yes; I initially wrote things this way incorrectly thinking more of the
tables would be populated than they are, but I think overall it's
a fairly readable way to go.

I noticed yesterday that these load/store implementations aren't
enforcing the alignment fault requirements, though, so I need to
add that.

thanks
-- PMM