On Fri, Oct 23, 2020 at 01:22:31PM -0500, Segher Boessenkool wrote:
> On Fri, Oct 23, 2020 at 04:45:29PM +1030, Alan Modra wrote:
> > Revised patch, removing changes to
> > gcc.target/powerpc/fold-vec-st-double.c,
> > gcc.target/powerpc/fold-vec-st-longlong.c,
> > gcc.target/powerpc/fold-vec-st-pixel.c. Fixing fails on those three
> > tests will be the subject of another patch.
>
> Okido.
>
> > Most of these changes are fairly obvious. Duplicated setbcr in
> > +/* { dg-final { scan-assembler-times {\maddic\M|\msetbcr\M} 4 } } */
> > +/* { dg-final { scan-assembler-times {\msubfe\M|\msetbcr\M} 1 } } */
> > is due to addic;subfe being replaced in one function with setbcr.
>
> But that won't really work. If there is more than one addic replaced by
> setbcr, that second scan fails (because it matches at least two times
> then).
Sure it works. I did test the patch! If future code gen changes, the
test will need updating at that point. scan-assembler tests require
maintenance..
> > * gcc.dg/pr56727-2.c,
> ...
> > * gcc.target/powerpc/ppc-eq0-1.c,
> > * gcc.target/powerpc/ppc-ne0-1.c,
> > * gcc.target/powerpc/pr86731-fwrapv-longlong.c: Match power10 insns.
>
> This should all be behind only one "*" (so delete it on all but the
> first line here).
Huh, ok.
> > --- a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-bb-slp-9a-pr63175.c
> > +++ b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-bb-slp-9a-pr63175.c
> > @@ -25,6 +25,6 @@ main1 (void)
> > with no word loads (lw, lwu, lwz, lwzu, or their indexed forms)
> > or word stores (stw, stwu, stwx, stwux, or their indexed forms). */
> >
> > -/* { dg-final { scan-assembler "\t(lvx|lxv|lvsr|stxv)" } } */
> > +/* { dg-final { scan-assembler "\t(lvx|lxv|lvsr|stxv|plxv|pstxv)" } } */
>
> /* { dg-final { scan-assembler "\t(lvx|p?lxv|lvsr|p?stxv)" } } */
> might be more readable/maintainable/extensible?
OK.
> > --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-load-builtin_vec_xl-char.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-load-builtin_vec_xl-char.c
> > @@ -36,4 +36,4 @@ BUILD_VAR_TEST( test10, vector unsigned char, signed long
> > long, vector unsigned
> > BUILD_VAR_TEST( test11, vector unsigned char, signed int, vector unsigned
> > char);
> > BUILD_CST_TEST( test12, vector unsigned char, 8, vector unsigned char);
> >
> > -/* { dg-final { scan-assembler-times
> > {\mlxvw4x\M|\mlxvd2x\M|\mlxvx\M|\mlvx\M} 12 } } */
> > +/* { dg-final { scan-assembler-times
> > {\mlxvw4x\M|\mlxvd2x\M|\mlxvx\M|\mlvx\M|\mplxv\M} 12 } } */
>
> Here, it did not allow lxv before. Should it?
>
> (in many files)
We don't generate lxv due to the insn only supporting DQ offsets. For
example, on power9 test3_cst from this test is
li 9,2
lxvx 34,3,9
blr
On power10
plxv 34,2(3)
blr
> Have you verified the p10 code generation actually makes sense?
Yes, I did.
> > --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-splat-longlong.c
> > @@ -25,7 +25,7 @@ vector signed long long test_sll () { const vector signed
> > long long y = {34, 45}
> > vector unsigned long long test_ull () { const vector unsigned long long y
> > = {56, 67}; return vec_splat (y, 0b00010); }
> >
> > /* Assorted load instructions for the initialization with known constants.
> > */
> > -/* { dg-final { scan-assembler-times {\mlvx\M|\mlxvd2x\M|\mlxv\M} 3 } } */
> > +/* { dg-final { scan-assembler-times
> > {\mlvx\M|\mlxvd2x\M|\mlxv\M|\mplxv\M|\mplxv\M} 3 } } */
>
> You have plxv twice here.
Oops.
> > --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-store-builtin_vec_xst-char.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-store-builtin_vec_xst-char.c
> > @@ -36,4 +36,4 @@ BUILD_VAR_TEST( test10, vector unsigned char, signed
> > long long, vector unsigned
> > BUILD_VAR_TEST( test11, vector unsigned char, signed int, vector unsigned
> > char );
> > BUILD_CST_TEST( test12, vector unsigned char, 12, vector unsigned char );
> >
> > -/* { dg-final { scan-assembler-times
> > {\mstxvw4x\M|\mstxvd2x\M|\mstxvx\M|\mstvx\M} 12 } } */
> > +/* { dg-final { scan-assembler-times
> > {\mstxvw4x\M|\mstxvd2x\M|\mstxvx\M|\mstvx\M|\mpstxv\M} 12 } } */
>
> Similarly, should it have plain stxv as well?
No. Same reason as to why lxv isn't generated.
> > --- a/gcc/testsuite/gcc.target/powerpc/lvsl-lvsr.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/lvsl-lvsr.c
> > @@ -1,12 +1,10 @@
> > -/* Test expected code generation for lvsl and lvsr on little endian.
> > - Note that lvsl and lvsr are each produced once, but the filename
> > - causes them to appear twice in the file. */
> > +/* Test expected code generation for lvsl and lvsr on little endian. */
> >
> > /* { dg-do compile { target { powerpc64le-*-* } } } */
> > /* { dg-options "-O0 -Wno-deprecated" } */
> > -/* { dg-final { scan-assembler-times "lvsl" 2 } } */
> > -/* { dg-final { scan-assembler-times "lvsr" 2 } } */
> > -/* { dg-final { scan-assembler-times {\mlxvd2x\M|\mlxv\M} 2 } } */
> > +/* { dg-final { scan-assembler-times {\slvsl\s} 1 } } */
> > +/* { dg-final { scan-assembler-times {\slvsr\s} 1 } } */
>
> This could use a comment (we normally use \m \M).
Seriously? That's about as useful as "i++; /* increment i */"
> Better is to just rename the file, of course :-)
I thought of that, but I'm not doing it.
> > +/* { dg-final { scan-assembler-times {\mlxvd2x\M|\mlxv\M|\mplxv\M} 2 } } */
>
> \mp?lxv\M
Yeah, I'll change all of those.
> > --- a/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/ppc-ne0-1.c
> > @@ -2,9 +2,9 @@
> > /* { dg-do compile } */
> > /* { dg-options "-O2 -mno-isel" } */
> >
> > -/* { dg-final { scan-assembler-times "addic" 4 } } */
> > -/* { dg-final { scan-assembler-times "subfe" 1 } } */
> > -/* { dg-final { scan-assembler-times "addze" 3 } } */
> > +/* { dg-final { scan-assembler-times {\maddic\M|\msetbcr\M} 4 } } */
> > +/* { dg-final { scan-assembler-times {\msubfe\M|\msetbcr\M} 1 } } */
> > +/* { dg-final { scan-assembler-times {\maddze\M} 3 } } */
>
> So this one won't work like this.
But it does.
--
Alan Modra
Australia Development Lab, IBM
