Patchwork [avr] : Remove -mshort-calls option

login
register
mail settings
Submitter Georg-Johann Lay
Date Oct. 5, 2012, 3:54 p.m.
Message ID <506F02BB.4050109@gjlay.de>
Download mbox | patch
Permalink /patch/189517/
State New
Headers show

Comments

Georg-Johann Lay - Oct. 5, 2012, 3:54 p.m.
As already discussed, this patch removes the -mshort-calls command option from
avr-gcc.

Ok to apply?

If the change is on order, changes to wwwdocs will follow, i.e. deprecate the
option in 4.7 and tell it is removed in the 4.8 caveats.

Johann


	* doc/invoke.texi (AVR Options): Remove -mshort-calls.
	* config/avr/avr.opt (-mshort-calls): Remove option.
	* config/avr/avr.h (AVR_HAVE_JMP_CALL): Don't depend on
	TARGET_SHORT_CALLS.
Denis Chertykov - Oct. 6, 2012, 11:09 a.m.
2012/10/5 Georg-Johann Lay <avr@gjlay.de>:
> As already discussed, this patch removes the -mshort-calls command option from
> avr-gcc.
>
> Ok to apply?
>
> If the change is on order, changes to wwwdocs will follow, i.e. deprecate the
> option in 4.7 and tell it is removed in the 4.8 caveats.
>
> Johann
>
>
>         * doc/invoke.texi (AVR Options): Remove -mshort-calls.
>         * config/avr/avr.opt (-mshort-calls): Remove option.
>         * config/avr/avr.h (AVR_HAVE_JMP_CALL): Don't depend on
>         TARGET_SHORT_CALLS.
>

Ok. Approved.

Denis.
Joerg Wunsch - Oct. 7, 2012, 7:19 a.m.
As Georg-Johann Lay wrote:

> As already discussed, this patch removes the -mshort-calls command
> option from avr-gcc.

> Ok to apply?

I'm more than happy with removing it.
Weddington, Eric - Oct. 7, 2012, 6:01 p.m.
> -----Original Message-----

> From: Georg-Johann Lay []

> Sent: Friday, October 05, 2012 9:55 AM

> To: gcc-patches@gcc.gnu.org

> Cc: Denis Chertykov; Weddington, Eric; Joerg Wunsch

> Subject: [Patch,avr]: Remove -mshort-calls option

> 

> As already discussed, this patch removes the -mshort-calls command option

> from

> avr-gcc.

> 

> Ok to apply?


Ok to apply, but...

> 

> If the change is on order, changes to wwwdocs will follow, i.e. deprecate

> the

> option in 4.7 and tell it is removed in the 4.8 caveats.

>


... but where do we notify the user that the switch is deprecated?
Oleg Endo - Oct. 7, 2012, 6:28 p.m.
On Sun, 2012-10-07 at 18:01 +0000, Weddington, Eric wrote:
> 
> > -----Original Message-----
> > From: Georg-Johann Lay []
> > Sent: Friday, October 05, 2012 9:55 AM
> > To: gcc-patches@gcc.gnu.org
> > Cc: Denis Chertykov; Weddington, Eric; Joerg Wunsch
> > Subject: [Patch,avr]: Remove -mshort-calls option
> > 
> > As already discussed, this patch removes the -mshort-calls command option
> > from
> > avr-gcc.
> > 
> > Ok to apply?
> 
> Ok to apply, but...
> 
> > 
> > If the change is on order, changes to wwwdocs will follow, i.e. deprecate
> > the
> > option in 4.7 and tell it is removed in the 4.8 caveats.
> >
> 
> ... but where do we notify the user that the switch is deprecated?
> 

Maybe would be better to first make the option a no-op that prints a
warning in GCC, remove it from all documentations and mention the
deprecation in the wwwdocs changes.  Then, in the next GCC release,
remove it completely.

Cheers,
Oleg
Georg-Johann Lay - Oct. 7, 2012, 7:37 p.m.
Oleg Endo wrote:
> Weddington, Eric wrote:
>>Georg-Johann Lay wrote:
>>>
>>> As already discussed, this patch removes the -mshort-calls command option
>>> from avr-gcc.
>>>
>>> Ok to apply?
>> Ok to apply, but...
>>
>>> If the change is on order, changes to wwwdocs will follow, i.e. deprecate
>>> the option in 4.7 and tell it is removed in the 4.8 caveats.
>>>
>> ... but where do we notify the user that the switch is deprecated?

Exactly there: In the wwwdocs, namely the caveats section of the release 
notes.  The release notes are the blackboard for anyone who wants to get 
informed about what changed in the new release series.

> Maybe would be better to first make the option a no-op that prints a
> warning in GCC, remove it from all documentations and mention the
> deprecation in the wwwdocs changes.  Then, in the next GCC release,
> remove it completely.

IMHO it's not reasonable to put effort into a declining feature.  Why 
shouldn't we follow common GCC practice here?

Johann
Oleg Endo - Oct. 7, 2012, 8:02 p.m.
On Sun, 2012-10-07 at 21:37 +0200, Georg-Johann Lay wrote:
> Oleg Endo wrote:
> > Maybe would be better to first make the option a no-op that prints a
> > warning in GCC, remove it from all documentations and mention the
> > deprecation in the wwwdocs changes.  Then, in the next GCC release,
> > remove it completely.
> 
> IMHO it's not reasonable to put effort into a declining feature.  Why 
> shouldn't we follow common GCC practice here?

"Effort" in this case is adding a "Warn(...)" to the existing option and
then remove it some time later.  I think it's more user friendly to
first warn and then do.  But that's just my opinion, feel free to
ignore :)

Cheers,
Oleg
Joerg Wunsch - Oct. 7, 2012, 8:07 p.m.
As Oleg Endo wrote:

> I think it's more user friendly to
> first warn and then do.

The problem is that this option would better not have existed straight
from the beginning.  When using it, the compiler is at the risk of
generating code that fails to link later, because the relative calls
used could not reach the entire address space of the respective
processor.
Georg-Johann Lay - Oct. 7, 2012, 8:12 p.m.
Joerg Wunsch wrote:
> As Oleg Endo wrote:
> 
>> I think it's more user friendly to
>> first warn and then do.
> 
> The problem is that this option would better not have existed straight
> from the beginning.  When using it, the compiler is at the risk of
> generating code that fails to link later, because the relative calls
> used could not reach the entire address space of the respective
> processor.

Also notice that this is only about the -mshort-calls option of the avr 
port, not about similar named options that exist for some other ports.

Johann

Patch

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 192131)
+++ doc/invoke.texi	(working copy)
@@ -505,7 +505,7 @@  Objective-C and Objective-C++ Dialects}.
 
 @emph{AVR Options}
 @gccoptlist{-mmcu=@var{mcu} -maccumulate-args -mbranch-cost=@var{cost} @gol
--mcall-prologues -mint8 -mno-interrupts -mrelax -mshort-calls @gol
+-mcall-prologues -mint8 -mno-interrupts -mrelax @gol
 -mstrict-X -mtiny-stack}
 
 @emph{Blackfin Options}
@@ -11291,13 +11291,6 @@  differ from instructions in the assemble
 Relaxing must be turned on if linker stubs are needed, see the
 section on @code{EIND} and linker stubs below.
 
-@item -mshort-calls
-@opindex mshort-calls
-Use @code{RCALL}/@code{RJMP} instructions even on devices with
-16@tie{}KiB or more of program memory, i.e.@: on devices that
-have the @code{CALL} and @code{JMP} instructions.
-See also the @code{-mrelax} command line option.
-
 @item -msp8
 @opindex msp8
 Treat the stack pointer register as an 8-bit register,
@@ -11560,7 +11553,7 @@  The device has a hardware multiplier.
 @item __AVR_HAVE_JMP_CALL__
 The device has the @code{JMP} and @code{CALL} instructions.
 This is the case for devices with at least 16@tie{}KiB of program
-memory and if @code{-mshort-calls} is not set.
+memory.
 
 @item __AVR_HAVE_EIJMP_EICALL__
 @item __AVR_3_BYTE_PC__
Index: config/avr/avr.opt
===================================================================
--- config/avr/avr.opt	(revision 192131)
+++ config/avr/avr.opt	(working copy)
@@ -50,10 +50,6 @@  Target Report Undocumented Mask(ORDER_1)
 morder2
 Target Report Undocumented Mask(ORDER_2)
 
-mshort-calls
-Target Report Mask(SHORT_CALLS)
-Use rjmp/rcall (limited range) on >8K devices
-
 mtiny-stack
 Target Report Mask(TINY_STACK)
 Change only the low 8 bits of the stack pointer
Index: config/avr/avr.h
===================================================================
--- config/avr/avr.h	(revision 192131)
+++ config/avr/avr.h	(working copy)
@@ -57,7 +57,7 @@  enum
 
 #define TARGET_CPU_CPP_BUILTINS()	avr_cpu_cpp_builtins (pfile)
 
-#define AVR_HAVE_JMP_CALL (avr_current_arch->have_jmp_call && !TARGET_SHORT_CALLS)
+#define AVR_HAVE_JMP_CALL (avr_current_arch->have_jmp_call)
 #define AVR_HAVE_MUL (avr_current_arch->have_mul)
 #define AVR_HAVE_MOVW (avr_current_arch->have_movw_lpmx)
 #define AVR_HAVE_LPMX (avr_current_arch->have_movw_lpmx)