Re: [PATCH v6 26/82] target/arm: Implement SVE2 SHRN, RSHRN

[Richard Henderson]

On 5/12/21 3:52 AM, Peter Maydell wrote:
> On Fri, 30 Apr 2021 at 21:49, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> > v2: Fix typo in gen_shrnb_vec (laurent desnogues)
> > v3: Replace DO_RSHR with an inline function
> > ---
> >   target/arm/helper-sve.h    |  16 ++++
> >   target/arm/sve.decode      |   8 ++
> >   target/arm/sve_helper.c    |  54 ++++++++++++-
> >   target/arm/translate-sve.c | 160 +++++++++++++++++++++++++++++++++++++
> >   4 files changed, 236 insertions(+), 2 deletions(-)
>
> > -#undef DO_SHR
> > -#undef DO_SHL
>
> Did we want to move the #undef DO_SHR/DO_SHL rather than just deleting them ?
> (I have to admit I'm not sure to what extent undeffing all of these macros is
> worth the effort -- I ran into similar minor awkwardness in the MVE
> helper .c file.)

Yeah, I'm not sure.  I've vaguely been thinking that the "operation" defines, 
such as this one, should stay defined
but that the expansion ones, such as...

> >   #undef DO_ASRD
> >   #undef DO_ZPZI
> >   #undef DO_ZPZI_D

... these should only stay for their "scope".  But even that could be 
considered pointless.

AFAIR, DO_SHR/DO_SHL are pre-existing from the SVE1 work?

> > +#define DO_SHRNB(NAME, TYPEW, TYPEN, OP) \
> > +void HELPER(NAME)(void *vd, void *vn, uint32_t desc)         \
> > +{                                                            \
> > +    intptr_t i, opr_sz = simd_oprsz(desc);                   \
> > +    int shift = simd_data(desc);                             \
> > +    for (i = 0; i < opr_sz; i += sizeof(TYPEW)) {            \
> > +        TYPEW nn = *(TYPEW *)(vn + i);                       \
> > +        *(TYPEW *)(vd + i) = (TYPEN)OP(nn, shift);           \
> > +    }                                                        \
> > +}
>
> Doesn't this need some H() macros, the way the T version does ?

No.  In the "B" version, we're manipulating strict columns (TYPEW in both VN 
and VD), and do not care in which order the columns are processed.

The "T" version reads TYPEW and writes TYPEN, which makes us have to care more 
about addressing.


r~