Some users have a preference for declaring interrupt handlers as static functions, as this enforces that interrupts should not be called directly (from other source files at least).
This patch allows interrupt handlers to be declared as static and also fixes an assertion failure when the interrupt attribute is written with the leading and trailing underscores included in the name. Successfully regtested gcc testsuite on trunk for msp430-elf in the -mcpu=msp430x/-mlarge variation. If the patch is acceptable, I would appreciate if someone would commit it for me, as I don't have write access.
>From 18d1980bb4061cee9d97f2b7737af2dfd9e52c34 Mon Sep 17 00:00:00 2001 From: Jozef Lawrynowicz <joze...@mittosystems.com> Date: Sun, 27 May 2018 12:55:41 +0100 Subject: [PATCH] MSP430: Allow interrupt handlers to be static and fix the __interrupt__ attribute causing an ICE 2018-05-27 Jozef Lawrynowicz <joze...@mittosystems.com> * gcc/config/msp430/msp430.md (msp430_attr): Allow interrupt handlers to be static and remove check on interrupt attribute name. gcc/testsuite/gcc.target/msp430/ * function-attributes-4.c: New test. * static-interrupts.c: New test. --- gcc/config/msp430/msp430.c | 17 ++-- .../gcc.target/msp430/function-attributes-4.c | 111 +++++++++++++++++++++ .../gcc.target/msp430/static-interrupts.c | 26 +++++ 3 files changed, 144 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/gcc.target/msp430/function-attributes-4.c create mode 100644 gcc/testsuite/gcc.target/msp430/static-interrupts.c diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c index a8fed12..85626a2 100644 --- a/gcc/config/msp430/msp430.c +++ b/gcc/config/msp430/msp430.c @@ -1867,11 +1867,9 @@ msp430_attr (tree * node, { gcc_assert (DECL_P (* node)); + /* Only the interrupt attribute takes an argument. */ if (args != NULL) { - /* Only the interrupt attribute takes an argument. */ - gcc_assert (TREE_NAME_EQ (name, ATTR_INTR)); - tree value = TREE_VALUE (args); switch (TREE_CODE (value)) @@ -1916,13 +1914,12 @@ msp430_attr (tree * node, if (TREE_CODE (TREE_TYPE (* node)) == FUNCTION_TYPE && ! VOID_TYPE_P (TREE_TYPE (TREE_TYPE (* node)))) message = "interrupt handlers must be void"; - - if (! TREE_PUBLIC (* node)) - message = "interrupt handlers cannot be static"; - - /* Ensure interrupt handlers never get optimised out. */ - TREE_USED (* node) = 1; - DECL_PRESERVE_P (* node) = 1; + else + { + /* Ensure interrupt handlers never get optimised out. */ + TREE_USED (* node) = 1; + DECL_PRESERVE_P (* node) = 1; + } } else if (TREE_NAME_EQ (name, ATTR_REENT)) { diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-4.c b/gcc/testsuite/gcc.target/msp430/function-attributes-4.c new file mode 100644 index 0000000..07d13c9 --- /dev/null +++ b/gcc/testsuite/gcc.target/msp430/function-attributes-4.c @@ -0,0 +1,111 @@ +/* { dg-do compile } */ +/* Check that the foo interrupt vectors aren't actually removed. */ +/* { dg-final { scan-assembler-times "__interrupt_vector_foo" 2 } } */ + +/* Check that warnings are emitted when attributes are used incorrectly and + that attributes are interpreted correctly whether leading and trailing + underscores are used or not. */ + +void __attribute__((__naked__,__reentrant__)) +fn1(void) +{ /* { dg-warning "naked functions cannot be reentrant" } */ +} + +void __attribute__((naked,reentrant)) +fn2(void) +{ /* { dg-warning "naked functions cannot be reentrant" } */ +} + +void __attribute__((__reentrant__,__naked__)) +fn3(void) +{ /* { dg-warning "reentrant functions cannot be naked" } */ +} + +void __attribute__((reentrant,naked)) +fn4(void) +{ /* { dg-warning "reentrant functions cannot be naked" } */ +} + +void __attribute__((__critical__,__reentrant__)) +fn5(void) +{ /* { dg-warning "critical functions cannot be reentrant" } */ +} + +void __attribute__((critical,reentrant)) +fn6(void) +{ /* { dg-warning "critical functions cannot be reentrant" } */ +} + +void __attribute__((__reentrant__,__critical__)) +fn7(void) +{ /* { dg-warning "reentrant functions cannot be critical" } */ +} + +void __attribute__((reentrant,critical)) +fn8(void) +{ /* { dg-warning "reentrant functions cannot be critical" } */ +} + +void __attribute__((__critical__,__naked__)) +fn9(void) +{ /* { dg-warning "critical functions cannot be naked" } */ +} + +void __attribute__((critical,naked)) +fn10(void) +{ /* { dg-warning "critical functions cannot be naked" } */ +} + +void __attribute__((__naked__,__critical__)) +fn11(void) +{ /* { dg-warning "naked functions cannot be critical" } */ +} + +void __attribute__((naked,critical)) +fn12(void) +{ /* { dg-warning "naked functions cannot be critical" } */ +} + +int __attribute__((interrupt)) +isr1 (void) +{ /* { dg-warning "interrupt handlers must be void" } */ +} + +int __attribute__((__interrupt__)) +isr2 (void) +{ /* { dg-warning "interrupt handlers must be void" } */ +} + +void __attribute__((interrupt("foo1"))) +isr3 (void) +{ /* { dg-warning "unrecognized interrupt vector argument" } */ +} + +void __attribute__((__interrupt__("foo2"))) +isr4 (void) +{ /* { dg-warning "unrecognized.*interrupt vector argument" } */ +} + +void __attribute__((interrupt(65))) +isr5 (void) +{ /* { dg-warning "numeric argument of 'interrupt' attribute must be in range 0..63" } */ +} + +void __attribute__((__interrupt__(100))) +isr6 (void) +{ /* { dg-warning "numeric argument of 'interrupt' attribute must be in range 0..63" } */ +} + +void __attribute__((interrupt(0.5))) +isr7 (void) +{ /* { dg-warning "argument of 'interrupt' attribute is not a string constant or number" } */ + volatile int __attribute__((__naked__)) + a; /* { dg-warning "'naked' attribute only applies to functions" } */ +} + +void __attribute__((__interrupt__(1.5))) +isr8 (void) +{ /* { dg-warning "argument of 'interrupt' attribute is not a string constant or number" } */ + volatile int __attribute__((naked)) + a; /* { dg-warning "'naked' attribute only applies to functions" } */ +} diff --git a/gcc/testsuite/gcc.target/msp430/static-interrupts.c b/gcc/testsuite/gcc.target/msp430/static-interrupts.c new file mode 100644 index 0000000..06d9ea6 --- /dev/null +++ b/gcc/testsuite/gcc.target/msp430/static-interrupts.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-Os" } */ +/* { dg-final { scan-assembler-times "__interrupt_vector_" 4 } } */ + +/* Test that interrupts aren't optimised out and that "__interrupt__" and + "interrupt" can be used interchangeably. */ + +static void __attribute__((interrupt(1))) +isr_static (void) +{ +} + +static void __attribute__((__interrupt__(2))) +isr_static_alt (void) +{ +} + +void __attribute__((interrupt(3))) +isr_global (void) +{ +} + +void __attribute__((__interrupt__(4))) +isr_global_alt (void) +{ +} -- 2.7.4