public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch] Fix PR54814
@ 2013-01-25 18:41 Georg-Johann Lay
  2013-01-25 19:51 ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Georg-Johann Lay @ 2013-01-25 18:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: Bernd Schmidt, Ulrich Weigand

[-- Attachment #1: Type: text/plain, Size: 540 bytes --]

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.

[-- Attachment #2: pr54814.diff --]
[-- Type: text/x-patch, Size: 1020 bytes --]

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;
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-25 18:41 [Patch] Fix PR54814 Georg-Johann Lay
@ 2013-01-25 19:51 ` Jeff Law
  2013-01-25 20:51   ` Georg-Johann Lay
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2013-01-25 19:51 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: gcc-patches, Bernd Schmidt, Ulrich Weigand

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-25 19:51 ` Jeff Law
@ 2013-01-25 20:51   ` Georg-Johann Lay
  2013-01-27 22:11     ` Georg-Johann Lay
  0 siblings, 1 reply; 11+ messages in thread
From: Georg-Johann Lay @ 2013-01-25 20:51 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches, Bernd Schmidt, Ulrich Weigand

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-25 20:51   ` Georg-Johann Lay
@ 2013-01-27 22:11     ` Georg-Johann Lay
  2013-01-27 22:15       ` Marc Glisse
                         ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Georg-Johann Lay @ 2013-01-27 22:11 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches, Bernd Schmidt, Ulrich Weigand

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-27 22:11     ` Georg-Johann Lay
@ 2013-01-27 22:15       ` Marc Glisse
  2013-01-27 22:27       ` Steven Bosscher
  2013-01-28 17:38       ` Jeff Law
  2 siblings, 0 replies; 11+ messages in thread
From: Marc Glisse @ 2013-01-27 22:15 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: Jeff Law, gcc-patches, Bernd Schmidt, Ulrich Weigand

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

-- 
Marc Glisse

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-27 22:11     ` Georg-Johann Lay
  2013-01-27 22:15       ` Marc Glisse
@ 2013-01-27 22:27       ` Steven Bosscher
  2013-01-28 10:25         ` Richard Biener
  2013-01-28 17:36         ` Jeff Law
  2013-01-28 17:38       ` Jeff Law
  2 siblings, 2 replies; 11+ messages in thread
From: Steven Bosscher @ 2013-01-27 22:27 UTC (permalink / raw)
  To: Georg-Johann Lay
  Cc: Jeff Law, gcc-patches, Bernd Schmidt, Ulrich Weigand, Jakub Jelinek

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-27 22:27       ` Steven Bosscher
@ 2013-01-28 10:25         ` Richard Biener
  2013-01-28 13:55           ` Ulrich Weigand
  2013-01-28 17:36         ` Jeff Law
  1 sibling, 1 reply; 11+ messages in thread
From: Richard Biener @ 2013-01-28 10:25 UTC (permalink / raw)
  To: Steven Bosscher
  Cc: Georg-Johann Lay, Jeff Law, gcc-patches, Bernd Schmidt,
	Ulrich Weigand, Jakub Jelinek

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-28 10:25         ` Richard Biener
@ 2013-01-28 13:55           ` Ulrich Weigand
  2013-01-28 17:45             ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Ulrich Weigand @ 2013-01-28 13:55 UTC (permalink / raw)
  To: Richard Biener
  Cc: Steven Bosscher, Georg-Johann Lay, Jeff Law, gcc-patches,
	Bernd Schmidt, Jakub Jelinek

Richard Biener wrote:
> 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.

The patch was originally by Bernd, but FWIW it looks good to me as well.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-27 22:27       ` Steven Bosscher
  2013-01-28 10:25         ` Richard Biener
@ 2013-01-28 17:36         ` Jeff Law
  1 sibling, 0 replies; 11+ messages in thread
From: Jeff Law @ 2013-01-28 17:36 UTC (permalink / raw)
  To: Steven Bosscher
  Cc: Georg-Johann Lay, gcc-patches, Bernd Schmidt, Ulrich Weigand,
	Jakub Jelinek

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-27 22:11     ` Georg-Johann Lay
  2013-01-27 22:15       ` Marc Glisse
  2013-01-27 22:27       ` Steven Bosscher
@ 2013-01-28 17:38       ` Jeff Law
  2 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2013-01-28 17:38 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: gcc-patches, Bernd Schmidt, Ulrich Weigand

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch] Fix PR54814
  2013-01-28 13:55           ` Ulrich Weigand
@ 2013-01-28 17:45             ` Jeff Law
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2013-01-28 17:45 UTC (permalink / raw)
  To: Ulrich Weigand
  Cc: Richard Biener, Steven Bosscher, Georg-Johann Lay, gcc-patches,
	Bernd Schmidt, Jakub Jelinek

On 01/28/2013 06:55 AM, Ulrich Weigand wrote:
> Richard Biener wrote:
>> 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.
>
> The patch was originally by Bernd, but FWIW it looks good to me as well.
Now that I know this is a regression, I've looked at it more closely and 
it looks good to me too.

George-Johann, please install this onto the trunk.  Thanks,

Jeff

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-01-28 17:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-25 18:41 [Patch] Fix PR54814 Georg-Johann Lay
2013-01-25 19:51 ` Jeff Law
2013-01-25 20:51   ` Georg-Johann Lay
2013-01-27 22:11     ` Georg-Johann Lay
2013-01-27 22:15       ` Marc Glisse
2013-01-27 22:27       ` Steven Bosscher
2013-01-28 10:25         ` Richard Biener
2013-01-28 13:55           ` Ulrich Weigand
2013-01-28 17:45             ` Jeff Law
2013-01-28 17:36         ` Jeff Law
2013-01-28 17:38       ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).