| Message ID | 4E9861F8.7020903@gjlay.de |
|---|---|
| State | New |
| Headers | show |
On 10/14/2011 06:23 PM, Georg-Johann Lay wrote: > +@item -mjump-to-noreturn > +@opindex mjump-to-noreturn > +Use a jump instruction instead of a call instruction when calling a > +no-return functions. This option is active if optimization is turned > +on and just affects the way a call instruction is printed out. > +Besides that, it has no effect on code generation or debug information. I think this is not really accurate given Richard's input. Paolo
Paolo Bonzini schrieb: > On 10/14/2011 06:23 PM, Georg-Johann Lay wrote: >> +@item -mjump-to-noreturn >> +@opindex mjump-to-noreturn >> +Use a jump instruction instead of a call instruction when calling a >> +no-return functions. This option is active if optimization is turned >> +on and just affects the way a call instruction is printed out. >> +Besides that, it has no effect on code generation or debug information. > > I think this is not really accurate given Richard's input. > > Paolo > Confused. The conclusion was to introduce a new command line option in order to have individual control over this feature. The option is named -mjump-to-noreturn now instead of -mjump-noreturn. Is that what you mean? Johann
On Fri, Oct 14, 2011 at 19:19, Georg-Johann Lay <avr@gjlay.de> wrote: > Paolo Bonzini schrieb: >> On 10/14/2011 06:23 PM, Georg-Johann Lay wrote: >>> +@item -mjump-to-noreturn >>> +@opindex mjump-to-noreturn >>> +Use a jump instruction instead of a call instruction when calling a >>> +no-return functions. This option is active if optimization is turned >>> +on and just affects the way a call instruction is printed out. >>> +Besides that, it has no effect on code generation or debug information. >> >> I think this is not really accurate given Richard's input. > > Confused. The conclusion was to introduce a new command line option in order > to have individual control over this feature. The option is named > -mjump-to-noreturn now instead of -mjump-noreturn. Is that what you mean? No, I mean that it has an effect on debuggability. You will not be able to derive the place where you crashed from the backtrace. Paolo
On Friday 14 October 2011 18:19:00, Georg-Johann Lay wrote: > Paolo Bonzini schrieb: > > On 10/14/2011 06:23 PM, Georg-Johann Lay wrote: > >> +@item -mjump-to-noreturn > >> +@opindex mjump-to-noreturn > >> +Use a jump instruction instead of a call instruction when calling a > >> +no-return functions. This option is active if optimization is turned > >> +on and just affects the way a call instruction is printed out. > >> +Besides that, it has no effect on code generation or debug information. > > > > I think this is not really accurate given Richard's input. > > Confused. The conclusion was to introduce a new command line option in order > to have individual control over this feature. The option is named > -mjump-to-noreturn now instead of -mjump-noreturn. Is that what you mean? My 2c. You've used implementor-speak to describe the option, while you should use user-speak. Mention that it saves stack, but the downside is that it affects backtracing (and suggest turning it off if you want to backtrace out of abort, etc.). Lose the "just affects the way a call instruction is printed out" bit -- what's "printed out"?, a user would ask. Some users aren't even aware there's a separate assembler that munches text is involved. I'd also suggest renaming the option to `-mallow-tailcall-noreturn' or `-mtailcall-noreturn' so that its spelling and description could be more easily shared with other targets (and perhaps promoted as -f option).
+@item -mjump-to-noreturn +@opindex mjump-to-noreturn +Use a jump instruction instead of a call instruction when calling a +no-return functions. This option is active if optimization is turned +on and just affects the way a call instruction is printed out. Would "emit" be better here than "printed out"? Gerald
> +@item -mjump-to-noreturn > +@opindex mjump-to-noreturn > +Use a jump instruction instead of a call instruction when calling a > +no-return functions. This option is active if optimization is turned > +on and just affects the way a call instruction is printed out. > > Would "emit" be better here than "printed out"? Maybe "generated"?
Pedro Alves schrieb: > On Friday 14 October 2011 18:19:00, Georg-Johann Lay wrote: > >>Paolo Bonzini schrieb: >> >>>On 10/14/2011 06:23 PM, Georg-Johann Lay wrote: >>> >>>>+@item -mjump-to-noreturn >>>>+@opindex mjump-to-noreturn >>>>+Use a jump instruction instead of a call instruction when calling a >>>>+no-return functions. This option is active if optimization is turned >>>>+on and just affects the way a call instruction is printed out. >>>>+Besides that, it has no effect on code generation or debug information. >>> >>>I think this is not really accurate given Richard's input. >> >>Confused. The conclusion was to introduce a new command line option in order >>to have individual control over this feature. The option is named >>-mjump-to-noreturn now instead of -mjump-noreturn. Is that what you mean? > > My 2c. You've used implementor-speak to describe the option, while you > should use user-speak. Mention that it saves stack, but the downside is > that it affects backtracing (and suggest turning it off if you want to > backtrace out of abort, etc.). Lose the "just affects the way a call > instruction is printed out" bit -- what's "printed out"?, a user > would ask. Some users aren't even aware there's a separate assembler > that munches text is involved. Thanks for your input. I am always grateful for hints on how to improve my english. > I'd also suggest renaming the option to `-mallow-tailcall-noreturn' > or `-mtailcall-noreturn' so that its spelling and description could > be more easily shared with other targets (and perhaps promoted > as -f option). Looking at -foptimize-sibling-calls there is just -foptimize-sibling-calls Optimize sibling and tail recursive calls. Enabled at levels -O2, -O3, -Os. so the new option could be -moptimize-noreturn-calls Optimize noreturn calls. This might make debugging harder but will save storing the return address when calling roreturn functions. Enabled at levels -O2, -O3, -Os. But the "makes debugging harder" clause is true for almost any optimization available, so should this tautology be mentioned along with each optimization that makes debugging harder? Or simply like this: -moptimize-noreturn-calls Optimize noreturn calls. Enabled at levels -O2, -O3, -Os. What about the -moptimize-noreturn-calls in analogy to -foptimize-sibling-calls? Sound good to me. Johann
> -moptimize-noreturn-calls > Optimize noreturn calls. This might make debugging harder but > will save storing the return address when calling roreturn > functions. > > Enabled at levels -O2, -O3, -Os. > > But the "makes debugging harder" clause is true for almost any optimization > available, so should this tautology be mentioned along with each > optimization that makes debugging harder? If there's one kind of debugging that you don't want to make harder, that's post mortem debugging. Paolo
Paolo Bonzini schrieb: >>-moptimize-noreturn-calls >> Optimize noreturn calls. This might make debugging harder but >> will save storing the return address when calling roreturn >> functions. >> >> Enabled at levels -O2, -O3, -Os. >> >>But the "makes debugging harder" clause is true for almost any optimization >>available, so should this tautology be mentioned along with each >>optimization that makes debugging harder? > > If there's one kind of debugging that you don't want to make harder, > that's post mortem debugging. > > Paolo And what is the conclusion? - Not to imlement this optimization at all? - Implement it but don't turn it on depending on -O* - Implement it as is but explicitely mention "post mortem debugging" in docs? There is no real post morten debugging on AVR as there is nothing like core dump. Johann
>There is no real post morten debugging on AVR as there is nothing like core dump.
But if there is any kind of debugging at all, you might use the debugger to put a breakpoint in abort(), and if so, having that invoked via jmp means you don't know who called it. So you'd want a way to suppress that optimization.
paul
Paul_Koning@Dell.com schrieb: >> There is no real post morten debugging on AVR as there is nothing like >> core dump. > > But if there is any kind of debugging at all, you might use the debugger to > put a breakpoint in abort(), and if so, having that invoked via jmp means > you don't know who called it. So you'd want a way to suppress that > optimization. > > paul Regarding the number of objections, I think it's best to drop this patch. The clean-up to the call insns is contained in http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01530.html Thanks for all of your feedback! Johann
Index: doc/invoke.texi =================================================================== --- doc/invoke.texi (revision 179993) +++ doc/invoke.texi (working copy) @@ -487,7 +487,7 @@ Objective-C and Objective-C++ Dialects}. @emph{AVR Options} @gccoptlist{-mmcu=@var{mcu} -mno-interrupts @gol --mcall-prologues -mtiny-stack -mint8 -mstrict-X} +-mcall-prologues -mtiny-stack -mint8 -mstrict-X -mjump-to-noreturn} @emph{Blackfin Options} @gccoptlist{-mcpu=@var{cpu}@r{[}-@var{sirevision}@r{]} @gol @@ -10690,6 +10690,13 @@ and long long will be 4 bytes. Please n comply to the C standards, but it will provide you with smaller code size. +@item -mjump-to-noreturn +@opindex mjump-to-noreturn +Use a jump instruction instead of a call instruction when calling a +no-return functions. This option is active if optimization is turned +on and just affects the way a call instruction is printed out. +Besides that, it has no effect on code generation or debug information. + @item -mstrict-X @opindex mstrict-X Use register @code{X} in a way proposed by the hardware. This means Index: config/avr/avr.md =================================================================== --- config/avr/avr.md (revision 179992) +++ config/avr/avr.md (working copy) @@ -133,11 +133,10 @@ (define_attr "length" "" ;; Following insn attribute tells if and how the adjustment has to be ;; done: ;; no No adjustment needed; attribute "length" is fine. -;; yes Analyse pattern in adjust_insn_length by hand. ;; Otherwise do special processing depending on the attribute. (define_attr "adjust_len" - "out_bitop, out_plus, addto_sp, tsthi, tstsi, compare, + "out_bitop, out_plus, addto_sp, tsthi, tstsi, compare, call, mov8, mov16, mov32, reload_in16, reload_in32, ashlqi, ashrqi, lshrqi, ashlhi, ashrhi, lshrhi, @@ -3634,21 +3633,12 @@ (define_insn "call_insn" ;; Operand 1 not used on the AVR. ;; Operand 2 is 1 for tail-call, 0 otherwise. "" - "@ - %!icall - %~call %x0 - %!ijmp - %~jmp %x0" + { + return avr_out_call (insn, operands[0], 0 != INTVAL (operands[2])); + } [(set_attr "cc" "clobber") - (set_attr_alternative "length" - [(const_int 1) - (if_then_else (eq_attr "mcu_mega" "yes") - (const_int 2) - (const_int 1)) - (const_int 1) - (if_then_else (eq_attr "mcu_mega" "yes") - (const_int 2) - (const_int 1))])]) + (set_attr "length" "1,*,1,*") + (set_attr "adjust_len" "*,call,*,call")]) (define_insn "call_value_insn" [(parallel[(set (match_operand 0 "register_operand" "=r,r,r,r") @@ -3658,21 +3648,12 @@ (define_insn "call_value_insn" ;; Operand 2 not used on the AVR. ;; Operand 3 is 1 for tail-call, 0 otherwise. "" - "@ - %!icall - %~call %x1 - %!ijmp - %~jmp %x1" + { + return avr_out_call (insn, operands[1], 0 != INTVAL (operands[3])); + } [(set_attr "cc" "clobber") - (set_attr_alternative "length" - [(const_int 1) - (if_then_else (eq_attr "mcu_mega" "yes") - (const_int 2) - (const_int 1)) - (const_int 1) - (if_then_else (eq_attr "mcu_mega" "yes") - (const_int 2) - (const_int 1))])]) + (set_attr "length" "1,*,1,*") + (set_attr "adjust_len" "*,call,*,call")]) (define_insn "nop" [(const_int 0)] Index: config/avr/avr.opt =================================================================== --- config/avr/avr.opt (revision 179993) +++ config/avr/avr.opt (working copy) @@ -65,3 +65,7 @@ Make the linker relaxation machine assum mstrict-X Target Report Var(avr_strict_X) Init(0) When accessing RAM, use X as imposed by the hardware, i.e. just use pre-decrement, post-increment and indirect addressing with the X register. Without this option, the compiler may assume that there is an addressing mode X+const similar to Y+const and Z+const and emit instructions to emulate such an addressing mode for X. + +mjump-to-noreturn +Target Report Var(avr_jump_to_noreturn) Init(0) +Jump to no-return functions instead of calling them. Index: config/avr/avr-protos.h =================================================================== --- config/avr/avr-protos.h (revision 179992) +++ config/avr/avr-protos.h (working copy) @@ -84,6 +84,7 @@ extern const char *avr_out_sbxx_branch ( extern const char* avr_out_bitop (rtx, rtx*, int*); extern const char* avr_out_plus (rtx*, int*, int*); extern const char* avr_out_addto_sp (rtx*, int*); +extern const char* avr_out_call (rtx, rtx, bool); extern bool avr_popcount_each_byte (rtx, int, int); extern int extra_constraint_Q (rtx x); Index: config/avr/avr.c =================================================================== --- config/avr/avr.c (revision 179993) +++ config/avr/avr.c (working copy) @@ -4928,6 +4928,28 @@ avr_out_plus (rtx *xop, int *plen, int * } +/* Print call insn INSN to the assembler file and return "". + ADDRESS is the target address. + If SIBCALL_P then INSN is a tail-call. */ + +const char* +avr_out_call (rtx insn, rtx address, bool sibcall_p) +{ + /* No need to waste stack or time for no-return calls. */ + + if (avr_jump_to_noreturn + && find_reg_note (insn, REG_NORETURN, NULL)) + sibcall_p = true; + + if (REG_P (address)) + output_asm_insn (sibcall_p ? "%!ijmp" : "%!icall", &address); + else + output_asm_insn (sibcall_p ? "%~jmp %x0" : "%~call %x0", &address); + + return ""; +} + + /* Output bit operation (IOR, AND, XOR) with register XOP[0] and compile time constant XOP[2]: @@ -5334,6 +5356,8 @@ adjust_insn_length (rtx insn, int len) case ADJUST_LEN_ASHLQI: ashlqi3_out (insn, op, &len); break; case ADJUST_LEN_ASHLHI: ashlhi3_out (insn, op, &len); break; case ADJUST_LEN_ASHLSI: ashlsi3_out (insn, op, &len); break; + + case ADJUST_LEN_CALL: len = AVR_HAVE_JMP_CALL ? 2 : 1; break; default: gcc_unreachable(); Index: common/config/avr/avr-common.c =================================================================== --- common/config/avr/avr-common.c (revision 179765) +++ common/config/avr/avr-common.c (working copy) @@ -29,6 +29,7 @@ static const struct default_options avr_option_optimization_table[] = { { OPT_LEVELS_1_PLUS, OPT_fomit_frame_pointer, NULL, 1 }, + { OPT_LEVELS_1_PLUS, OPT_mjump_to_noreturn, NULL, 1 }, { OPT_LEVELS_NONE, 0, NULL, 0 } };