Bug 20296 - Speeding up small interrupts on avr
Summary: Speeding up small interrupts on avr
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.1.0
: P5 enhancement
Target Milestone: 8.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on: 81268
Blocks:
  Show dependency treegraph
 
Reported: 2005-03-02 23:47 UTC by berndtrog
Modified: 2017-07-11 03:46 UTC (History)
6 users (show)

See Also:
Host:
Target: avr
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-11-26 01:35:32


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description berndtrog 2005-03-02 23:47:47 UTC
When I compile (avr-gcc -Os -c -mmcu=at90s2313) this:

void SIG_PIN_CHANGE0 (void) __attribute__ ((signal)); void SIG_PIN_CHANGE0 (void)
{
  (*(volatile unsigned char *)((0x12) + 0x20)) |= 1;
}

I get:
00000000 <SIG_PIN_CHANGE0>:
   0:   1f 92           push    r1
   2:   0f 92           push    r0
   4:   0f b6           in      r0, 0x3f        ; 63
   6:   0f 92           push    r0
   8:   11 24           eor     r1, r1
   a:   90 9a           sbi     0x12, 0 ; 18
   c:   0f 90           pop     r0
   e:   0f be           out     0x3f, r0        ; 63
  10:   0f 90           pop     r0
  12:   1f 90           pop     r1
  14:   18 95           reti


If the optimizer?/backend knows that the 'sbi' instruction does not
 modify r1 or the status register (0x3f) it could generate this instead:
  
sbi 0x12, 0
reti

Comments?
Comment 1 Björn Haase 2005-03-04 19:51:20 UTC
Hi, 
 
IMHO everyone working on the avr back-end is aware of this problem. The 
difficulty is, that the present architecture of the avr back-end does not 
easily permit to improve this case: Every instruction pattern (like "multiply 
two 16 bit integers" or "sign-extend a 16 bit variable to 32 bits") presently 
is free to assume that may overwrite or change r0 and r1 unless it leaves the 
"__zero_reg__" with 0 after finishing it's task. 
 
Resolving this issue, IMHO, would require a major refactoring of the 
back-end. ... IIUC the keyword is "replace all of the more complex instruction 
patterns by RTL expressions." 
 
I suggest to mark this bug as "desired enhancement". 
 
Yours, 
 
Björn 
Comment 2 Wim Lewis 2011-11-06 20:39:49 UTC
FWIW, confirming that versions 4.3.4 and 4.6.2 still emit these unnecessary saves of r0 and r1 for small ISRs (the generated code is identical to what berndtrog@yahoo.com posted, at least for optimization levels >= 1).
Comment 3 Georg-Johann Lay 2017-07-10 09:49:50 UTC
Author: gjl
Date: Mon Jul 10 09:49:18 2017
New Revision: 250093

URL: https://gcc.gnu.org/viewcvs?rev=250093&root=gcc&view=rev
Log:
gcc/
	Better ISR prologues by supporting GASes __gcc_isr pseudo insn.
	PR target/20296
	PR target/81268
	* configure.ac [target=avr]: Add GAS check for -mgcc-isr.
	(HAVE_AS_AVR_MGCCISR_OPTION):  If so, AC_DEFINE it.
	* config.in: Regenerate.
	* configure: Regenerate.
	* doc/extend.texi (AVR Function Attributes) <no_gccisr>: Document it.
	* doc/invoke.texi (AVR Options) <-mgas-isr-prologues>: Document it.
	* config/avr/avr.opt (-mgas-isr-prologues): New option and...
	(TARGET_GASISR_PROLOGUES): ...target mask.
	* common/config/avr/avr-common.c
	(avr_option_optimization_table) [OPT_LEVELS_1_PLUS_NOT_DEBUG]:
	Set -mgas-isr-prologues.
	* config/avr/avr-passes.def (avr_pass_pre_proep): Add
	INSERT_PASS_BEFORE for it.
	* config/avr/avr-protos.h (make_avr_pass_pre_proep): New proto.
	* config/avr/avr.c (avr_option_override)
	[!HAVE_AS_AVR_MGCCISR_OPTION]: Unset TARGET_GASISR_PROLOGUES.
	(avr_no_gccisr_function_p, avr_hregs_split_reg): New static functions.
	(avr_attribute_table) <no_gccisr>: Add new function attribute.
	(avr_set_current_function) <is_no_gccisr>: Init machine field.
	(avr_pass_data_pre_proep, avr_pass_pre_proep): New pass data
	and rtl_opt_pass.
	(make_avr_pass_pre_proep): New function.
	(emit_push_sfr) <treg>: Add argument to function and use it
	instead of TMP_REG.
	(avr_expand_prologue) [machine->gasisr.maybe]: Emit gasisr insn
	and set machine->gasisr.yes.
	(avr_expand_epilogue) [machine->gasisr.yes]: Similar.
	(avr_asm_function_end_prologue) [machine->gasisr.yes]: Add
	__gcc_isr.n_pushed to .L__stack_usage.
	(TARGET_ASM_FINAL_POSTSCAN_INSN): Define to...
	(avr_asm_final_postscan_insn): ...this new static function.
	* config/avr/avr.h (machine_function)
	<is_no_gccisr, use_L__stack_usage>: New fields.
	<gasisr, gasisr.yes, gasisr.maybe, gasisr.regno>: New fields.
	* config/avr/avr.md (UNSPECV_GASISR): Add unspecv enum.
	(GASISR_Prologue, GASISR_Epilogue, GASISR_Done): New define_constants.
	(gasisr, *gasisr): New expander and insn.
	* config/avr/gen-avr-mmcu-specs.c (print_mcu)
	[HAVE_AS_AVR_MGCCISR_OPTION]: Print asm_gccisr spec.
	* config/avr/specs.h (ASM_SPEC) <asm_gccisr>: Add sub spec.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/common/config/avr/avr-common.c
    trunk/gcc/config.in
    trunk/gcc/config/avr/avr-passes.def
    trunk/gcc/config/avr/avr-protos.h
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.h
    trunk/gcc/config/avr/avr.md
    trunk/gcc/config/avr/avr.opt
    trunk/gcc/config/avr/gen-avr-mmcu-specs.c
    trunk/gcc/config/avr/specs.h
    trunk/gcc/configure
    trunk/gcc/configure.ac
    trunk/gcc/doc/extend.texi
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/gcc.target/avr/isr-test.h
Comment 4 Georg-Johann Lay 2017-07-10 10:03:46 UTC
Fixed in v8 by means of PR81268.

Binutils must support

https://sourceware.org/bugzilla/show_bug.cgi?id=21683

in order for the feature to become active.  Binutils 2.29 should do.