[Bug target/94254] New: [10 regression] r10-7312 causes compiler hangs

[Bug target/94254] New: [10 regression] r10-7312 causes compiler hangs

[seurer at linux dot vnet.ibm.com]

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

            Bug ID: 94254
           Summary: [10 regression] r10-7312 causes compiler hangs
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: seurer at linux dot vnet.ibm.com
  Target Milestone: ---

g:497498c878d48754318e486428e2aa30854020b9, r10-7312 

FAIL: gcc.target/powerpc/pr39902-2.c (test for excess errors)
FAIL: gcc.target/powerpc/pr79916.c (test for excess errors)
FAIL: gcc.target/powerpc/sd-pwr6.c (test for excess errors)

All 3 of those compilations hang starting with this revision.  I saw this on a
power 8 and power 9.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[zsojka at seznam dot cz]

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

Zdenek Sojka <zsojka at seznam dot cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zsojka at seznam dot cz

--- Comment #1 from Zdenek Sojka <zsojka at seznam dot cz> ---
I observe the same issue, and it breaks libgcc build for me:

$
/repo/build-trunk-r10-7320-20200321085330-g497498c878d-checking-yes-rtl-df-extra-powerpc/./gcc/xgcc
-B/repo/build-trunk-r10-7320-20200321085330-g497498c878d-checking-yes-rtl-df-extra-powerpc/./gcc/
-B/repo/gcc-trunk//binary-trunk-r10-7320-20200321085330-g497498c878d-checking-yes-rtl-df-extra-powerpc/powerpc-unknown-linux-gnu/bin/
-B/repo/gcc-trunk//binary-trunk-r10-7320-20200321085330-g497498c878d-checking-yes-rtl-df-extra-powerpc/powerpc-unknown-linux-gnu/lib/
-isystem
/repo/gcc-trunk//binary-trunk-r10-7320-20200321085330-g497498c878d-checking-yes-rtl-df-extra-powerpc/powerpc-unknown-linux-gnu/include
-isystem
/repo/gcc-trunk//binary-trunk-r10-7320-20200321085330-g497498c878d-checking-yes-rtl-df-extra-powerpc/powerpc-unknown-linux-gnu/sys-include
   -g -O2 -O2  -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -W -Wall
-Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes
-Wmissing-prototypes -Wold-style-definition  -isystem ./include   -fPIC
-mlong-double-128 -mno-minimal-toc -g -DIN_LIBGCC2 -fbuilding-libgcc
-fno-stack-protector   -fPIC -mlong-double-128 -mno-minimal-toc -I. -I.
-I../.././gcc -I/repo/gcc-trunk/libgcc -I/repo/gcc-trunk/libgcc/.
-I/repo/gcc-trunk/libgcc/../gcc -I/repo/gcc-trunk/libgcc/../include
-I/repo/gcc-trunk/libgcc/../libdecnumber/dpd
-I/repo/gcc-trunk/libgcc/../libdecnumber -DHAVE_CC_TLS  -o _sd_to_si.o -MT
_sd_to_si.o -MD -MP -MF _sd_to_si.dep -DFINE_GRAINED_LIBRARIES -DL_sd_to_si
-DWIDTH=32 -c /repo/gcc-trunk/libgcc/dfp-bit.c

hangs; I can provide reduced testcase if needed

(for reasons unknown to me, git gcc-descr returns "r10-7320" for me for the
same  git checkout as in #c0)

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[zsojka at seznam dot cz]

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

--- Comment #2 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Zdenek Sojka from comment #1)
> I observe the same issue, and it breaks libgcc build for me:
...
> 
> (for reasons unknown to me, git gcc-descr returns "r10-7320" for me for the
> same  git checkout as in #c0)

Sorry, I accidentally sent the message before I removed this part of the
comment; apparently git gcc-descr works with the most recent commit in the
branch, not with the actual checkout.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rsandifo at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
                 CC|                            |rsandifo at gcc dot gnu.org
   Last reconfirmed|                            |2020-03-22

--- Comment #3 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Mine.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[rsandifo at gcc dot gnu.org]

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

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
The cycling comes from reloading:

(insn 7 6 8 2 (set (reg:SD 122 [ a32 ])
        (mem/c:SD (reg/f:DI 120) [1 a32+0 S4 A32]))
"gcc/testsuite/gcc.target/powerpc/pr39902-2.c":15:13 516 {movsd_hardfloat}
     (expr_list:REG_DEAD (reg/f:DI 120)
        (nil)))

r122 is assigned an FPR, and the power6 pattern doesn't provide
any alternatives that load from memory into FPRs, so we convert
this into a secondary memory reload.

Doing that for SDmode memory would cycle, but the rs6000 port has:

/* Implement TARGET_SECONDARY_RELOAD_NEEDED_MODE.  For SDmode values we         
   need to use DDmode, in all other cases we can use the same mode.  */
static machine_mode
rs6000_secondary_memory_needed_mode (machine_mode mode)
{
  if (lra_in_progress && mode == SDmode)
    return DDmode;
  return mode;
}

which says that the move should happen in DDmode instead.
This means that the eventual FPR reload will happen in DDmode
rather than SDmode.

The problem is that rs6000_can_change_mode_class doesn't allow
FPRs to change from SDmode to DDmode:

          if (from_size < 8 || to_size < 8)
            return false;

So there seems to be a contradiction here: secondary memory
reloads for FPRs have to happen in DDmode rather than SDmode,
but FPRs aren't allowed to change to DDmode from SDmode.

Previously this worked because LRA ignored
rs6000_can_change_mode_class and changed the mode of the
FPR regardless.  I guess that must have been the right
thing to do in context, but it would be good to pin down
exactly why the SD->DD mode change is OK for rs6000 in the
specific context of secondary memory reloads but not otherwise.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[rsandifo at gcc dot gnu.org]

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

--- Comment #5 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Zdenek Sojka from comment #1)
> I observe the same issue, and it breaks libgcc build for me:

What configure arguments do you use?

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[zsojka at seznam dot cz]

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

--- Comment #6 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to rsandifo@gcc.gnu.org from comment #5)
> (In reply to Zdenek Sojka from comment #1)
> > I observe the same issue, and it breaks libgcc build for me:
> 
> What configure arguments do you use?

Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl
--with-sysroot=/usr/powerpc-unknown-linux-gnu --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=powerpc-unknown-linux-gnu
--with-ld=/usr/bin/powerpc-unknown-linux-gnu-ld
--with-as=/usr/bin/powerpc-unknown-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r10-7229-20200317212116-cd0b7124273-checking-yes-rtl-df-extra-powerpc

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[rsandifo at gcc dot gnu.org]

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

--- Comment #7 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Created attachment 48080
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48080&action=edit
Proof-of-concept hack to back up the point in comment 4

This hack shows what I mean in comment 4.  It "fixes" the three
testcases but almost certainly isn't correct.  The point is that
before r10-7312 we did the mode change regardless of what
rs6000_can_change_mode_class said.

I don't know enough about powerpc to know what cases:

          if (from_size < 8 || to_size < 8)
            return false;

is handling.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[segher at gcc dot gnu.org]

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

--- Comment #8 from Segher Boessenkool <segher at gcc dot gnu.org> ---
SFmode values are stored as DP IEEE float normally.  There may be other
cases as well, but this is the obvious one.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.0
           Keywords|                            |ra
           Priority|P3                          |P1

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[rsandifo at gcc dot gnu.org]

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

--- Comment #9 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #8)
> SFmode values are stored as DP IEEE float normally.  There may be other
> cases as well, but this is the obvious one.

OK, thanks.  Does the same problem affect decimal floats (i.e. SD an DD)?

If not, maybe the answer really is to allow these cases through,
but probably without the lra_in_progress check in the hack/patch.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[segher at gcc dot gnu.org]

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

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

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

--- Comment #10 from Segher Boessenkool <segher at gcc dot gnu.org> ---
An SDmode value is stored in the rightmost 32 bits of an FPR, while a DDmode
one uses all 64 bits of an FPR.  So yeah I think you're plan is correct and
will work.

(Cc: Mike, he should know more, I only read manuals :-) )

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #48080|0                           |1
        is obsolete|                            |

--- Comment #11 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Created attachment 48088
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48088&action=edit
Candidate patch

OK, thanks. This version drops the lra_in_progress hack and
makes the test symmetric.  As mentioned in the commit
message, I'd wondered about generalising it further and
allowing any 64-bit mode, but that seems unnecessarily
dangerous for stage 4.

The previous hack passed bootstrap & regtest on gcc112.
I'll try with this version too.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[segher at gcc dot gnu.org]

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

--- Comment #12 from Segher Boessenkool <segher at gcc dot gnu.org> ---
That patch looks fine, thank you!

Is there some way you can test if this works?  Ideally in the testsuite
of course, but that can be hard :-/

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[rsandifo at gcc dot gnu.org]

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

--- Comment #13 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #12)
> That patch looks fine, thank you!
> 
> Is there some way you can test if this works?  Ideally in the testsuite
> of course, but that can be hard :-/

I've now tried doing a bootstrap & regtest with --with-cpu-power6 as well,
comparing without the lra patch with after both patches.  (With master the
build hangs in the same way as it did for Zdenek.)  There didn't seem
to be any differences in test results.

Hopefully that will have given some execution test coverage when
running things like gcc.dg/dfp and c-c++-common/dfp.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[zsojka at seznam dot cz]

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

--- Comment #14 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to rsandifo@gcc.gnu.org from comment #11)
> Created attachment 48088 [details]
> Candidate patch

It fixes the build for me.
I am unable to find a way how to run the testsuite on a non-native system
(using qemu userspace emulation).

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[segher at gcc dot gnu.org]

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

--- Comment #15 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #13)
> (In reply to Segher Boessenkool from comment #12)
> > That patch looks fine, thank you!
> > 
> > Is there some way you can test if this works?  Ideally in the testsuite
> > of course, but that can be hard :-/
> 
> I've now tried doing a bootstrap & regtest with --with-cpu-power6 as well,
> comparing without the lra patch with after both patches.  (With master the
> build hangs in the same way as it did for Zdenek.)  There didn't seem
> to be any differences in test results.
> 
> Hopefully that will have given some execution test coverage when
> running things like gcc.dg/dfp and c-c++-common/dfp.

Excellent :-)


Segher

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[segher at gcc dot gnu.org]

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

--- Comment #16 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Zdenek Sojka from comment #14)
> (In reply to rsandifo@gcc.gnu.org from comment #11)
> > Created attachment 48088 [details]
> > Candidate patch
> 
> It fixes the build for me.
> I am unable to find a way how to run the testsuite on a non-native system
> (using qemu userspace emulation).

Yeah, with qemu it is harder than with proper emulators (qemu does not
mimic any existing cpu model closely).  It should be possible if you do
some magic in a boards file.

Maybe ask on gcc@?  And/or start a topic for it on the wiki?

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[doko at debian dot org]

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

Matthias Klose <doko at debian dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |doko at debian dot org

--- Comment #17 from Matthias Klose <doko at debian dot org> ---
*** Bug 94318 has been marked as a duplicate of this bug. ***

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[law at redhat dot com]

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

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com

--- Comment #18 from Jeffrey A. Law <law at redhat dot com> ---
I put the candidate patch into my tester which will run it on ppc64 and ppc64le
native.  Both ran fine and it fixed the regressions associated with Richard's
LRA patch.  They also both successfully built glibc and the kernel.

Given Segher's c#12, I'm going to go ahead and commit the patch from c#11.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[cvs-commit at gcc dot gnu.org]

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

--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jeff Law <law@gcc.gnu.org>:

https://gcc.gnu.org/g:713ecb3d417363a4b12c725b335fce10355da206

commit r10-7380-g713ecb3d417363a4b12c725b335fce10355da206
Author: Jeff Law <law@redhat.com>
Date:   Wed Mar 25 10:37:31 2020 -0600

        rs6000: Allow FPRs to change between SDmode and DDmode [PR94254]

        g:497498c878d48754318e486428e2aa30854020b9 caused lra to cycle
        on some SDmode reloads for power6.  As explained in more detail
        in the PR comments, the problem was a conflict between two target
        hooks: rs6000_secondary_memory_needed_mode required SDmode FPR
        reloads to use DDmode memory (rightly, since using SDmode memory
        wouldn't make progress) but rs6000_can_change_mode_class didn't
        allow FPRs to change from SDmode to DDmode.  Previously lra
        ignored that and changed the mode anyway.

        From what Segher says, it sounds like the "from_size < 8 || to_size <
8"
        check is mostly there for SF<->64-bit subregs, and that SDmode is
stored
        in the way that target-independent code expects.  This patch therefore
        allows SD<->DD changes.

        I wondered about checking for SD<->64-bit changes instead, but that
        seemed like an unnecessary generalisation for this stage.

        2020-03-23  Richard Sandiford  <richard.sandiford@arm.com>

        gcc/
                PR target/94254
                * config/rs6000/rs6000.c (rs6000_can_change_mode_class): Allow
                FPRs to change between SDmode and DDmode.

[Bug target/94254] [10 regression] r10-7312 causes compiler hangs

[law at redhat dot com]

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

Jeffrey A. Law <law at redhat dot com> changed:

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

--- Comment #20 from Jeffrey A. Law <law at redhat dot com> ---
Fixed by Richard's patch.