This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add generic support for "noinit" attribute


On 7/8/19 5:10 AM, Christophe Lyon wrote:
On Sat, 6 Jul 2019 at 19:57, Martin Sebor <msebor@gmail.com> wrote:

On 7/4/19 9:27 AM, Christophe Lyon wrote:
Hi,

Similar to what already exists for TI msp430 or in TI compilers for
arm, this patch adds support for the "noinit" attribute.

It is convenient for embedded targets where the user wants to keep the
value of some data when the program is restarted: such variables are
not zero-initialized. It is mostly a helper/shortcut to placing
variables in a dedicated section.

It's probably desirable to add the following chunk to the GNU linker:
diff --git a/ld/emulparams/armelf.sh b/ld/emulparams/armelf.sh
index 272a8bc..9555cec 100644
--- a/ld/emulparams/armelf.sh
+++ b/ld/emulparams/armelf.sh
@@ -10,7 +10,19 @@ OTHER_TEXT_SECTIONS='*(.glue_7t) *(.glue_7)
*(.vfp11_veneer) *(.v4_bx)'
   OTHER_BSS_SYMBOLS="${CREATE_SHLIB+PROVIDE (}__bss_start__ =
.${CREATE_SHLIB+)};"
   OTHER_BSS_END_SYMBOLS="${CREATE_SHLIB+PROVIDE (}_bss_end__ =
.${CREATE_SHLIB+)}; ${CREATE_SHLIB+PROVIDE (}__bss_end__ =
.${CREATE_SHLIB+)};"
   OTHER_END_SYMBOLS="${CREATE_SHLIB+PROVIDE (}__end__ = .${CREATE_SHLIB+)};"
   -OTHER_SECTIONS='.note.gnu.arm.ident 0 : { KEEP (*(.note.gnu.arm.ident)) }'
   +OTHER_SECTIONS='
   +.note.gnu.arm.ident 0 : { KEEP (*(.note.gnu.arm.ident)) }
   +  /* This section contains data that is not initialised during load
   +     *or* application reset.  */
   +   .noinit (NOLOAD) :
   +   {
   +     . = ALIGN(2);
   +     PROVIDE (__noinit_start = .);
   +     *(.noinit)
   +     . = ALIGN(2);
   +     PROVIDE (__noinit_end = .);
   +   }
   +'

so that the noinit section has the "NOLOAD" flag.

I'll submit that part separately to the binutils project if OK.

However, I'm not sure for which other targets (beyond arm), I should
update the linker scripts.

I left the new testcase in gcc.target/arm, guarded by
dg-do run { target { *-*-eabi } }
but since this attribute is now generic, I suspect I should move the
test to some other place. But then, which target selector should I
use?

Finally, I tested on arm-eabi, but not on msp430 for which I do not
have the environment, so advice from msp430 maintainers is
appreciated. Since msp430 does not use the same default helpers as
arm, I left the "noinit" handling code in place, to avoid changing
other generic functions which I don't know how to test
(default_select_section, default_section_type_flags).


Since the section attribute is mutually exclusive with noinit,
defining an attribute_exclusions array with the mutually exclusive
attributes and setting the last member of the corresponding
c_common_attribute_table element to that array (and updating
the arrays for the other mutually exclusive attributes) would
be the expected way to express that constraint:

+  { "noinit",                 0, 0, true,  false, false, false,
+                             handle_noinit_attribute, NULL },
                                                         ^^^^
This should also make it possible to remove the explicit handling
from handle_section_attribute and handle_noinit_attribute.  If
that's not completely possible then in the following please be
sure to quote the names of the attributes:

@@ -1912,6 +1915,13 @@ handle_section_attribute (tree *node, tree
ARG_UNUSED (name), tree args,
         goto fail;
       }

+  if (DECL_P (decl) && lookup_attribute ("noinit", DECL_ATTRIBUTES
(decl)) != NULL_TREE)
+    {
+      warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+               "section attribute cannot be applied to variables with noinit
attribute");

Martin


Thanks, here is an updated version:
- adds exclusion array
- updates testcase with new error message (because of exclusion)

The exclusion bits look good, thank you!

Martin

- moves testcase to gcc.c-torture/execute
- adds "noinit" effective-target

Christophe

Thanks,

Christophe

gcc/ChangeLog:

2019-07-04  Christophe Lyon  <christophe.lyon@linaro.org>

* doc/extend.texi: Add "noinit" attribute documentation.
* varasm.c
(default_section_type_flags): Add support for "noinit" section.
(default_elf_select_section): Add support for "noinit" attribute.

gcc/c-family/ChangeLog:

2019-07-04  Christophe Lyon  <christophe.lyon@linaro.org>

* c-attribs.c (c_common_attribute_table): Add "noinit" entry.
(handle_section_attribute): Add support for "noinit" attribute.
(handle_noinit_attribute): New function.

gcc/config/ChangeLog:

2019-07-04  Christophe Lyon  <christophe.lyon@linaro.org>

* msp430/msp430.c (msp430_attribute_table): Remove "noinit" entry.

gcc/testsuite/ChangeLog:

2019-07-04  Christophe Lyon  <christophe.lyon@linaro.org>

* gcc.target/arm/noinit-attribute.c: New test.




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]