[Bug target/115254] New: [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[Bug target/115254] New: [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

            Bug ID: 115254
           Summary: [15 Regression] GCN regressions from "Avoid splitting
                    store dataref groups during SLP discovery"
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: testsuite-fail
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tschwinge at gcc dot gnu.org
                CC: ams at gcc dot gnu.org, rguenth at gcc dot gnu.org
  Target Milestone: ---
            Target: GCN

If my tracking is to be believed, the recent commit
r15-812-gc71886f2ca2e46ce1449c7064d6f1b447d02fcba "Avoid splitting store
dataref groups during SLP discovery" is causing the following regressions for
GCN target testing (tested '-march=gfx908'):

    PASS: gcc.dg/vect/slp-cond-2-big-array.c (test for excess errors)
    PASS: gcc.dg/vect/slp-cond-2-big-array.c execution test
    [-PASS:-]{+FAIL:+} gcc.dg/vect/slp-cond-2-big-array.c scan-tree-dump-times
vect "vectorizing stmts using SLP" 3

    gcc.dg/vect/slp-cond-2-big-array.c: pattern found 4 times


    PASS: gcc.dg/vect/slp-cond-2.c (test for excess errors)
    PASS: gcc.dg/vect/slp-cond-2.c execution test
    [-PASS:-]{+FAIL:+} gcc.dg/vect/slp-cond-2.c scan-tree-dump-times vect
"vectorizing stmts using SLP" 3

    gcc.dg/vect/slp-cond-2.c: pattern found 4 times


    PASS: gcc.dg/vect/vect-gather-4.c (test for excess errors)
    [-PASS:-]{+FAIL:+} gcc.dg/vect/vect-gather-4.c scan-tree-dump-not vect
"Loop contains only SLP stmts"

Per commit 85e2ce10f76aee93e43aab6558cf8e39cec911e4 "Fix
gcc.dg/vect/vect-gather-4.c for cascadelake", that one later additionally
changed:

    FAIL: gcc.dg/vect/vect-gather-4.c scan-tree-dump-not vect [-"Loop contains
only SLP stmts"-]{+"vectorizing stmts using SLP"+}

..., but that's not relevant for the original regression.


In other words, if I locally revert that patch, these all PASS again.

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-05-28
   Target Milestone|---                         |15.0
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
For gcc.dg/vect/slp-cond-2-big-array.c 4 is indeed expected.  We run into

slp-cond-2-big-array.c:39:17: note:   SLP discovery limit exceeded

for f3 on x86-64.  I have a patch to cherry-pick to avoid this.

gcc.dg/vect/slp-cond-2.c is the same testcase.

gcc.dg/vect/vect-gather-4.c is a bad written testcase, we now indeed expect
to SLP here.

I'll see to pick the change that should help a bit.

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Note for gcc.dg/vect/vect-gather-4.c with -mgather and gather support in the
ISA on x86_64 I get two 'vectorizing stmts using SLP', for f1 and f2 only.

Does that match GCN?

We unfortunately cannot handle masked gathers as "emulated".

And we don't have good dejagnu target selectors for this either.

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:eaaa4b88038d4d6eda1b20ab662f1568fd9be31f

commit r15-859-geaaa4b88038d4d6eda1b20ab662f1568fd9be31f
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Sep 29 15:12:54 2023 +0200

    tree-optimization/115254 - don't account single-lane SLP against discovery
limit

    The following avoids accounting single-lane SLP to the discovery
    limit.  As the two testcases show this makes discovery fail,
    unfortunately even not the same across targets.  The following
    should fix two FAILs for GCN as a side-effect.

            PR tree-optimization/115254
            * tree-vect-slp.cc (vect_build_slp_tree): Only account
            multi-lane SLP to limit.

            * gcc.dg/vect/slp-cond-2-big-array.c: Expect 4 times SLP.
            * gcc.dg/vect/slp-cond-2.c: Likewise.

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
The gcc.dg/vect/vect-gather-4.c FAIL should be still present.

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

--- Comment #5 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
Created attachment 58300
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58300&action=edit
GCN target ('-march=gfx908') 'vect-gather-4.c.179t.vect'

(In reply to GCC Commits from comment #3)
> commit r15-859-geaaa4b88038d4d6eda1b20ab662f1568fd9be31f

>             * gcc.dg/vect/slp-cond-2-big-array.c: Expect 4 times SLP.
>             * gcc.dg/vect/slp-cond-2.c: Likewise.

ACK, again PASS for GCN target ('-march=gfx908'), thanks!


(In reply to Richard Biener from comment #4)
> The gcc.dg/vect/vect-gather-4.c FAIL should be still present.

Yes.

(In reply to Richard Biener from comment #2)
> Note for gcc.dg/vect/vect-gather-4.c with -mgather and gather support in the
> ISA on x86_64 I get two 'vectorizing stmts using SLP', for f1 and f2 only.
> 
> Does that match GCN?

In addition to 'f1', 'f2', GCN target ('-march=gfx908') apparently can do 'f3',
too:

    [...]/gcc.dg/vect/vect-gather-4.c:37:21: note:   vectorizing stmts using
SLP.

Attaching that 'vect-gather-4.c.179t.vect'.

> We unfortunately cannot handle masked gathers as "emulated".
> 
> And we don't have good dejagnu target selectors for this either.

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Thomas Schwinge from comment #5)
[..]
> (In reply to Richard Biener from comment #2)
> > Note for gcc.dg/vect/vect-gather-4.c with -mgather and gather support in the
> > ISA on x86_64 I get two 'vectorizing stmts using SLP', for f1 and f2 only.
> > 
> > Does that match GCN?
> 
> In addition to 'f1', 'f2', GCN target ('-march=gfx908') apparently can do
> 'f3', too:
> 
>     [...]/gcc.dg/vect/vect-gather-4.c:37:21: note:   vectorizing stmts using
> SLP.
> 
> Attaching that 'vect-gather-4.c.179t.vect'.

Yeah, so GCN can handle all gathers.

> > We unfortunately cannot handle masked gathers as "emulated".
> > 
> > And we don't have good dejagnu target selectors for this either.

Which we'd need to "fix" this - note we didn't check at all that the
loops are vectorized!  What we did want to check is that we do not
mangle both feeding masked gathers into the same SLP branch, but we
have really no indicator for this now.

I suppose we could change this to scan

note:   node 0x4300808 (max_nunits=64, refcnt=1) vector(64) int
note:    op template: patt_34 = .MASK_GATHER_LOAD ((sizetype) _71, _5, 4, 0,
_37);
note:    stmt 0 patt_34 = .MASK_GATHER_LOAD ((sizetype) _71, _5, 4, 0, _37);
note:    children 0x4300560 0x43004d8

specifically _not_

note:    stmt 1 ... = .MASK_GATHER_LOAD

but then on x86-64 you'd not see .MASK_GATHER_LOAD, neither for emulated
gather discovery.  And you _do_ have a 'stmt 1' for the SLP store.
On x86-64 with native gather support there's .MASK_LOAD, so I suppose
given we know we cannot emulate a mask gather we can change it to
a scan-not of 'stmt 1 .* = .MASK'

The following works for me - does it work for you?

diff --git a/gcc/testsuite/gcc.dg/vect/vect-gather-4.c
b/gcc/testsuite/gcc.dg/vect/vect-gather-4.c
index d18094d6982..edd9a6783c2 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-gather-4.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-gather-4.c
@@ -45,4 +45,7 @@ f3 (int *restrict y, int *restrict x, int *restrict indices)
     }
 }

-/* { dg-final { scan-tree-dump-not "vectorizing stmts using SLP" vect } } */
+/* We do not want to see a two-lane .MASK_LOAD or .MASK_GATHER_LOAD since
+   the gathers are different on each lane.  This is a bit fragile and
+   should possibly be turned into a runtime test.  */
+/* { dg-final { scan-tree-dump-not "stmt 1 \[^\r\n\]* = .MASK" vect } } */

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

--- Comment #7 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #6)
> The following works for me - does it work for you?

> --- a/gcc/testsuite/gcc.dg/vect/vect-gather-4.c
> +++ b/gcc/testsuite/gcc.dg/vect/vect-gather-4.c

> -/* { dg-final { scan-tree-dump-not "vectorizing stmts using SLP" vect } } */
> +/* We do not want to see a two-lane .MASK_LOAD or .MASK_GATHER_LOAD since
> +   the gathers are different on each lane.  This is a bit fragile and
> +   should possibly be turned into a runtime test.  */
> +/* { dg-final { scan-tree-dump-not "stmt 1 \[^\r\n\]* = .MASK" vect } } */

Yes:

    PASS: gcc.dg/vect/vect-gather-4.c (test for excess errors)
    PASS: gcc.dg/vect/vect-gather-4.c scan-tree-dump-not vect "stmt 1 [^\r\n]*
= .MASK"

But indeed, "a bit fragile".  ;-)

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:d8d70b783765361a8acef70fc9b54db526cd6ff5

commit r15-862-gd8d70b783765361a8acef70fc9b54db526cd6ff5
Author: Richard Biener <rguenther@suse.de>
Date:   Tue May 28 15:55:59 2024 +0200

    target/115254 - fix gcc.dg/vect/vect-gather-4.c dump scanning

    The dump scanning is supposed to check that we do not merge two
    sligtly different gathers into one SLP node but since we now
    SLP the store scanning for "ectorizing stmts using SLP" is no
    longer good.  Instead the following makes us look for
    "stmt 1 .* = .MASK" which would be how the second lane of an SLP
    node looks like.  We have to handle both .MASK_GATHER_LOAD (for
    targets with ifun mask gathers) and .MASK_LOAD (for ones without).

    Tested on x86_64-linux with and without native gather and on GCN
    where this now avoids a FAIL.

            PR target/115254
            * gcc.dg/vect/vect-gather-4.c: Adjust dump scan.

[Bug target/115254] [15 Regression] GCN regressions from "Avoid splitting store dataref groups during SLP discovery"

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115254

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed then.