Bug 53256 - [avr] Attribute 'interrupt' shall override attribute 'signal'
[avr] Attribute 'interrupt' shall override attribute 'signal'
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: target
4.7.0
: P4 enhancement
: 4.7.1
Assigned To: Georg-Johann Lay
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-06 11:41 UTC by Georg-Johann Lay
Modified: 2012-06-08 22:42 UTC (History)
1 user (show)

See Also:
Host:
Target: avr
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-05-06 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Georg-Johann Lay 2012-05-06 11:41:03 UTC
AVR-LibC offered attributes 'interrupt' and 'signal' as INTERRUPT and SIGNAL through its API.  This changed at some point in time to ISR which may lead to code that adds attribute 'interrupt' and attribute 'signal' at the same time.

This leads to code that is not IRQ-safe in the current implementation which is:

1) Attributes 'interrupt' and 'signal' are exclusive. Specifying both at the same time for the same function has not a well-defined behavior.

2) 'interrupt' emits a SEI instruction as the very first instruction of the function.

3) Neither attributes make assumptions on the function's worker code. In particular, they do *not* assume that interrupts are not (re-)enabled by the user code.  Thus, it is safe to enable IRQs at any place in the function body.  This "safe" covers only the generated code and IRQ-safeness of SP changes and the instructions themselves, it does *not* cover the IRQ and timing layout of the application.

4) The AVR hardware globally disables IRQs when an IRQ is raised. This allows more efficient code in 'signal' prologues compared to 'interrupt' prologues in cases where the stack pointer has to be written.  Writing SP is not an atomic operation on all devices, and special precautions must be taken if SP is written and IRQs might be on.


As of this extension request 1) shall be changed to:

1') Attribute 'interrupt' shall override attribute 'signal'. No warning should be issued in such a case.  The documentation should be extended.
Comment 1 Georg-Johann Lay 2012-05-09 16:29:06 UTC
Author: gjl
Date: Wed May  9 16:28:53 2012
New Revision: 187342

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187342
Log:
	PR target/53256
	* config/avr/elf.h (ASM_DECLARE_FUNCTION_NAME): Remove.
	* config/avr/avr-protos.h (avr_asm_declare_function_name): Remove.
	* config/avr/avr.h (struct machine_function): Add attributes_checked_p.
	* config/avr/avr.c (avr_asm_declare_function_name): Remove.
	(expand_prologue): Move initialization of cfun->machine->is_naked,
	is_interrupt, is_signal, is_OS_task, is_OS_main from here to...
	(avr_set_current_function): ...this new static function.
	(TARGET_SET_CURRENT_FUNCTION): New define.
	(avr_function_ok_for_sibcall): Use cfun->machine->is_* instead of
	checking attributes of current_function_decl.
	(avr_regs_to_save): Ditto.
	(signal_function_p): Rename to avr_signal_function_p.
	(interrupt_function_p): Rename to avr_interrupt_function_p.
	* doc/extend.texi (Function Attributes): Better explanation of
	'interrupt' and 'signal' for AVR. Move 'ifunc' down to establish
	alphabetical order.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr-protos.h
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.h
    trunk/gcc/config/avr/elf.h
    trunk/gcc/doc/extend.texi
Comment 2 Georg-Johann Lay 2012-05-09 16:39:39 UTC
Author: gjl
Date: Wed May  9 16:39:33 2012
New Revision: 187343

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187343
Log:
	Backport from 2012-05-12 mainline r187342.
	PR target/53256
	* config/avr/elf.h (ASM_DECLARE_FUNCTION_NAME): Remove.
	* config/avr/avr-protos.h (avr_asm_declare_function_name): Remove.
	* config/avr/avr.h (struct machine_function): Add attributes_checked_p.
	* config/avr/avr.c (avr_asm_declare_function_name): Remove.
	(expand_prologue): Move initialization of cfun->machine->is_naked,
	is_interrupt, is_signal, is_OS_task, is_OS_main from here to...
	(avr_set_current_function): ...this new static function.
	(TARGET_SET_CURRENT_FUNCTION): New define.
	(avr_function_ok_for_sibcall): Use cfun->machine->is_* instead of
	checking attributes of current_function_decl.
	(avr_regs_to_save): Ditto.
	(signal_function_p): Rename to avr_signal_function_p.
	(interrupt_function_p): Rename to avr_interrupt_function_p.
	* doc/extend.texi (Function Attributes): Better explanation of
	'interrupt' and 'signal' for AVR. Move 'ifunc' down to establish
	alphabetical order.


Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/config/avr/avr-protos.h
    branches/gcc-4_7-branch/gcc/config/avr/avr.c
    branches/gcc-4_7-branch/gcc/config/avr/avr.h
    branches/gcc-4_7-branch/gcc/config/avr/elf.h
    branches/gcc-4_7-branch/gcc/doc/extend.texi
Comment 3 Georg-Johann Lay 2012-05-09 17:03:49 UTC
Added in 4.7.1
Comment 4 Georg-Johann Lay 2012-06-08 22:42:27 UTC
See also the following AVR-LibC bug report:

http://savannah.nongnu.org/bugs/index.php?26734