Re: [PATCH 2/3] testsuite: Require 128-bit vectors for bb-slp-pr95839.c

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Jul 11, 2023 at 5:01 PM Maciej W. Rozycki <macro@embecosm.com> wrote:
>
> On Fri, 7 Jul 2023, Richard Biener wrote:
>
> > > The bb-slp-pr95839.c test assumes quad-single float vector support, but
> > > some targets only support pairs of floats, causing this test to fail
> > > with such targets.  Limit this test to targets that support at least
> > > 128-bit vectors then, and add a complementing test that can be run with
> > > targets that have support for 64-bit vectors only.  There is no need to
> > > adjust bb-slp-pr95839-2.c as 128 bits are needed even for the smallest
> > > vector of doubles, so support is implied by the presence of vectors of
> > > doubles.
> >
> > I wonder why you see the testcase FAIL, on x86-64 when doing
> >
> > typedef float __attribute__((vector_size(32))) v4f32;
> >
> > v4f32 f(v4f32 a, v4f32 b)
> > {
> >   /* Check that we vectorize this CTOR without any loads.  */
> >   return (v4f32){a[0] + b[0], a[1] + b[1], a[2] + b[2], a[3] + b[3],
> >   a[4] + b[4], a[5] + b[5], a[6] + b[6], a[7] + b[7]};
> > }
> >
> > I see we vectorize the add and the "store".  We fail to perform
> > extraction from the incoming vectors (unless you enable AVX),
> > that's a missed optimization.
> >
> > So with paired floats I would expect sth similar?  Maybe
> > x86 is saved by kind-of-presence (but disabled) of V8SFmode vectors.
>
>  I am not familiar enough with this stuff to answer your question.
>
>  As we pass and return V2SF data in FP registers just as with complex
> float data with this hardware the function from my bb-slp-pr95839-v8.c
> expands to a single vector FP add instruction, followed by a function
> return.
>
>  Conversely, the original function from bb-slp-pr95839.c expands to a
> sequence of 22 instructions to extract incoming vector FP data from 4
> 64-bit GPRs into 8 FPRs, add the vectors piecemeal with 4 scalar FP add
> instructions, and then insert outgoing vector FP data from 4 FPRs back to
> 2 64-bit GPRs.  As an experiment I have modified the backend minimally so
> as to pass and return V4SF data in FP registers as well, but that didn't
> make the vectoriser trigger.
>
> > That said, we should handle this better so can you file an
> > enhancement bugreport for this?
>
>  Filed as PR -optimization/110630.

Thanks!

>  I can't publish RISC-V information
> related to the hardware affected, but as a quick check I ran the MIPS
> compiler:
>
> $ mips-linux-gnu-gcc -march=mips64 -mabi=64 -mpaired-single -O2 -S bb-slp-pr95839*.c
>
> and got this code for bb-slp-pr95839-v8.c (mind the branch delay slot):
>
>         jr      $31
>         add.ps  $f0,$f12,$f13
>
> vs code for bb-slp-pr95839.c:
>
>         daddiu  $sp,$sp,-64
>         sd      $5,24($sp)
>         sd      $7,40($sp)
>         lwc1    $f0,24($sp)
>         lwc1    $f1,40($sp)
>         sd      $4,16($sp)
>         sd      $6,32($sp)
>         add.s   $f3,$f0,$f1
>         lwc1    $f0,28($sp)
>         lwc1    $f1,44($sp)
>         lwc1    $f4,36($sp)
>         swc1    $f3,56($sp)
>         add.s   $f2,$f0,$f1
>         lwc1    $f0,16($sp)
>         lwc1    $f1,32($sp)
>         swc1    $f2,60($sp)
>         add.s   $f1,$f0,$f1
>         lwc1    $f0,20($sp)
>         ld      $3,56($sp)
>         add.s   $f0,$f0,$f4
>         swc1    $f1,48($sp)
>         swc1    $f0,52($sp)
>         ld      $2,48($sp)
>         jr      $31
>         daddiu  $sp,$sp,64
>
> so this is essentially the same scenario (up to the machine instruction
> count), and therefore it seems backend-agnostic.  I can imagine the latter
> case could expand to something like (instruction reordering surely needed
> for performance omitted for clarity):
>
>         dmtc1   $4,$f0
>         dmtc1   $5,$f1
>         dmtc1   $6,$f2
>         dmtc1   $7,$f3
>         add.ps  $f0,$f0,$f1
>         add.ps  $f2,$f2,$f3
>         dmfc1   $2,$f0
>         jr      $31
>         dmfc1   $3,$f2
>
> saving a lot of cycles, and removing the need for spilling temporaries to
> the stack and for frame creation in the first place.
>
>  Do you agree it still makes sense to include bb-slp-pr95839-v8.c with the
> testsuite?

Sure, more coverage is always  nice.

Richard.

>   Maciej