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

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  <joe.s@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.

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  <joe.s@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" } */
> 
>