Patchwork [AVR] : Fix PR45099

login
register
mail settings
Submitter Georg-Johann Lay
Date May 2, 2011, 3:23 p.m.
Message ID <4DBECC84.4080701@gjlay.de>
Download mbox | patch
Permalink /patch/93680/
State New
Headers show

Comments

Georg-Johann Lay - May 2, 2011, 3:23 p.m.
PR45099 is an extension that gives an error when a fixed register is
needed to pass a parameter to a function.

Because the program will show malfunction when such code is generated,
anyway, I think an error is more appropriate than a warning (as
proposed in the PR).

Johann

2011-05-02  Georg-Johann Lay  <avr@gjlay.de>

	PR target/45099

	* config/avr/avr.c (avr_function_arg_advance): Error if a fixed
	register is needed for a function argument.
Nathan Froyd - May 2, 2011, 4:18 p.m.
On Mon, May 02, 2011 at 05:23:48PM +0200, Georg-Johann Lay wrote:
> PR45099 is an extension that gives an error when a fixed register is
> needed to pass a parameter to a function.
> 
> Because the program will show malfunction when such code is generated,
> anyway, I think an error is more appropriate than a warning (as
> proposed in the PR).

This seems like something that should be handled by common code.

-Nathan
Georg-Johann Lay - May 6, 2011, 1:54 p.m.
Nathan Froyd schrieb:
> On Mon, May 02, 2011 at 05:23:48PM +0200, Georg-Johann Lay wrote:
>> PR45099 is an extension that gives an error when a fixed register is
>> needed to pass a parameter to a function.
>>
>> Because the program will show malfunction when such code is generated,
>> anyway, I think an error is more appropriate than a warning (as
>> proposed in the PR).
> 
> This seems like something that should be handled by common code.
> 
> -Nathan

Yes, I agree. However, common code it too complicated for me to run
tests for, so I restrict myself to avr backend.

Until such a test will find its way into common code, it might still
be useful in avr backend. I think this has quite low priority for
other targets because global registers are not very common in, e.g. i386.

Johann
Denis Chertykov - May 10, 2011, 11:48 a.m.
2011/5/6 Georg-Johann Lay <avr@gjlay.de>:
> Nathan Froyd schrieb:
>> On Mon, May 02, 2011 at 05:23:48PM +0200, Georg-Johann Lay wrote:
>>> PR45099 is an extension that gives an error when a fixed register is
>>> needed to pass a parameter to a function.
>>>
>>> Because the program will show malfunction when such code is generated,
>>> anyway, I think an error is more appropriate than a warning (as
>>> proposed in the PR).
>>
>> This seems like something that should be handled by common code.
>>
>> -Nathan
>
> Yes, I agree. However, common code it too complicated for me to run
> tests for, so I restrict myself to avr backend.
>
> Until such a test will find its way into common code, it might still
> be useful in avr backend. I think this has quite low priority for
> other targets because global registers are not very common in, e.g. i386.
>

I think that better to have this patch.
So, I approve it.
George, please make a working testcase against the trunk and post a
GCC core bug.

Denis.
Georg-Johann Lay - May 16, 2011, 2:28 p.m.
Denis Chertykov schrieb:
> 2011/5/6 Georg-Johann Lay <avr@gjlay.de>:
>> Nathan Froyd schrieb:
>>> On Mon, May 02, 2011 at 05:23:48PM +0200, Georg-Johann Lay wrote:
>>>> PR45099 is an extension that gives an error when a fixed register is
>>>> needed to pass a parameter to a function.
>>>>
>>>> Because the program will show malfunction when such code is generated,
>>>> anyway, I think an error is more appropriate than a warning (as
>>>> proposed in the PR).
>>> This seems like something that should be handled by common code.
>>>
>>> -Nathan
>> Yes, I agree. However, common code it too complicated for me to run
>> tests for, so I restrict myself to avr backend.
>>
>> Until such a test will find its way into common code, it might still
>> be useful in avr backend. I think this has quite low priority for
>> other targets because global registers are not very common in, e.g. i386.
>>
> 
> I think that better to have this patch.
> So, I approve it.

Ok, applied it as it was:
http://gcc.gnu.org/viewcvs?view=revision&revision=173791

> George, please make a working testcase against the trunk and post a
> GCC core bug.

How should such a testcase look like, and for what target? This is
highly target and ABI dependent. Moreover, the patch above just tests
for argument registers, not return registers.

Basically, I think of someone is hacking with global registers, he
must know what he is doing. There will always be cases where global
regs might violate some things if one is not careful.

Johann

> 
> Denis.
>
Denis Chertykov - May 16, 2011, 3:32 p.m.
2011/5/16 Georg-Johann Lay <avr@gjlay.de>:
> Denis Chertykov schrieb:
>> 2011/5/6 Georg-Johann Lay <avr@gjlay.de>:
>>> Nathan Froyd schrieb:
>>>> On Mon, May 02, 2011 at 05:23:48PM +0200, Georg-Johann Lay wrote:
>>>>> PR45099 is an extension that gives an error when a fixed register is
>>>>> needed to pass a parameter to a function.
>>>>>
>>>>> Because the program will show malfunction when such code is generated,
>>>>> anyway, I think an error is more appropriate than a warning (as
>>>>> proposed in the PR).
>>>> This seems like something that should be handled by common code.
>>>>
>>>> -Nathan
>>> Yes, I agree. However, common code it too complicated for me to run
>>> tests for, so I restrict myself to avr backend.
>>>
>>> Until such a test will find its way into common code, it might still
>>> be useful in avr backend. I think this has quite low priority for
>>> other targets because global registers are not very common in, e.g. i386.
>>>
>>
>> I think that better to have this patch.
>> So, I approve it.
>
> Ok, applied it as it was:
> http://gcc.gnu.org/viewcvs?view=revision&revision=173791
>
>> George, please make a working testcase against the trunk and post a
>> GCC core bug.
>
> How should such a testcase look like, and for what target? This is
> highly target and ABI dependent. Moreover, the patch above just tests
> for argument registers, not return registers.
>
> Basically, I think of someone is hacking with global registers, he
> must know what he is doing. There will always be cases where global
> regs might violate some things if one is not careful.

Ok.

Denis.

Patch

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(Revision 172902)
+++ config/avr/avr.c	(Arbeitskopie)
@@ -1794,6 +1794,20 @@  avr_function_arg_advance (CUMULATIVE_ARG
       cfun->machine->sibcall_fails = 1;
     }
 
+  /* Test if all registers needed by the ABI are actually available.  If the
+     user has fixed a GPR needed to pass an argument, an (implicit) function
+     call would clobber that fixed register.  See PR45099 for an example.  */
+  
+  if (cum->regno >= 0)
+    {
+      int regno;
+
+      for (regno = cum->regno; regno < cum->regno + bytes; regno++)
+        if (fixed_regs[regno])
+          error ("Register %s is needed to pass a parameter but is fixed",
+                 reg_names[regno]);
+    }
+      
   if (cum->nregs <= 0)
     {
       cum->nregs = 0;