Bug 21683

Summary:	[avr] Support a pseudo-instruction to allow more efficient GCC ISR prologues
Product:	binutils	Reporter:	Georg-Johann Lay <gjl>
Component:	gas	Assignee:	Georg-Johann Lay <gjl>
Status:	RESOLVED FIXED
Severity:	enhancement	CC:	ampva300, cyrusssslim, dianeBerlusconi, mehmetgelisin, olivernicholle9052
Priority:	P3
Version:	unspecified
Target Milestone:	2.29
Host:		Target:	avr
Build:		Last reconfirmed:
Attachments:	bin-pr21683-gccisr.diff: Proposed patch.

Description Georg-Johann Lay 2017-06-28 10:56:28 UTC

Background: avr-gcc currently generated ISR prologues and epilogues less efficient than possible.  This is due to avr back-end design and decisions.  Solving this in avr-gcc alone would be inappropriately amount of work and instability, maybe even reducing code performance in other places than ISRs.  This is the reason for why

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=20296

is open for > 12 years now and has been suspended.

A comined GCC + GAS approach would be feasible, and this PR is the GAS part:

GAS implements a __gcc_isr pseudo-instruction which scans the code and emits parts of ISR prologue and epilogue.

__gcc_isr 1: GAS will replace it by an ISR prologue chunk (at most 6 insns).

__gcc_isr 2: GAS will replace it by an ISR epilogue chunk (at most 5 insns).

__gcc_isr 0, Rx:  Scan the code since the last prologue chunk and replace all encountered prologue and epilogue chunks by appropriate code. Rx is general purpose register.

During parse (md_assemble), prologue and epilogue chunks are represented as rs_machine_dependent fragments.  If "__gcc_isr 0, Rx" is seen, all rs_machine_dependent fragments in the current chain are transformed into ordinary rs_fill code fragments of then known fixed size.

After parse (md_pre_output_hook) a sanity check is performed which reports any dangling chunks.

__gcc_isr is only available with the new command line option -mgcc-isr

Comment 1 Georg-Johann Lay 2017-06-28 15:10:35 UTC

Created attachment 10232 [details]
bin-pr21683-gccisr.diff: Proposed patch.

/
	PR gas/21683
	* include/opcode/avr.h (AVR_INSN): Add one for __gcc_isr.

gas/
	PR gas/21683
	* doc/c-avr.texi (AVR Options) <-mgcc-isr>: Document it.
	(AVR Pseudo Instructions): New node.

	* config/tc-avr.h (md_pre_output_hook): Define to avr_pre_output_hook.
	(md_undefined_symbol): Define to avr_undefined_symbol.
	(avr_pre_output_hook, avr_undefined_symbol): New protos.

	* config/tc-avr.c (struc-symbol.h): Include it.
	(ISR_CHUNK_Done, ISR_CHUNK_Prologue, ISR_CHUNK_Epilogue): New enums.
	(avr_isr, avr_gccisr_opcode)
	(avr_no_sreg_hash, avr_no_sreg): New static variables.
	(avr_opt_s) <have_gccisr>: Add field.
	(avr_opt): Add initializer for have_gccisr.
	(enum options) <OPTION_HAVE_GCCISR>: Add enum.
	(md_longopts) <"mgcc-isr">: Add entry.
	(md_show_usage): Document -mgcc-isr.
	(md_parse_option) [OPTION_HAVE_GCCISR]: Handle it.
	(md_undefined_symbol): Remove.
	(avr_undefined_symbol, avr_pre_output_hook): New fuctions.
	(md_begin) <avr_no_sreg_hash, avr_gccisr_opcode>: Initialize them.
	(avr_operand) <pregno>: Add argument and set *pregno if function
	is called for a register constraint.
	[N]: Handle constraint.
	(avr_operands) <avr_operand>: Pass 5th parameter to calls.
	[avr_opt.have_gccisr]: Call avr_update_gccisr.  Call
	avr_gccisr_operands instead of avr_operands.
	(avr_update_gccisr, avr_emit_insn, avr_patch_gccisr_frag)
	(avr_gccisr_operands, avr_check_gccisr_done): New static functions.

	* testsuite/gas/avr/gccisr-01.d: New test.
	* testsuite/gas/avr/gccisr-01.s: New test.
	* testsuite/gas/avr/gccisr-02.d: New test.
	* testsuite/gas/avr/gccisr-02.s: New test.
	* testsuite/gas/avr/gccisr-03.d: New test.
	* testsuite/gas/avr/gccisr-03.s: New test.

Comment 2 Sourceware Commits 2017-06-30 15:39:14 UTC

The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=32f76c677333510350f21a40db062a8d17995c53

commit 32f76c677333510350f21a40db062a8d17995c53
Author: Georg-Johann Lay <avr@gjlay.de>
Date:   Fri Jun 30 16:37:39 2017 +0100

    Add support for a __gcc_isr pseudo isntruction to the AVR assembler.
    
        PR gas/21683
    include * opcode/avr.h (AVR_INSN): Add one for __gcc_isr.
    
    gas * doc/c-avr.texi (AVR Options) <-mgcc-isr>: Document it.
        (AVR Pseudo Instructions): New node.
        * config/tc-avr.h (md_pre_output_hook): Define to avr_pre_output_hook.
        (md_undefined_symbol): Define to avr_undefined_symbol.
        (avr_pre_output_hook, avr_undefined_symbol): New protos.
        * config/tc-avr.c (struc-symbol.h): Include it.
        (ISR_CHUNK_Done, ISR_CHUNK_Prologue, ISR_CHUNK_Epilogue): New enums.
        (avr_isr, avr_gccisr_opcode)
        (avr_no_sreg_hash, avr_no_sreg): New static variables.
        (avr_opt_s) <have_gccisr>: Add field.
        (avr_opt): Add initializer for have_gccisr.
        (enum options) <OPTION_HAVE_GCCISR>: Add enum.
        (md_longopts) <"mgcc-isr">: Add entry.
        (md_show_usage): Document -mgcc-isr.
        (md_parse_option) [OPTION_HAVE_GCCISR]: Handle it.
        (md_undefined_symbol): Remove.
        (avr_undefined_symbol, avr_pre_output_hook): New fuctions.
        (md_begin) <avr_no_sreg_hash, avr_gccisr_opcode>: Initialize them.
        (avr_operand) <pregno>: Add argument and set *pregno if function
        is called for a register constraint.
        [N]: Handle constraint.
        (avr_operands) <avr_operand>: Pass 5th parameter to calls.
        [avr_opt.have_gccisr]: Call avr_update_gccisr.  Call
        avr_gccisr_operands instead of avr_operands.
        (avr_update_gccisr, avr_emit_insn, avr_patch_gccisr_frag)
        (avr_gccisr_operands, avr_check_gccisr_done): New static functions.
        * testsuite/gas/avr/gccisr-01.d: New test.
        * testsuite/gas/avr/gccisr-01.s: New test.
        * testsuite/gas/avr/gccisr-02.d: New test.
        * testsuite/gas/avr/gccisr-02.s: New test.
        * testsuite/gas/avr/gccisr-03.d: New test.
        * testsuite/gas/avr/gccisr-03.s: New test.

Comment 3 Georg-Johann Lay 2017-06-30 16:24:54 UTC

Done.

Comment 4 Roman Jay Almaza 2021-06-26 01:38:59 UTC Comment hidden (spam)

Thanks for the fixed. Please keep it up.
https://jrsretainingwallsanaheim.com/

Comment 5 Joe Anderson 2021-07-02 02:11:08 UTC Comment hidden (spam)

Nice! You did great on figuring out what's the real problem. Thanks for the hard work in fixing the currently generated ISR prologues. 

Greetings from all of us at https://www.pasadenaconcretepros.com/

Comment 6 Clyde 2021-07-30 10:16:44 UTC Comment hidden (spam)

Glad it has been fixed. Thanks!
https://www.gulfcoastwebsitedesign.com

Comment 7 Mehmet gelisin 2021-09-10 19:36:38 UTC Comment hidden (spam)

lst the ISA of AVR ATtiny416 / 417 / 816 / 817 would fit avrxmega2, these sevices feature the flash being accessible via the SRAM address range and instructions.  This means that .rodata need not to be part of .data which is the case for all other AVR devices (except avrtiny).

Using SRAM addresses for .rodata requires that such SRAM addresses are offset by 0x8000 compared to the flash addresses as accessible by LPM, for example like so:

__RODATA_PM_OFFSET__ = DEFINED(__RODATA_PM_OFFSET__) ? __RODATA_PM_OFFSET__ : 0x8000;


  .text   :
  {
     ...   
  }  > text
  .rodata ADDR(.text) + SIZEOF (.text) + __RODATA_PM_OFFSET__ :
  {
    *(.rodata)
    *(.rodata*)
    *(.gnu.linkonce.r*)
  } AT> text
  .data :
  {
     ...

Consequently, we want a new emulation to support that feature in an optimal

lst the ISA of AVR ATtiny416 / 417 / 816 / 817 would fit avrxmega2, these sevices feature the flash being accessible via the SRAM address range and instructions.  This means that .rodata need not to be part of .data which is the case for all other AVR devices (except avrtiny).
 https://komiya-dental.com/ 
Using SRAM addresses for .rodata requires that such SRAM addresses are offset by 0x8000 compared to the flash addresses as accessible by LPM, for example like so:

__RODATA_PM_OFFSET__ = DEFINED(__RODATA_PM_OFFSET__) ? __RODATA_PM_OFFSET__ : 0x8000;

http://www.iu-bloomington.com/ 
  .text   :
  {
     ...   
  }  > text
  .rodata ADDR(.text) + SIZEOF (.text) + __RODATA_PM_OFFSET__ :
  {
    *(.rodata)
    *(.rodata*)
    *(.gnu.linkonce.r*) https://www.webb-dev.co.uk/ 
  } AT> text
  .data :
  {
     ...

Consequently, we want a new emulation to support that feature in an optimal
 https://waytowhatsnext.com/  
lst the ISA of AVR ATtiny416 / 417 / 816 / 817 would fit avrxmega2, these sevices feature the flash being accessible via the SRAM address range and instructions.  This means that .rodata need not to be part of .data which is the case for all other AVR devices (except avrtiny).

Using SRAM addresses for .rodata requires that such SRAM addresses are offset by 0x8000 compared to the flash addresses as accessible by LPM, for example like so:
http://www.acpirateradio.co.uk/ 
__RODATA_PM_OFFSET__ = DEFINED(__RODATA_PM_OFFSET__) ? __RODATA_PM_OFFSET__ : 0x8000;


  .text   :
  {
     ...   
  }  > text http://www.logoarts.co.uk/ 
  .rodata ADDR(.text) + SIZEOF (.text) + __RODATA_PM_OFFSET__ : 
  {
    *(.rodata)
    *(.rodata*)
    *(.gnu.linkonce.r*)
  } AT> text
  .data :
  { http://www.slipstone.co.uk/ 
     ...

Consequently, we want a new emulation to support that feature in an optimal

lst the ISA of AVR ATtiny416 / 417 / 816 / 817 would fit avrxmega2, these http://embermanchester.uk/ sevices feature the flash being accessible via the SRAM address range and instructions.  This means that .rodata need not to be part of .data which is the case for all other AVR devices (except avrtiny). http://connstr.net/ 

Using SRAM addresses for .rodata requires that such http://joerg.li/ SRAM addresses are offset by 0x8000 compared to the flash addresses as accessible by LPM, for example like so:
http://www.jopspeech.com/
__RODATA_PM_OFFSET__ = DEFINED(__RODATA_PM_OFFSET__) ? __RODATA_PM_OFFSET__ : 0x8000;


  .text   : http://www.wearelondonmade.com/
  {
     ...   
  }  > text
  .rodata ADDR(.text) + SIZEOF (.text) + __RODATA_PM_OFFSET__ :
  {
    *(.rodata) http://www.compilatori.com/
    *(.rodata*)
    *(.gnu.linkonce.r*)
  } AT> text
  .data :
  {
     ... http://www-look-4.com/

Consequently, we want a new emulation to support that feature in an optimal

Comment 8 Steve Anderson 2021-09-22 03:50:25 UTC Comment hidden (spam)

Scan the code since the last prologue chunk and replace all encountered prologue and epilogue chunks by appropriate code. Rx is general purpose register.
https://www.albuquerque-artificial-grass.com