Ping? Le mar. 9 juil. 2019 à 00:04, Martin Sebor <mse...@gmail.com> a écrit :
> On 7/8/19 5:10 AM, Christophe Lyon wrote: > > 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) > > 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.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. > >>> > >> > >