diff mbox

[AVR,committed] : ad PR45099: change error to warning

Message ID 4E79B33E.4000805@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Sept. 21, 2011, 9:49 a.m. UTC
As proposed in PR45099, avr-gcc will now just print a warning instead of an
error when a fixed register is needed to pass a parameter to a function.

That way the user can inspect the source, and there are situation imaginable
where clobbering a fixed register is not a problem.

Committed as trunk r179040:

http://gcc.gnu.org/viewcvs?view=revision&revision=179040

Johann

	PR target/45099
	* config/avr/avr.c (avr_function_arg_advance): Change error to
	warning if a fixed register is needed as function argument.

Comments

Nathan Froyd Sept. 21, 2011, 1:11 p.m. UTC | #1
On 9/21/2011 5:49 AM, Georg-Johann Lay wrote:
> As proposed in PR45099, avr-gcc will now just print a warning instead of an
> error when a fixed register is needed to pass a parameter to a function.

Where's the proposal in the PR?  I see a problem report that was 
addressed four months ago by adding an error, the addition of an error 
message, an explanation of why we're issuing an error message, and then 
this commit, which changes the error back into a warning (!).

I guess the original report proposed a warning, but an error seems like 
a *much* better idea.  Better to tell the user early that things are 
going to break, IMHO.

-Nathan
Georg-Johann Lay Sept. 21, 2011, 5:23 p.m. UTC | #2
Nathan Froyd wrote:
> On 9/21/2011 5:49 AM, Georg-Johann Lay wrote:
>> As proposed in PR45099, avr-gcc will now just print a warning instead
>> of an
>> error when a fixed register is needed to pass a parameter to a function.
> 
> Where's the proposal in the PR?  I see a problem report that was
> addressed four months ago by adding an error, the addition of an error
> message, an explanation of why we're issuing an error message, and then
> this commit, which changes the error back into a warning (!).

The initial state up to 4.6 was that there was no diagnosis at allwhen
a fixed register overlapped a parameter.
My patch in #c1 (trunk r173791) used error as diagnosis.

> I guess the original report proposed a warning, 

Yes, the OP proposes a warning in #c0 of the PR.

> but an error seems like a *much* better idea.
> Better to tell the user early that things are
> going to break, IMHO.

Initially, I was of the same opinion.  Now I think a warning
is more appropriately because the compiler should not try to be
smarter than it actually is/can be.

One example: the user uses a function to initialize his application
before setting up the global registers.  There are other scenarios
imaginable.

So I think a warning is better than an error because the user can
inspect the code and take the decision if he is fine with it or not
and is not patronized by the compiler.

For example, if the user gets a hint that fixed reg collides with
a function call, he can save/restore the global register around
the call.

It's not possible to tell what the user strives to do
with the fixed register; the important thing is that there is
some message that there *might* be a problem.

We cannot conclude more.

Johann

> -Nathan
diff mbox

Patch

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c    (revision 179038)
+++ config/avr/avr.c    (working copy)
@@ -1810,7 +1810,7 @@  avr_function_arg_advance (cumulative_arg

   /* 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.  */
+     call will clobber that fixed register.  See PR45099 for an example.  */

   if (cum->regno >= 8
       && cum->nregs >= 0)
@@ -1819,8 +1819,8 @@  avr_function_arg_advance (cumulative_arg

       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]);
+          warning (0, "fixed register %s used to pass parameter to function",
+                   reg_names[regno]);
     }

   if (cum->nregs <= 0)