Re: [PATCH for-6.2 17/34] target/arm: Implement MVE VMLAS

[Peter Maydell]

On Sat, 17 Jul 2021 at 21:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 7/17/21 3:06 AM, Peter Maydell wrote:
> > On Fri, 16 Jul 2021 at 23:12, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> On 7/13/21 6:37 AM, Peter Maydell wrote:
> >>> Implement the MVE VMLAS insn, which multiplies a vector by a vector
> >>> and adds a scalar.
> >>>
> >>> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> >>> ---
> >>>    target/arm/helper-mve.h    |  8 ++++++++
> >>>    target/arm/mve.decode      |  3 +++
> >>>    target/arm/mve_helper.c    | 31 +++++++++++++++++++++++++++++++
> >>>    target/arm/translate-mve.c |  2 ++
> >>>    4 files changed, 44 insertions(+)
> >> ...
> >>
> >>> +/* Vector by vector plus scalar */
> >>> +#define DO_VMLAS(D, N, M) ((N) * (D) + (M))
> >>> +
> >>> +DO_2OP_ACC_SCALAR_S(vmlass, DO_VMLAS)
> >>> +DO_2OP_ACC_SCALAR_U(vmlasu, DO_VMLAS)
> >>
> >> This is confusing.  The ARM says
> >>
> >> # Operations that do not perform
> >> # widening are always unsigned (encoded with U=1),
> >
> > I have noticed that that text often appears for insns where it doesn't
> > really apply. I mostly ignore the text in favour of looking at
> > the pseudocode for working out what is supposed to be done.

NB that in the latest version of the v8M Arm ARM (B.p) the
"operations that do not perform widening..." text has been removed.

> Yes, but in this case there's nothing about the pseudocode that suggests that 
> sign matters
> at all.  Neither the multiply nor the addition are widening.  So is there 
> really a signed
> VMLAS instruction?

There really is a U bit, but it doesn't affect the result :-)
I will remove the unnecessary extra set of helpers (similarly
for VMLA vector-by-scalar-plus-vector)...

thanks
-- PMM