On Sat, 6 Jul 2019 at 19:57, Martin Sebor <mse...@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)
- moves testcase to gcc.c-torture/execute
- adds "noinit" effective-target
Christophe
> > Thanks,
> >
> > Christophe
> >
> > gcc/ChangeLog:
> >
> > 2019-07-04 Christophe Lyon <christophe.l...@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.l...@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.l...@linaro.org>
> >
> > * msp430/msp430.c (msp430_attribute_table): Remove "noinit" entry.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2019-07-04 Christophe Lyon <christophe.l...@linaro.org>
> >
> > * gcc.target/arm/noinit-attribute.c: New test.
> >
>
commit 3a9dd81162eae36a87a067018a5e5bf1e7b978df
Author: Christophe Lyon <christophe.l...@linaro.org>
Date: Thu Jul 4 14:46:00 2019 +0000
Add generic support for noinit attribute.
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 added a testcase if gcc.c-torture/execute, gated by the new noinit
effective-target.
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.
Thanks,
Christophe
gcc/ChangeLog:
2019-07-04 Christophe Lyon <christophe.l...@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.l...@linaro.org>
* c-attribs.c (c_common_attribute_table): Add "noinit" entry. Add
exclusion with "section" attribute.
(attr_noinit_exclusions): New table.
(handle_noinit_attribute): New function.
gcc/config/ChangeLog:
2019-07-04 Christophe Lyon <christophe.l...@linaro.org>
* msp430/msp430.c (msp430_attribute_table): Remove "noinit" entry.
gcc/testsuite/ChangeLog:
2019-07-04 Christophe Lyon <christophe.l...@linaro.org>
* lib/target-supports.exp (check_effective_target_noinit): New
proc.
* gcc.c-torture/execute/noinit-attribute.c: New test.
Change-Id: Id8e0baa728d187e05541c7520bd5726ccf803c4f
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 48819e7..399fef3 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -92,6 +92,7 @@ static tree handle_section_attribute (tree *, tree, tree,
int, bool *);
static tree handle_aligned_attribute (tree *, tree, tree, int, bool *);
static tree handle_warn_if_not_aligned_attribute (tree *, tree, tree,
int, bool *);
+static tree handle_noinit_attribute (tree *, tree, tree, int, bool *);
static tree handle_weak_attribute (tree *, tree, tree, int, bool *) ;
static tree handle_noplt_attribute (tree *, tree, tree, int, bool *) ;
static tree handle_alias_ifunc_attribute (bool, tree *, tree, tree, bool *);
@@ -235,6 +236,13 @@ static const struct attribute_spec::exclusions
attr_const_pure_exclusions[] =
ATTR_EXCL (NULL, false, false, false)
};
+static const struct attribute_spec::exclusions attr_noinit_exclusions[] =
+{
+ ATTR_EXCL ("noinit", true, true, true),
+ ATTR_EXCL ("section", true, true, true),
+ ATTR_EXCL (NULL, false, false, false),
+};
+
/* Table of machine-independent attributes common to all C-like languages.
Current list of processed common attributes: nonnull. */
@@ -307,7 +315,7 @@ const struct attribute_spec c_common_attribute_table[] =
{ "mode", 1, 1, false, true, false, false,
handle_mode_attribute, NULL },
{ "section", 1, 1, true, false, false, false,
- handle_section_attribute, NULL },
+ handle_section_attribute, attr_noinit_exclusions
},
{ "aligned", 0, 1, false, false, false, false,
handle_aligned_attribute,
attr_aligned_exclusions },
@@ -458,6 +466,8 @@ const struct attribute_spec c_common_attribute_table[] =
handle_nocf_check_attribute, NULL },
{ "copy", 1, 1, false, false, false, false,
handle_copy_attribute, NULL },
+ { "noinit", 0, 0, true, false, false, false,
+ handle_noinit_attribute, attr_noinit_exclusions },
{ NULL, 0, 0, false, false, false, false, NULL, NULL }
};
@@ -2224,6 +2234,50 @@ handle_weak_attribute (tree *node, tree name,
return NULL_TREE;
}
+/* Handle a "noinit" attribute; arguments as in struct
+ attribute_spec.handler. Check whether the attribute is allowed
+ here and add the attribute to the variable decl tree or otherwise
+ issue a diagnostic. This function checks NODE is of the expected
+ type and issues diagnostics otherwise using NAME. If it is not of
+ the expected type *NO_ADD_ATTRS will be set to true. */
+
+static tree
+handle_noinit_attribute (tree * node,
+ tree name,
+ tree args,
+ int flags ATTRIBUTE_UNUSED,
+ bool *no_add_attrs)
+{
+ const char *message = NULL;
+
+ gcc_assert (DECL_P (*node));
+ gcc_assert (args == NULL);
+
+ if (TREE_CODE (*node) != VAR_DECL)
+ message = G_("%qE attribute only applies to variables");
+
+ /* Check that it's possible for the variable to have a section. */
+ if ((TREE_STATIC (*node) || DECL_EXTERNAL (*node) || in_lto_p)
+ && DECL_SECTION_NAME (*node))
+ message = G_("%qE attribute cannot be applied to variables "
+ "with specific sections");
+
+ /* If this var is thought to be common, then change this. Common
+ variables are assigned to sections before the backend has a
+ chance to process them. */
+ if (DECL_COMMON (*node))
+ DECL_COMMON (*node) = 0;
+
+ if (message)
+ {
+ warning (OPT_Wattributes, message, name);
+ *no_add_attrs = true;
+ }
+
+ return NULL_TREE;
+}
+
+
/* Handle a "noplt" attribute; arguments as in
struct attribute_spec.handler. */
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 365e9eb..264e852 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -2108,8 +2108,6 @@ const struct attribute_spec msp430_attribute_table[] =
{ ATTR_EITHER, 0, 0, true, false, false, false, msp430_section_attr,
NULL },
- { ATTR_NOINIT, 0, 0, true, false, false, false, msp430_data_attr,
- NULL },
{ ATTR_PERSIST, 0, 0, true, false, false, false, msp430_data_attr,
NULL },
@@ -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))
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index f2619e1..850153e 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7129,6 +7129,12 @@ The @code{visibility} attribute is described in
The @code{weak} attribute is described in
@ref{Common Function Attributes}.
+@item noinit
+@cindex @code{noinit} variable attribute
+Any data with the @code{noinit} attribute will not be initialized by
+the C runtime startup code, or the program loader. Not initializing
+data in this way can reduce program startup times.
+
@end table
@node ARC Variable Attributes
diff --git a/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c
b/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c
new file mode 100644
index 0000000..f33c0d0
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c
@@ -0,0 +1,59 @@
+/* { dg-do run } */
+/* { dg-require-effective-target noinit */
+/* { dg-options "-O2" } */
+
+/* This test checks that noinit data is handled correctly. */
+
+extern void _start (void) __attribute__ ((noreturn));
+extern void abort (void) __attribute__ ((noreturn));
+extern void exit (int) __attribute__ ((noreturn));
+
+int var_common;
+int var_zero = 0;
+int var_one = 1;
+int __attribute__((noinit)) var_noinit;
+int var_init = 2;
+
+int __attribute__((noinit)) func(); /* { dg-warning "attribute only applies to
variables" } */
+int __attribute__((section ("mysection"), noinit)) var_section1; /* {
dg-warning "because it conflicts with attribute" } */
+int __attribute__((noinit, section ("mysection"))) var_section2; /* {
dg-warning "because it conflicts with attribute" } */
+
+
+int
+main (void)
+{
+ /* Make sure that the C startup code has correctly initialised the ordinary
variables. */
+ if (var_common != 0)
+ abort ();
+
+ /* Initialised variables are not re-initialised during startup, so
+ check their original values only during the first run of this
+ test. */
+ if (var_init == 2)
+ if (var_zero != 0 || var_one != 1)
+ abort ();
+
+ switch (var_init)
+ {
+ case 2:
+ /* First time through - change all the values. */
+ var_common = var_zero = var_one = var_noinit = var_init = 3;
+ break;
+
+ case 3:
+ /* Second time through - make sure that d has not been reset. */
+ if (var_noinit != 3)
+ abort ();
+ exit (0);
+
+ default:
+ /* Any other value for var_init is an error. */
+ abort ();
+ }
+
+ /* Simulate a processor reset by calling the C startup code. */
+ _start ();
+
+ /* Should never reach here. */
+ abort ();
+}
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index 815e837..ae05c0a 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -364,6 +364,18 @@ proc check_weak_override_available { } {
return [check_weak_available]
}
+# The noinit attribute is only supported by some targets.
+# This proc returns 1 if it's supported, 0 if it's not.
+
+proc check_effective_target_noinit { } {
+ if { [istarget arm*-*-eabi]
+ || [istarget msp430-*-*] } {
+ return 1
+ }
+
+ return 0
+}
+
###############################
# proc check_visibility_available { what_kind }
###############################
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 626a4c9..7740e88 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6428,6 +6428,9 @@ default_section_type_flags (tree decl, const char *name,
int reloc)
|| strncmp (name, ".gnu.linkonce.tb.", 17) == 0)
flags |= SECTION_TLS | SECTION_BSS;
+ if (strcmp (name, ".noinit") == 0)
+ flags |= SECTION_WRITE | SECTION_BSS | SECTION_NOTYPE;
+
/* Various sections have special ELF types that the assembler will
assign by default based on the name. They are neither SHT_PROGBITS
nor SHT_NOBITS, so when changing sections we don't want to print a
@@ -6748,11 +6751,14 @@ decl_readonly_section (const_tree decl, int reloc)
/* Select a section based on the above categorization. */
+static section *noinit_section = NULL;
+
section *
default_elf_select_section (tree decl, int reloc,
unsigned HOST_WIDE_INT align)
{
const char *sname;
+
switch (categorize_decl_for_section (decl, reloc))
{
case SECCAT_TEXT:
@@ -6790,6 +6796,14 @@ default_elf_select_section (tree decl, int reloc,
sname = ".tdata";
break;
case SECCAT_BSS:
+ if (DECL_P (decl)
+ && lookup_attribute ("noinit", DECL_ATTRIBUTES (decl)) != NULL_TREE)
+ {
+ if (noinit_section == NULL)
+ noinit_section = get_named_section (decl, ".noinit", reloc);
+ return noinit_section;
+ }
+
if (bss_section)
return bss_section;
sname = ".bss";
