Hi,
> diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
> index 365e9eb..8266fa0 100644
> --- a/gcc/config/msp430/msp430.c
> +++ b/gcc/config/msp430/msp430.c
> @@ -1807,7 +1807,6 @@ const char * const ATTR_CRIT = "critical";
> const char * const ATTR_LOWER = "lower";
> const char * const ATTR_UPPER = "upper";
> const char * const ATTR_EITHER = "either";
> -const char * const ATTR_NOINIT = "noinit";
> const char * const ATTR_PERSIST = "persistent";
>
> static inline bool
With this change removed so that ATTR_NOINIT is still defined, your patch is
ok for msp430 - the attribute still behaves as expected.
I'm holding off using default_elf_select_section instead of
default_select_section in msp430.c since there could be some possible conflicts
with the MSPABI introduced by using elf_select_section that need to be properly
considered before making the change.
I think default_select_section is supposed to be very terse, so I'm not sure
if it would be even be suitable to extend it to handle noinit.
With that said, could you please add the following FIXME to your patch:
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 365e9eba747..b403b1f5a42 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -2338,6 +2336,8 @@ msp430_select_section (tree decl, int reloc, unsigned
HOST_WIDE_INT align) {
if (TREE_CODE (decl) == FUNCTION_DECL)
return text_section;
+ /* FIXME: ATTR_NOINIT is handled generically in
+ default_elf_select_section. */
else if (has_attr (ATTR_NOINIT, decl))
return noinit_section;
else if (has_attr (ATTR_PERSIST, decl))
Also, the gcc.target/arm/noinit-attribute.c test works with msp430.
Why not create a effective-target keyword which checks for noinit support, so
the test can be gated on that condition and put in a generic part of the
testsuite?
After doing some further testing, I noticed the attribute does not work on
static variables. The attribute handling code allows the attribute to be set on
a static variable, but then that variable does not get placed in the .noinit
section.
What are your thoughts on this?
Does it even makes sense for a static local variable to be declared as "noinit"?
One should want a static local variable to be initialized, as having it not
initialized and hold a random value could cause problems.
Perhaps a user could think of some use for this, but I can't.
Finally, I would point out that "contrib/check_GNU_style.sh" reports some style
issues with your patch.
Thanks and regards,
Jozef
