[Bug target/94123] New: [10 regression] r10-7093 causes gcc.target/powerpc/pr87507.c to fail

[Bug target/94123] New: [10 regression] r10-7093 causes gcc.target/powerpc/pr87507.c to fail

[seurer at gcc dot gnu.org]

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

            Bug ID: 94123
           Summary: [10 regression] r10-7093 causes
                    gcc.target/powerpc/pr87507.c to fail
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: seurer at gcc dot gnu.org
  Target Milestone: ---

Executing on host: /home3/seurer/gcc/git/build/gcc-test2/gcc/xgcc
-B/home3/seurer/gcc/git/build/gcc-test2/gcc/
/home/seurer/gcc/git/gcc-test2/gcc/testsuite/gcc.target/powerpc/pr87507.c   
-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
-fdiagnostics-color=never  -fdiagnostics-urls=never  -O2 -mdejagnu-cpu=power8
-ffat-lto-objects -fno-ident -S -o pr87507.s    (timeout = 300)
spawn -ignore SIGHUP /home3/seurer/gcc/git/build/gcc-test2/gcc/xgcc
-B/home3/seurer/gcc/git/build/gcc-test2/gcc/
/home/seurer/gcc/git/gcc-test2/gcc/testsuite/gcc.target/powerpc/pr87507.c
-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
-fdiagnostics-color=never -fdiagnostics-urls=never -O2 -mdejagnu-cpu=power8
-ffat-lto-objects -fno-ident -S -o pr87507.s
PASS: gcc.target/powerpc/pr87507.c (test for excess errors)
gcc.target/powerpc/pr87507.c: \\mstd\\M found 6 times
FAIL: gcc.target/powerpc/pr87507.c scan-assembler-times \\mstd\\M 4
FAIL: gcc.target/powerpc/pr87507.c scan-assembler-not \\mld\\M
Executing on host: /home3/seurer/gcc/git/build/gcc-test2/gcc/xgcc
-B/home3/seurer/gcc/git/build/gcc-test2/gcc/ vmx_hw_available12365.c   
-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
-fdiagnostics-color=never  -fdiagnostics-urls=never  -mno-vsx  -lm  -o
vmx_hw_available12365.exe    (timeout = 300)
spawn -ignore SIGHUP /home3/seurer/gcc/git/build/gcc-test2/gcc/xgcc
-B/home3/seurer/gcc/git/build/gcc-test2/gcc/ vmx_hw_available12365.c
-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers
-fdiagnostics-color=never -fdiagnostics-urls=never -mno-vsx -lm -o
vmx_hw_available12365.exe
Setting LD_LIBRARY_PATH to
:/home3/seurer/gcc/git/build/gcc-test2/gcc::/home3/seurer/gcc/git/build/gcc-test2/gcc:/home/seurer/gcc/git/build/gcc-test2/./gmp/.libs:/home/seurer/gcc/git/build/gcc-test2/./prev-gmp/.libs:/home/seurer/gcc/git/build/gcc-test2/./mpfr/src/.libs:/home/seurer/gcc/git/build/gcc-test2/./prev-mpfr/src/.libs:/home/seurer/gcc/git/build/gcc-test2/./mpc/src/.libs:/home/seurer/gcc/git/build/gcc-test2/./prev-mpc/src/.libs:/home/seurer/gcc/git/build/gcc-test2/./isl/.libs:/home/seurer/gcc/git/build/gcc-test2/./prev-isl/.libs
Execution timeout is: 300
spawn [open ...]
testcase
/home/seurer/gcc/git/gcc-test2/gcc/testsuite/gcc.target/powerpc/powerpc.exp
completed in 1 seconds

                === gcc Summary ===

# of expected passes            1
# of unexpected failures        2


The test now generates a bunch more loads and stores.  Looks like it is
spilling some registers.

Old code (r10-7092):

.LFB0:
        .cfi_startproc
        cmpdi 0,3,0
        beqlr 0
        mr 10,6
        mr 11,5
        addi 3,4,16
        mr 8,10
        mr 9,11
        std 11,0(4)
        std 10,8(4)
        std 9,0(3)
        std 8,8(3)
        blr


New code (r10-7093):

.LFB0:
        .cfi_startproc
        cmpdi 0,3,0
        beqlr 0
        std 30,-16(1)
        std 31,-8(1)
        .cfi_offset 30, -16
        .cfi_offset 31, -8
        mr 30,6
        mr 31,5
        addi 9,4,16
        mr 10,30
        mr 11,31
        std 31,0(4)
        std 30,8(4)
        std 11,0(9)
        std 10,8(9)
        ld 30,-16(1)
        ld 31,-8(1)
        .cfi_restore 31
        .cfi_restore 30
        blr

[Bug target/94123] [10 regression] r10-7093 causes gcc.target/powerpc/pr87507.c to fail

[seurer at gcc dot gnu.org]

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

seurer at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |powerpc64le-linux-gnu
              Build|                            |powerpc64le-linux-gnu
               Host|                            |powerpc64le-linux-gnu
                 CC|                            |bergner at gcc dot gnu.org,
                   |                            |vmakarov at redhat dot com,
                   |                            |wschmidt at gcc dot gnu.org

--- Comment #1 from seurer at gcc dot gnu.org ---
git g:5dc1390b41db5c1765e25fd21dad1a930a015aac, r10-7093

[Bug target/94123] [10 regression] r10-7093 causes gcc.target/powerpc/pr87507.c to fail

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, did it fail also two weeks ago when the patch that got reverted wasn't even
in?

[Bug target/94123] [10 regression] r10-7093 causes gcc.target/powerpc/pr87507.c to fail

[seurer at gcc dot gnu.org]

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

--- Comment #3 from seurer at gcc dot gnu.org ---
Hmmm.  This test case did fail previously and was reported in pr91797 but that
was marked as fixed although the patch for that actually didn't change this
particular test case.

[Bug target/94123] [10 regression] r10-7093 causes gcc.target/powerpc/pr87507.c to fail

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.0

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[seurer at gcc dot gnu.org]

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

seurer at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10 regression] r10-7093    |[10 regression] r10-1734,
                   |causes                      |SVN r273240, causes
                   |gcc.target/powerpc/pr87507. |gcc.target/powerpc/pr87507.
                   |c to fail                   |c to fail
                 CC|                            |segher at gcc dot gnu.org

--- Comment #4 from seurer at gcc dot gnu.org ---
g:b18081df8cca5f2306e99709fa2c06b9cbeea8d0, r10-1734, SVN r273240

Sorry about the confusion with the reverted patch.

The current code generated for this test case is really messed up.  Prior to
r10-1734 this is what was generated for gcc.target/powerpc/pr87507.c

.cfi_startproc
        cmpdi 0,3,0
        beqlr 0
        std 5,0(4)
        std 6,8(4)
        std 5,16(4)
        std 6,24(4)
        blr

Starting with r10-1734 (and still today) this is what is generated:

.cfi_startproc
        cmpdi 0,3,0
        beqlr 0
        std 30,-16(1)
        std 31,-8(1)
        .cfi_offset 30, -16
        .cfi_offset 31, -8
        mr 30,6
        mr 31,5
        addi 9,4,16
        mr 10,30
        mr 11,31
        std 31,0(4)
        std 30,8(4)
        std 11,0(9)
        std 10,8(9)
        ld 30,-16(1)
        ld 31,-8(1)
        .cfi_restore 31
        .cfi_restore 30
        blr

All sorts of extra moves, loads, and stores.

Options used:  -O2 -mdejagnu-cpu=power8

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[segher at gcc dot gnu.org]

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

Segher Boessenkool <segher at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-03-11
     Ever confirmed|0                           |1

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Confirmed.

On p9 it is all fine.

On p7 it is worse, you get std's followed by an lxvd2x from the same stack
address (big LHS/SHL hazard there), and then two stxvd2x.

On p8 the TImode values aren't split at all, not until final output anyway.

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[bergner at gcc dot gnu.org]

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

Peter Bergner <bergner at gcc dot gnu.org> changed:

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

--- Comment #6 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #5)
> Confirmed.
> 
> On p9 it is all fine.
> 
> On p7 it is worse, you get std's followed by an lxvd2x from the same stack
> address (big LHS/SHL hazard there), and then two stxvd2x.
> 
> On p8 the TImode values aren't split at all, not until final output anyway.

My fix for PR87507 allowed them to be split early so they could be optimized,
so this must have regressed without us seeing it.  I'll have a quick look at
what is happening.

Richi, given this is a performance issue for TImode values, is there a reason
you want this as a P1 bug?

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Peter Bergner from comment #6)
> (In reply to Segher Boessenkool from comment #5)
> > Confirmed.
> > 
> > On p9 it is all fine.
> > 
> > On p7 it is worse, you get std's followed by an lxvd2x from the same stack
> > address (big LHS/SHL hazard there), and then two stxvd2x.
> > 
> > On p8 the TImode values aren't split at all, not until final output anyway.
> 
> My fix for PR87507 allowed them to be split early so they could be
> optimized, so this must have regressed without us seeing it.  I'll have a
> quick look at what is happening.
> 
> Richi, given this is a performance issue for TImode values, is there a
> reason you want this as a P1 bug?

Just default rules applied - the bug is new in GCC 10.  Since it's also a
testsuite regression it woudl be nice to at least make that clean.  If
we understand why the regression happens and can live with it we can demote
it to P2.

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[bergner at gcc dot gnu.org]

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

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |bergner at gcc dot gnu.org

--- Comment #8 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> Just default rules applied - the bug is new in GCC 10.  Since it's also a
> testsuite regression it woudl be nice to at least make that clean.  If
> we understand why the regression happens and can live with it we can demote
> it to P2.

So Segher's commit that caused this, added -fsplit-wide-types-early.  If you
use -fno-split-wide-types-early then we get the expected code.  It's strange
that both Segher's patch and my fix from PR87507 tries to break these TImode
uses up early and somehow, we're tripping over each other.  I will continue to
debug this and generate a fix.

That said, I think given there is a work around and that __int128 usage isn't
all that common, that we can probably reduce the priority of this to P2.

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P2

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK, please make sure to fix the FAIL for the release in some way (add
-fno-split-wide-types-early?)

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[bergner at gcc dot gnu.org]

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

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |https://gcc.gnu.org/piperma
                   |                            |il/gcc-patches/2020-March/5
                   |                            |42840.html

--- Comment #10 from Peter Bergner <bergner at gcc dot gnu.org> ---
I submitted 2 proposed patches that both fix the issue.  Waiting to see which
patch people prefer...if any.

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Peter Bergner <bergner@gcc.gnu.org>:

https://gcc.gnu.org/g:7546463b9f7a0b001cf61a94dcfc18f540721390

commit r10-7501-g7546463b9f7a0b001cf61a94dcfc18f540721390
Author: Peter Bergner <bergner@linux.ibm.com>
Date:   Wed Apr 1 14:24:07 2020 -0500

    subreg: Fix PR94123, SVN r273240 causes gcc.target/powerpc/pr87507.c to
fail

    Segher's patch that added -fsplit-wide-types-early and enabled by default
    for rs6000, caused pr87507.c to FAIL because when running lower-subreg
    earlier, we don't see any pseudo-to-pseudo copies of our wide type,
    which are created by combine, therefore, we skip decomposing our TImode
    accesses.  The fix here is just to always run the third pass of
lower-subreg
    instead of disabling it if we ran the second pass.

    2020-04-01  Peter Bergner  <bergner@linux.ibm.com>

            PR rtl-optimization/94123
            * lower-subreg.c (pass_lower_subreg3::gate): Remove test for
            flag_split_wide_types_early.

[Bug target/94123] [10 regression] r10-1734, SVN r273240, causes gcc.target/powerpc/pr87507.c to fail

[bergner at gcc dot gnu.org]

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

Peter Bergner <bergner at gcc dot gnu.org> changed:

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

--- Comment #12 from Peter Bergner <bergner at gcc dot gnu.org> ---
Fixed by always running the 3rd lower-subreg pass, regardless of whether we ran
the 2nd pass or not.

I will revisit the first patch above, where we allow hard regs copies to
trigger decomposing wide types when stage1 reopens.