This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add generic support for "noinit" attribute
- From: Christophe Lyon <christophe dot lyon at linaro dot org>
- To: Martin Sebor <msebor at gmail dot com>
- Cc: gcc Patches <gcc-patches at gcc dot gnu dot org>, Richard Earnshaw <richard dot earnshaw at arm dot com>, nick clifton <nickc at redhat dot com>
- Date: Tue, 16 Jul 2019 11:07:00 +0200
- Subject: Re: [PATCH] Add generic support for "noinit" attribute
- References: <CAKdteObaTtriRDdWTkzr66Yym0zapsYM=0meT10ihC4K9dmFJg@mail.gmail.com> <d6bc458d-298e-a1f1-31cb-ab48a6bd2280@gmail.com> <CAKdteObPa22MWCK-LpwR2nUFPX2upzawfZ8EUM5ce=rOk4Rftw@mail.gmail.com> <260defe9-dda1-a204-dad9-01bb0a5151a8@gmail.com>
Ping?
Le mar. 9 juil. 2019 à 00:04, Martin Sebor <msebor@gmail.com> a écrit :
> 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.
> >>>
> >>
>
>