[PATCH][ARM] Add support for "noinit" attribute

Richard Earnshaw Richard.Earnshaw@arm.com
Tue Jul 2 10:30:00 GMT 2019 


On 02/07/2019 11:13, Richard Earnshaw wrote:
> 
> 
> On 02/07/2019 09:39, Richard Earnshaw wrote:
>>
>>
>> On 01/07/2019 16:58, Kyrill Tkachov wrote:
>>> Hi Christophe,
>>>
>>> On 6/13/19 4:13 PM, Christophe Lyon wrote:
>>>> Hi,
>>>>
>>>> Similar to what already exists for TI msp430 or in TI compilers for
>>>> arm, this patch adds support for "noinit" attribute for arm. It's very
>>>> similar to the corresponding code in GCC for msp430.
>>>>
>>>> It is useful 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.
>>>>
>>>> OK?
>>>>
>>>
>>> This is mostly ok by me, with a few code comments inline.
>>>
>>> I wonder whether this is something we could implement for all targets 
>>> in the midend, but this would require linker script support for the 
>>> target to be effective...
>>
>> Can't this be done using named sections?  If the sections were of the 
>> form .bss.<foo> then it would be easy to make linker scripts do 
>> something sane by default and users could filter them out to special 
>> noinit sections if desired.
>>
> 
> To answer my own question, it would appear to be yes.  You can write today:
> 
> int xxx __attribute__ ((section (".bss.noinit")));
> 
> int main ()
> {
>    return xxx;
> }
> 
> And the compiler will generate
>      .section    .bss.noinit,"aw",@nobits
>      .align 4
>      .type    xxx, @object
>      .size    xxx, 4
> xxx:
>      .zero    4
> 
> So at this point, all you need is a linker script to filter .bss.noinit 
> into your special part of the final image.
> 
> This will most likely work today on any GCC target that supports named 
> sections, which is pretty much all of them these days.
> 

Alternatively, we already have:

https://gcc.gnu.org/ml/gcc-patches/2014-09/msg01302.html

R.

> R.
> 
>> R.
>>
>>>
>>> Thanks,
>>>
>>> Kyrill
>>>
>>>> Thanks,
>>>>
>>>> Christophe
>>>
>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>> index e3e71ea..332c41b 100644
>>> --- a/gcc/config/arm/arm.c
>>> +++ b/gcc/config/arm/arm.c
>>> @@ -151,6 +151,7 @@ static tree arm_handle_notshared_attribute (tree 
>>> *, tree, tree, int, bool *);
>>>   #endif
>>>   static tree arm_handle_cmse_nonsecure_entry (tree *, tree, tree, 
>>> int, bool *);
>>>   static tree arm_handle_cmse_nonsecure_call (tree *, tree, tree, 
>>> int, bool *);
>>> +static tree arm_data_attr (tree *, tree, tree, int, bool *);
>>>   static void arm_output_function_epilogue (FILE *);
>>>   static void arm_output_function_prologue (FILE *);
>>>   static int arm_comp_type_attributes (const_tree, const_tree);
>>> @@ -375,7 +376,8 @@ static const struct attribute_spec 
>>> arm_attribute_table[] =
>>>       arm_handle_cmse_nonsecure_entry, NULL },
>>>     { "cmse_nonsecure_call", 0, 0, true, false, false, true,
>>>       arm_handle_cmse_nonsecure_call, NULL },
>>> -  { NULL, 0, 0, false, false, false, false, NULL, NULL }
>>> +  { "noinit", 0, 0, true, false, false, false, arm_data_attr, NULL },
>>> +  { NULL, 0, 0, false, false, false, false, NULL, NULL },
>>>   };
>>>
>>>   /* Initialize the GCC target structure.  */
>>> @@ -808,6 +810,10 @@ static const struct attribute_spec 
>>> arm_attribute_table[] =
>>>
>>>   #undef TARGET_CONSTANT_ALIGNMENT
>>>   #define TARGET_CONSTANT_ALIGNMENT arm_constant_alignment
>>> +
>>> +#undef  TARGET_ASM_SELECT_SECTION
>>> +#define TARGET_ASM_SELECT_SECTION arm_select_section
>>> +
>>>
>>>   /* Obstack for minipool constant handling.  */
>>>   static struct obstack minipool_obstack;
>>> @@ -7150,6 +7156,47 @@ arm_handle_cmse_nonsecure_call (tree *node, 
>>> tree name,
>>>     return NULL_TREE;
>>>   }
>>>
>>> +/* Called when the noinit attribute is used. 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
>>> +arm_data_attr (tree * node,
>>> +          tree   name,
>>> +          tree   args ATTRIBUTE_UNUSED,
>>> +          int    flags ATTRIBUTE_UNUSED,
>>> +          bool * no_add_attrs ATTRIBUTE_UNUSED)
>>> +{
>>>
>>> no_add_attrs is set in this function, so should not be ATTRIBUTE_UNUSED?
>>> Arguably args is also checked against NULL, so it's technically not 
>>> unused either.
>>>
>>> +  const char * message = NULL;
>>> +
>>> +  gcc_assert (DECL_P (* node));
>>>
>>> The house style doesn't have a space after '*'. Same elsewhere in the 
>>> patch.
>>>
>>> +  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;
>>> +}
>>> +
>>>   /* Return 0 if the attributes for two types are incompatible, 1 if 
>>> they
>>>      are compatible, and 2 if they are nearly compatible (which causes a
>>>      warning to be generated).  */
>>> @@ -27890,6 +27937,8 @@ arm_asm_emit_except_personality (rtx 
>>> personality)
>>>
>>>   /* Implement TARGET_ASM_INITIALIZE_SECTIONS.  */
>>>
>>> +static section *noinit_section;
>>> +
>>>   static void
>>>   arm_asm_init_sections (void)
>>>   {
>>> @@ -27902,6 +27951,19 @@ arm_asm_init_sections (void)
>>>     if (target_pure_code)
>>>       text_section->unnamed.data = "\t.section 
>>> .text,\"0x20000006\",%progbits";
>>>   #endif
>>> +
>>> +  noinit_section = get_unnamed_section (0, output_section_asm_op, 
>>> ".section .noinit,\"aw\"");
>>> +}
>>> +
>>> +static section *
>>> +arm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
>>> +{
>>>
>>> Please add a function comment.
>>>
>>> +  gcc_assert (decl != NULL_TREE);
>>> +
>>> +  if (DECL_P (decl) && lookup_attribute ("noinit", DECL_ATTRIBUTES 
>>> (decl)) != NULL_TREE)
>>> +    return noinit_section;
>>> +  else
>>> +    return default_elf_select_section (decl, reloc, align);
>>>   }
>>>
>>>   /* Output unwind directives for the start/end of a function.  */
>>> @@ -31520,6 +31582,9 @@ arm_elf_section_type_flags (tree decl, const 
>>> char *name, int reloc)
>>>     if (decl && TREE_CODE (decl) == FUNCTION_DECL && target_pure_code)
>>>       flags |= SECTION_ARM_PURECODE;
>>>
>>> +  if (strcmp (name, ".noinit") == 0)
>>> +    flags = SECTION_WRITE | SECTION_BSS | SECTION_NOTYPE;
>>> +
>>>
>>>
>>> You're overwriting the flags here. Are you sure you don't want "flags 
>>> |= ..." here?
>>>
>>>
>>>   return flags;
>>>   }
>>>
>>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>>> index 2520835..d544527 100644
>>> --- a/gcc/doc/extend.texi
>>> +++ b/gcc/doc/extend.texi
>>> @@ -6684,6 +6684,7 @@ attributes.
>>>   @menu
>>>   * Common Variable Attributes::
>>>   * ARC Variable Attributes::
>>> +* ARM Variable Attributes::
>>>   * AVR Variable Attributes::
>>>   * Blackfin Variable Attributes::
>>>   * H8/300 Variable Attributes::
>>> @@ -7131,6 +7132,18 @@ given via attribute argument.
>>>
>>>   @end table
>>>
>>> +@node ARM Variable Attributes
>>> +@subsection ARM Variable Attributes
>>> +
>>> +@table @code
>>> +@item noinit
>>> +@cindex @code{noinit} variable attribute, ARM
>>> +Any data with the @code{noinit} attribute will not be initialised by
>>> +the C runtime startup code, or the program loader.  Not initialising
>>> +data in this way can reduce program startup times.
>>> +
>>> +@end table
>>> +
>>>   @node AVR Variable Attributes
>>>   @subsection AVR Variable Attributes
>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/data-attributes.c 
>>> b/gcc/testsuite/gcc.target/arm/data-attributes.c
>>> new file mode 100644
>>> index 0000000..323c8e0
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/data-attributes.c
>>> @@ -0,0 +1,51 @@
>>> +/* { dg-do run { target { ! *-*-linux* } } } */
>>> +/* { 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
>>> +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 ();
>>> +}
>>>

More information about the Gcc-patches mailing list