[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 Jeffrey A. Law changed: What|Removed |Added Resolution|--- |FIXED Status|REOPENED|RESOLVED --- Comment #11 from Jeffrey A. Law --- Fixed, correctly this time, on the trunk.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244
--- Comment #10 from GCC Commits ---
The master branch has been updated by Jeff Law :
https://gcc.gnu.org/g:388910144a3f11ba61208fc80afb2fa78d657163
commit r15-7428-g388910144a3f11ba61208fc80afb2fa78d657163
Author: Jeff Law
Date: Fri Feb 7 09:10:59 2025 -0700
[rtl-optimization/116244] Don't create bogus regs in alter_subreg
> Jeff Law writes:
>> So pulling on this thread leads me into the code that sets up
>> ALLOCNO_WMODE in create_insn_allocnos:
>>
>>>if ((a = ira_curr_regno_allocno_map[regno]) == NULL)
>>> {
>>>a = ira_create_allocno (regno, false,
ira_curr_loop_tree_node);
>>>if (outer != NULL && GET_CODE (outer) == SUBREG)
>>> {
>>>machine_mode wmode = GET_MODE (outer);
>>>if (partial_subreg_p (ALLOCNO_WMODE (a), wmode))
>>> ALLOCNO_WMODE (a) = wmode;
>>> }
>>> }
>> Note how we only set ALLOCNO_MODE only at allocno creation, so it'll
>> work as intended if and only if the first reference is via a SUBREG.
>
> Huh, yeah, I agree that that looks wrong.
>
>> ISTM the fix here is to always do the check and set ALLOCNO_WMODE.
>>[ Snipped discussion on a non-issue. ]
>
> So ISTM that moving the code out of the "if (... == NULL)" should be
> enough on its own.
>
>> And it all makes sense that you caught this. You and another colleague
>> at ARM were trying to address this exact problem ~11 years ago ;-)
>
> Heh, thought it sounded familiar :)
So attached is the updated patch that adjusts IRA to avoid this problem.
Georg-Johann, this may explain an issue you were running into as well where
you
got an invalid allocation. I think yours was at the higher end of the
register
file, but the core issue is potentially the same (looking at the first use
rather than all of them for paradoxical subregs).
I've had this in my tester about a week. So it's been through the crosses
as
well as various native bootstraps, including but not limited to m68k, ppc,
s390, hppa, sh4, etc. And just for good measure I bootstrapped &
regression
tested it on x86_64 a few minutes ago.
Pushing to the trunk.
PR rtl-optimization/116244
gcc/
* ira-build.cc (create_insn_allocnos): Do not restrict the check
for
subreg uses to allocno creation time. Do it for all uses.
gcc/testsuite/
* g++.target/m68k/m68k.exp: New test driver.
* g++.target/m68k/pr116244.C: New test.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 --- Comment #9 from Jeffrey A. Law --- Starting to think this is actually an IRA bug that was partially fixed about 11 years ago. Testing a potential fix.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 --- Comment #8 from Jeffrey A. Law --- The failure is limited to reload which I don't think is used by any primary target. On the other hand it trips building glibc, which makes it more important in mind. On the whole I don't think it's P1 worthy though. Still on my list though.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #7 from Jakub Jelinek --- Any progress on this? Should it be P1 when it only affects non-primary/secondary target?
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 Jeffrey A. Law changed: What|Removed |Added Resolution|FIXED |--- Status|RESOLVED|REOPENED --- Comment #6 from Jeffrey A. Law --- Patch reverted, reopening.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 --- Comment #5 from Sam James --- The hook didn't fire for some reason but r15-2894-ge9738e77674e23 was the fix.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 Jeffrey A. Law changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #4 from Jeffrey A. Law --- Fixed on the trunk.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 --- Comment #3 from Jeffrey A. Law --- Adjusting subreg_regno isn't going to work. reload depends on it producing "bogus" results to then trigger reloading inner parts of the subreg expressions. Adjusting alter_reg might be an option.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244
Jeffrey A. Law changed:
What|Removed |Added
Status|NEW |ASSIGNED
--- Comment #2 from Jeffrey A. Law ---
Looks like another path using subreg_regno_offset without verifying it's
actually a subreg expression that can be simplified. Like the prior m68k bug
we have
(subreg:DI (reg:SI d0) 0)
So a paradoxical reference to register 0. This time it's reload calling
alter_subreg. alter_subreg calls subreg_regno which looks like:
/* Return the final regno that a subreg expression refers to. */
unsigned int
subreg_regno (const_rtx x)
{
unsigned int ret;
rtx subreg = SUBREG_REG (x);
int regno = REGNO (subreg);
ret = regno + subreg_regno_offset (regno,
GET_MODE (subreg),
SUBREG_BYTE (x),
GET_MODE (x));
return ret;
}
Which is going to return -1 for the case above. So we install -1 as the
register number and all hell breaks loose after that.
I think the sensible thing to do with a subreg we're not going to simplify is
to just return the original register #. But I need to put that through the
testsuite on a few targets to ensure no fallout. subreg_regno isn't a heavily
used API thankfully.
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244 Richard Biener changed: What|Removed |Added Target Milestone|--- |15.0
[Bug rtl-optimization/116244] [15 Regression] reload ICE building libstdc++ for coldfire
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116244
Jeffrey A. Law changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
Ever confirmed|0 |1
Last reconfirmed||2024-08-06
--- Comment #1 from Jeffrey A. Law ---
A lot of similarities to the most recent bug on the m68k.
We had this prior to ext-dce (from a somewhat reduced testcase, so insn #s
won't match):
(insn 878 877 881 19 (set (reg:DI 35 [ diff$__r ])
(zero_extend:DI (reg:SI 49 [ _32 ])))
"/notnfs/josmyers/glibc-manual/build/compilers/m68k-linux-gnu-coldfire/gcc/m68k-glibc-linux-gnu/libstdc++-v3/include/bits/chrono.h":574:10
85 {*zero_extendsidi2}
(expr_list:REG_DEAD (reg:SI 49 [ _32 ])
(nil)))
We know the upper bits aren't needed, so we turn the zero_extend into a subreg:
(insn 878 877 881 19 (set (reg:DI 35 [ diff$__r ])
(subreg:DI (reg:SI 49 [ _32 ]) 0))
"/notnfs/josmyers/glibc-manual/build/compilers/m68k-linux-gnu-coldfire/gcc/m68k-glibc-linux-gnu/libstdc++-v3/include/bits/chrono.h":574:10
76 {*m68k.md:1608}
(expr_list:REG_DEAD (reg:SI 49 [ _32 ])
(nil)))
So far, so good. We assign (reg:SI 49) to d0 resulting in this in the middle
of reload:
(insn 878 877 1042 19 (set (reg:DI 0 %d0 [orig:35 diff$__r ] [35])
(subreg:DI (reg:SI 0 %d0) 0))
"/notnfs/josmyers/glibc-manual/build/compilers/m68k-linux-gnu-coldfire/gcc/m68k-glibc-linux-gnu/libstdc++-v3/include/bits/chrono.h":574:10
76 {*m68k.md:1608}
(expr_list:REG_DEAD (reg:SI 0 %d0 [orig:49 _32 ] [49])
(nil)))
We eventually try to simplify the subreg resulting in:
(insn 878 877 1042 19 (set (reg:DI 0 %d0 [orig:35 diff$__r ] [35])
(reg:DI -1 [+-4 ]))
"/notnfs/josmyers/glibc-manual/build/compilers/m68k-linux-gnu-coldfire/gcc/m68k-glibc-linux-gnu/libstdc++-v3/include/bits/chrono.h":574:10
76 {*m68k.md:1608}
(nil))
Which is clearly very bad.
