Re: [PATCH v3 01/44] target/arm: Implement MVE VLDR/VSTR (non-widening forms)

[Peter Maydell]

On Thu, 17 Jun 2021 at 14:30, Claudio Fontana <cfontana@suse.de> wrote:
>
> On 6/17/21 2:15 PM, Peter Maydell wrote:
> > Implement the forms of the MVE VLDR and VSTR insns which perform
> > non-widening loads of bytes, halfwords or words from memory into
> > vector elements of the same width (encodings T5, T6, T7).
>
> This is if I understand correctly, M-Profile only, and thus TCG-only right?

Yes.

> >
> > (At the moment we know for MVE and M-profile in general that
> > vfp_access_check() can never return false, but we include the
> > conventional return-true-on-failure check for consistency
> > with non-M-profile translation code.)
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > +/* Values for M-profile PSR.ECI for MVE insns */
> > +enum MVEECIState {
> > +    ECI_NONE = 0, /* No completed beats */
> > +    ECI_A0 = 1, /* Completed: A0 */
> > +    ECI_A0A1 = 2, /* Completed: A0, A1 */
> > +    /* 3 is reserved */
> > +    ECI_A0A1A2 = 4, /* Completed: A0, A1, A2 */
> > +    ECI_A0A1A2B0 = 5, /* Completed: A0, A1, A2, B0 */
> > +    /* All other values reserved */
> > +};
> > +
>
> I wonder if this should be in a MVE-specific header,

For an enum definition, it doesn't seem worthwhile. internals.h
is generally for "stuff target/arm wants that other parts of the
system don't need".

> and I also wonder, when looking at code using things like ECI_A0,
>
> what about MVE_ECI_NONE , MVE_ECI_A0, ... - would help the reader connect 
> code containing these enums with the MVE feature?

You'll only see them if you're looking at MVE code in the first place...

thanks
-- PMM