| Message ID | 5102D1CD.4020509@gjlay.de |
|---|---|
| State | New |
| Headers | show |
On 01/25/2013 11:41 AM, Georg-Johann Lay wrote: > PR54814 causes spill fails because reload.c:find_valid_class_1 tests only one > hard register instead of all hard registers of regno:mode in rclass: > > http://gcc.gnu.org/PR54814 > > > The patch was originally worked out by Bernd Schmidt and fixed a problem > introduced in > > http://gcc.gnu.org/r190252 > > The patch is bootstrapped and tested on x86_64-linux and also fixes the spill > fails that originally occured on avr-unknown-one. > > Ok to apply? > > > PR other/54814 > * reload.c (find_valid_class_1): Use in_hard_reg_set_p instead of > TEST_HARD_REG_BIT. Is this a regression relative to a prior version of GCC? If not, it'll probably need release manager approval before it can go in. Please attach your patch to PR54814 and attach PR 54814 to the 4.9 pending patches meta bug. jeff
Jeff Law schrieb: > On 01/25/2013 11:41 AM, Georg-Johann Lay wrote: >> PR54814 causes spill fails because reload.c:find_valid_class_1 tests >> only one >> hard register instead of all hard registers of regno:mode in rclass: >> >> http://gcc.gnu.org/PR54814 >> >> >> The patch was originally worked out by Bernd Schmidt and fixed a problem >> introduced in >> >> http://gcc.gnu.org/r190252 >> >> The patch is bootstrapped and tested on x86_64-linux and also fixes >> the spill >> fails that originally occurred on avr-unknown-one. >> >> Ok to apply? >> >> >> PR other/54814 >> * reload.c (find_valid_class_1): Use in_hard_reg_set_p instead of >> TEST_HARD_REG_BIT. > Is this a regression relative to a prior version of GCC? Yes, it's 4.8 regression. 4.7 works and r190252 was only applied to 4.8 trunk. > If not, it'll probably need release manager approval before it can go in. > > Please attach your patch to PR54814 and attach PR 54814 to the 4.9 > pending patches meta bug. The patch *is* already attached, I can attach it again if that helps. Johann
Georg-Johann Lay schrieb: > Jeff Law schrieb: >> On 01/25/2013 11:41 AM, Georg-Johann Lay wrote: >>> PR54814 causes spill fails because reload.c:find_valid_class_1 tests >>> only one >>> hard register instead of all hard registers of regno:mode in rclass: >>> >>> http://gcc.gnu.org/PR54814 >>> >>> >>> The patch was originally worked out by Bernd Schmidt and fixed a problem >>> introduced in >>> >>> http://gcc.gnu.org/r190252 >>> >>> The patch is bootstrapped and tested on x86_64-linux and also fixes >>> the spill >>> fails that originally occurred on avr-unknown-one. >>> >>> Ok to apply? >>> >>> >>> PR other/54814 >>> * reload.c (find_valid_class_1): Use in_hard_reg_set_p instead of >>> TEST_HARD_REG_BIT. >> Is this a regression relative to a prior version of GCC? > > Yes, it's 4.8 regression. 4.7 works and r190252 was only applied to 4.8 > trunk. > >> If not, it'll probably need release manager approval before it can go in. >> >> Please attach your patch to PR54814 and attach PR 54814 to the 4.9 >> pending patches meta bug. Does this mean the fix is rejected for 4.8? I found no "4.9 meta-bug" in the 47 meta-bugs. You have th PR? http://gcc.gnu.org/bugzilla/buglist.cgi?keywords=meta-bug%2C%20&keywords_type=allwords&list_id=51998&cf_known_to_fail_type=allwords&cf_known_to_work_type=allwords&query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&product=gcc FYI, this bug breaks the avr port almost completely. Many real-world programs ICE.
On Sun, 27 Jan 2013, Georg-Johann Lay wrote:
> I found no "4.9 meta-bug" in the 47 meta-bugs. You have th PR?
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55996
On Sun, Jan 27, 2013 at 11:09 PM, Georg-Johann Lay wrote: >>>> The patch was originally worked out by Bernd Schmidt and fixed a problem >>>> introduced in >>>> >>>> http://gcc.gnu.org/r190252 Ironically, this revision fixes a reload problem on x86/x86_64 -- which doesn't use reload anymore now... > Does this mean the fix is rejected for 4.8? No, just that it probably helps to add a RM to the CC list. FWIW, it seems to me that this patch should go into 4.8, because the bug is probably not limited to AVR. Ciao! Steven
On Sun, Jan 27, 2013 at 11:26 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote: > On Sun, Jan 27, 2013 at 11:09 PM, Georg-Johann Lay wrote: >>>>> The patch was originally worked out by Bernd Schmidt and fixed a problem >>>>> introduced in >>>>> >>>>> http://gcc.gnu.org/r190252 > > Ironically, this revision fixes a reload problem on x86/x86_64 -- > which doesn't use reload anymore now... > > >> Does this mean the fix is rejected for 4.8? > > No, just that it probably helps to add a RM to the CC list. > > FWIW, it seems to me that this patch should go into 4.8, because the > bug is probably not limited to AVR. Indeed, the fix also looks quite obvious though I know nothing about the code at all. Thus, ok from a RM perspective if a reload-affine person approves it. Thanks, Richard. > Ciao! > Steven
On 01/27/2013 03:26 PM, Steven Bosscher wrote: > On Sun, Jan 27, 2013 at 11:09 PM, Georg-Johann Lay wrote: >>>>> The patch was originally worked out by Bernd Schmidt and fixed a problem >>>>> introduced in >>>>> >>>>> http://gcc.gnu.org/r190252 > > Ironically, this revision fixes a reload problem on x86/x86_64 -- > which doesn't use reload anymore now... > > >> Does this mean the fix is rejected for 4.8? > > No, just that it probably helps to add a RM to the CC list. > > FWIW, it seems to me that this patch should go into 4.8, because the > bug is probably not limited to AVR. At this stage, I tend to be more conservative. However, it looks like Ulrich & Richi have taken a looksie and think the patch is fine. I'm certainly not going to object. jeff
On 01/27/2013 03:09 PM, Georg-Johann Lay wrote: >> >>> If not, it'll probably need release manager approval before it can go >>> in. >>> >>> Please attach your patch to PR54814 and attach PR 54814 to the 4.9 >>> pending patches meta bug. > > Does this mean the fix is rejected for 4.8? Not necessarily. We're in a regression bugfix only stage; so regressions can obviously be fixed. If a change does not fix a regression, then it really needs the release manager's approval to go forward at this stage. Jeff
Index: gcc/reload.c =================================================================== --- gcc/reload.c (revision 194553) +++ gcc/reload.c (working copy) @@ -709,7 +709,7 @@ find_valid_class (enum machine_mode oute } /* We are trying to reload a subreg of something that is not a register. - Find the largest class which has at least one register valid in + Find the largest class which contains only registers valid in mode MODE. OUTER is the mode of the subreg, DEST_CLASS the class in which we would eventually like to obtain the object. */ @@ -729,10 +729,12 @@ find_valid_class_1 (enum machine_mode ou { int bad = 0; for (regno = 0; regno < FIRST_PSEUDO_REGISTER && !bad; regno++) - if (TEST_HARD_REG_BIT (reg_class_contents[rclass], regno) - && !HARD_REGNO_MODE_OK (regno, mode)) - bad = 1; - + { + if (in_hard_reg_set_p (reg_class_contents[rclass], mode, regno) + && !HARD_REGNO_MODE_OK (regno, mode)) + bad = 1; + } + if (bad) continue;