Re: [PATCH for-6.2 20/34] target/arm: Implement MVE integer min/max across vector

[Peter Maydell]

On Mon, 19 Jul 2021 at 16:28, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sat, 17 Jul 2021 at 21:46, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > On 7/13/21 6:37 AM, Peter Maydell wrote:
> > > +/* Max and min of absolute values */
> > > +static int64_t do_maxa(int64_t n, int64_t m)
> > > +{
> > > +    if (n < 0) {
> > > +        n = -n;
> > > +    }
> > > +    if (m < 0) {
> > > +        m = -m;
> > > +    }
> > > +    return MAX(n, m);
> > > +}
> >
> > This doesn't look quite right.  The n operand is extracted unsigned, and 
> > only the m
> > operand is subjected to ABS.
>
> Yes, you're right (this patch does always extract ra (here n)
> unsigned, so the only case where it makes a difference is
> VMINAV when the input ra is negative). I noticed this wrinkle
> when implementing the FP variant of this insn, but missed it here.

In fact it doesn't make a difference for this integer version:
because we always extract n as an unsigned value, 'n' is always
positive (either because it's the initial unsigned 'ra' or
because it's the result of a previous do_maxa/do_mina, which
must also always be positive) and so taking the absolute value
of 'n' never affects it. But we might as well delete the unused code.

(For float it *does* make a difference, because there's no
"extract as unsigned" for floats, and so the input 'ra' and
thus the first 'n' can be negative there.)

-- PMM