Re: [PATCH][msp430] Don't output __interrupt_vector sections for weak functions

[Joe Seymour]

On 29/08/2016 22:09, DJ Delorie wrote:
> 
> Which results in a more user-obvious case, ignoring the interrupt
> attribute or ignoring the weak attribute?  I would think that we never
> want to compile and link successfully if we can't do what the user
> wants, and omitting an interrupt handler is... bad.
> 
> I think this should either be a hard error, so the user must fix it
> right away, or ignore the weak, which results in a linker error
> (somehow? maybe?) if the user specifies two handlers for the same
> interrupt.

Thanks for the review.  My original intention was to make it an error, but
msp430_attr() seemed to set the precedent of emitting warnings for invalid
attributes, so I tried to follow that convention.

Here's a patch that produces an error instead:

2016-09-XX  Joe Seymour  <jo...@somniumtech.com>

        gcc/
        PR target/70713
        * config/msp430/msp430.c (msp430_start_function): Emit an error
        if a function is both weak and specifies an interrupt number.

        gcc/testsuite/
        PR target/70713
        * gcc.target/msp430/function-attributes-1.c: New test.
        * gcc.target/msp430/function-attributes-2.c: New test.
        * gcc.target/msp430/function-attributes-3.c: New test.

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index dba4d19..c40d2da 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -2108,6 +2108,13 @@ msp430_start_function (FILE *file, const char *name, 
tree decl)
        {
          char buf[101];
 
+         /* Interrupt vector sections should be unique, but use of weak
+            functions implies multiple definitions.  */
+         if (DECL_WEAK (decl))
+           {
+             error ("argument to interrupt attribute is unsupported for weak 
functions");
+           }
+
          intr_vector = TREE_VALUE (intr_vector);
 
          /* The interrupt attribute has a vector value.  Turn this into a
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-1.c 
b/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
new file mode 100644
index 0000000..7a3b7be
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
@@ -0,0 +1,9 @@
+void __attribute__((weak, interrupt))
+weak_interrupt (void) {
+}
+
+void __attribute__((interrupt(11)))
+interrupt_number (void) {
+}
+
+/* { dg-final { scan-assembler-times "__interrupt_vector_" 1 } } */
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-2.c 
b/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
new file mode 100644
index 0000000..fcb2fb2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
@@ -0,0 +1,3 @@
+void __attribute__((weak, interrupt(10)))
+weak_interrupt_number (void) {
+} /* { dg-error "argument to interrupt attribute is unsupported for weak 
functions" } */
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-3.c 
b/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
new file mode 100644
index 0000000..b0acf4a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
@@ -0,0 +1,3 @@
+void __attribute__((interrupt("nmi"))) __attribute__((weak))
+interrupt_name_weak (void) {
+} /* { dg-error "argument to interrupt attribute is unsupported for weak 
functions" } */