Is the revised patch acceptable?
If it is, I don't have commit access, so I'd apprecite it if someone
could commit it for me. Similarly, I can comment on the Bugzilla
entry but can't change it's state - assuming that would be appropriate
if the patch is accepted.
Thanks,
Joe
On 01/09/2016 17:18, Joe Seymour wrote:
> 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" } */
>
>
