[Bug testsuite/102861] New: [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[Bug testsuite/102861] New: [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[seurer at gcc dot gnu.org]

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

            Bug ID: 102861
           Summary: [12 regression] gcc.dg/vect/bb-slp-16.c fails after
                    r12-4526
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: testsuite
          Assignee: unassigned at gcc dot gnu.org
          Reporter: seurer at gcc dot gnu.org
  Target Milestone: ---

g:d8edfadfc7a9795b65177a50ce44fd348858e844, r12-4526

make  -k check-gcc RUNTESTFLAGS="vect.exp=gcc.dg/vect/bb-slp-16.c"
FAIL: gcc.dg/vect/bb-slp-16.c execution test
# of expected passes            5
# of unexpected failures        1


No messages from the failure:

Running /home/seurer/gcc/git/gcc-test/gcc/testsuite/gcc.dg/vect/vect.exp ...
FAIL: gcc.dg/vect/bb-slp-16.c execution test

                === gcc Summary ===

# of expected passes            5
# of unexpected failures        1


Hmmm.  Looks like this used to be xfailed.

commit d8edfadfc7a9795b65177a50ce44fd348858e844 (HEAD, refs/bisect/bad)
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Oct 4 09:47:02 2021 +0200

    Disallow loop rotation and loop header crossing in jump threaders.

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[aldyh at gcc dot gnu.org]

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

--- Comment #1 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
Similarly to 102860.

The referenced patch reduces the amount of threaded paths, not increase them. 
This actually looks like another pass hiccuping because it was expecting a
threaded path that is no longer there.

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Aldy Hernandez from comment #1)
> Similarly to 102860.
> 
> The referenced patch reduces the amount of threaded paths, not increase
> them.  This actually looks like another pass hiccuping because it was
> expecting a threaded path that is no longer there.

The commit changed vect/bb-slp-16.c by a lot ...

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org
   Target Milestone|---                         |12.0

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Yeah, I think these kind of testsuite changes are no good.

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[aldyh at gcc dot gnu.org]

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

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> Yeah, I think these kind of testsuite changes are no good.

The loop threading restrictions disallow a couple jump threads (rotating the
loop and crossing the loop header without exiting the loop).  The test was
depending on these invalid threads to trigger the "optimized: basic block"
diagnostic.  Jeff suggested manually rotating the loop to continue testing the
SLP functionality.

Is there something else we could do?

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[rguenther at suse dot de]

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

--- Comment #5 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 21 Oct 2021, aldyh at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102861
> 
> --- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #3)
> > Yeah, I think these kind of testsuite changes are no good.
> 
> The loop threading restrictions disallow a couple jump threads (rotating the
> loop and crossing the loop header without exiting the loop).  The test was
> depending on these invalid threads to trigger the "optimized: basic block"
> diagnostic.  Jeff suggested manually rotating the loop to continue testing the
> SLP functionality.
> 
> Is there something else we could do?

Look what exactly is missing and restore that - testcases are written
from real-world code that we expect to optimize, adjusting them to
hide the fact that we don't any longer is not good.

Even XFAIL would have been better.

That said, loop header copying is supposed to do the rotation.

Note what I see on the unchanged testcase is that we now loop vectorize
which is great (and obviously no BB vectorization opportunity remains).
So a different fix would be to simply add

/* Avoid loop vectorization.  */
/* { dg-additional-options "-fno-tree-loop-vectorize" } */

which makes the testcase PASS again (bonus for adding a variant
of the testcase that verifies we loop vectorize this).

Let me do all this for you.

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-10-21
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[aldyh at gcc dot gnu.org]

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

--- Comment #6 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
> > Is there something else we could do?
> 
> Look what exactly is missing and restore that - testcases are written
> from real-world code that we expect to optimize, adjusting them to
> hide the fact that we don't any longer is not good.
> 
> Even XFAIL would have been better.

Heh.  That was my original suggestion, but I was vetoed :).

> 
> That said, loop header copying is supposed to do the rotation.
> 
> Note what I see on the unchanged testcase is that we now loop vectorize
> which is great (and obviously no BB vectorization opportunity remains).
> So a different fix would be to simply add
> 
> /* Avoid loop vectorization.  */
> /* { dg-additional-options "-fno-tree-loop-vectorize" } */
> 
> which makes the testcase PASS again (bonus for adding a variant
> of the testcase that verifies we loop vectorize this).
> 
> Let me do all this for you.

Thanks so much!

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Btw,

  i = N;
  if (i > 0)
    {
      do
        {
          *pout++ = *pin++ + a;
          *pout++ = *pin++ + a;
          *pout++ = *pin++ + a;
          *pout++ = *pin++ + a;
          *pout++ = *pin++ + a;
          *pout++ = *pin++ + a;
          *pout++ = *pin++ + a;
          *pout++ = *pin++ + a;
          if (arr[i] = i)
            a = i;
          else
            a = 2;
        }
      while (i < N);

that was obviously bogus - you wanted if (N > 0) and initialize i to zero ...

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS 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:d438dd2523a35e073eed04d146f1e73e2514d93d

commit r12-4593-gd438dd2523a35e073eed04d146f1e73e2514d93d
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Oct 21 10:00:27 2021 +0200

    testsuite/102861 - adjust gcc.dg/vect/bb-slp-16.c change

    This reverts the bogus previous change causing runtime failures
    and instead realizes that we now have the loop condition
    if-converted and the BB vectorization opportunity realized during
    the loop vectorization pass.

    2021-10-21  Richard Biener  <rguenther@suse.de>

            PR testsuite/102861
            * gcc.dg/vect/bb-slp-16.c: Revert previous change, scan
            the vect dump instead.

[Bug testsuite/102861] [12 regression] gcc.dg/vect/bb-slp-16.c fails after r12-4526

[rguenth at gcc dot gnu.org]

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

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.