[MSP430] Allow interrupt handers to be static and fix __interrupt__ attribute causing an ICE
diff mbox series

Message ID 7e90fb25-64b7-ffc6-cf9f-49b0fe388038@mittosystems.com
State New
Headers show
Series
  • [MSP430] Allow interrupt handers to be static and fix __interrupt__ attribute causing an ICE
Related show

Commit Message

Jozef Lawrynowicz May 27, 2018, 1:19 p.m. UTC
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.

Comments

Jozef Lawrynowicz May 27, 2018, 2:13 p.m. UTC | #1
On 27/05/18 14:19, Jozef Lawrynowicz wrote:
> If the patch is acceptable, I would appreciate if someone would commit 
> it for
> me, as I don't have write access.
>
"msp430.md" in the ChangeLog entry should be "msp430.c".

Patch
diff mbox series

From 18d1980bb4061cee9d97f2b7737af2dfd9e52c34 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@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  <jozef.l@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