Re: RFC: ARM: Add comment enumerating emitted .eabi_attribute tags

[Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org>]

Hi Nick,

On 27 September 2011 15:38, Nick Clifton <nickc@redhat.com> wrote:
>  Any comments or objections to this patch ?  If not, I will apply it
>  next week.

I've got a few objections to this patch as it stands today while I
don't object to the motivation for it.

> +/* Get the definitions of the ARM EABI Attribute tag values.  */
> +#define BFD_ARCH_SIZE
> +#include "elf/arm.h"

Defining BFD_ARCH_SIZE appears to be a bit of a hack. I would also
ifdef this inclusion on TARGET_AAPCS since we shouldn't really be
caring about object attributes for non AAPCS configurations.

Does elf/arm.h get installed by bfd / binutils in some form as a part
of it's build process ? Can the compiler find elf/arm.h by default for
a non-combined-tree arm-linux-gnueabi / arm-eabi cross-build or a
native boostrap  ? (I suspect it might just about manage in a combined
build in some form) . Expecting to add an additional include path for
a normal build would leave a few people grumpy.

Instead may I suggest rejigging this in a form so that we share this
header file between gcc and binutils and make sure that this is
sufficiently documented that one master copy lives in one repository
while the other is a copy of this is in the other repository  ?  It
has the same problems that we have with sharing other files between
these 2 repositories but causing pain to every single script out there
to add an include path to a default build sounds wrong to me.

>
> gcc/ChangeLog
> 2011-09-27  Nick Clifton  <nickc@redhat.com>
>
>        * config/arm/arm.c (EMIT_EABI_ATTRIBUTE): New macro.
>        (arm_file_start): Use it to generate .eabi_attribute assembler
>        pseudo-ops.

The hunk for arm-c.c isn't documented in the Changelog.

So, sorry I'm not happy as it stands today.

cheers
Ramana

>
> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c        (revision 179269)
> +++ gcc/config/arm/arm.c        (working copy)
> @@ -22220,6 +22220,24 @@
>     asm_fprintf (stream, "%U%s", name);
>  }
>
> +/* Get the definitions of the ARM EABI Attribute tag values.  */
> +#define BFD_ARCH_SIZE
> +#include "elf/arm.h"



> +
> +/* This macro is used to emit an EABI tag and its associated value.
> +   We emit the numerical value of the tag in case the assembler does not
> +   support textual tags.  (Eg gas prior to 2.20).  We include the tag
> +   name in a comment so that anyone reading the assembler output will
> +   know which tag is being set.  */
> +#define EMIT_EABI_ATTRIBUTE(name,val)                                  \
> +  do                                                                   \
> +    {                                                                  \
> +      asm_fprintf (asm_out_file, "\t.eabi_attribute %d, %d\t%s " #name "\n", \
> +                  name, val, ASM_COMMENT_START);                       \
> +    }                                                                  \
> +  while (0)
> +
> +
>  static void
>  arm_file_start (void)
>  {
> @@ -22248,12 +22266,13 @@
>       else
>        {
>          fpu_name = arm_fpu_desc->name;
> +
>          if (arm_fpu_desc->model == ARM_FP_MODEL_VFP)
>            {
>              if (TARGET_HARD_FLOAT)
> -               asm_fprintf (asm_out_file, "\t.eabi_attribute 27, 3\n");
> +               EMIT_EABI_ATTRIBUTE (Tag_ABI_HardFP_use, 3);
>              if (TARGET_HARD_FLOAT_ABI)
> -               asm_fprintf (asm_out_file, "\t.eabi_attribute 28, 1\n");
> +               EMIT_EABI_ATTRIBUTE (Tag_ABI_VFP_args, 1);
>            }
>        }
>       asm_fprintf (asm_out_file, "\t.fpu %s\n", fpu_name);
> @@ -22262,31 +22281,25 @@
>          are used.  However we don't have any easy way of figuring this out.
>         Conservatively record the setting that would have been used.  */
>
> -      /* Tag_ABI_FP_rounding.  */
>       if (flag_rounding_math)
> -       asm_fprintf (asm_out_file, "\t.eabi_attribute 19, 1\n");
> +       EMIT_EABI_ATTRIBUTE (Tag_ABI_FP_rounding, 1);
> +
>       if (!flag_unsafe_math_optimizations)
>        {
> -         /* Tag_ABI_FP_denomal.  */
> -         asm_fprintf (asm_out_file, "\t.eabi_attribute 20, 1\n");
> -         /* Tag_ABI_FP_exceptions.  */
> -         asm_fprintf (asm_out_file, "\t.eabi_attribute 21, 1\n");
> +         EMIT_EABI_ATTRIBUTE (Tag_ABI_FP_denormal, 1);
> +         EMIT_EABI_ATTRIBUTE (Tag_ABI_FP_exceptions, 1);
>        }
> -      /* Tag_ABI_FP_user_exceptions.  */
> +
>       if (flag_signaling_nans)
> -       asm_fprintf (asm_out_file, "\t.eabi_attribute 22, 1\n");
> -      /* Tag_ABI_FP_number_model.  */
> -      asm_fprintf (asm_out_file, "\t.eabi_attribute 23, %d\n",
> -                  flag_finite_math_only ? 1 : 3);
> +       EMIT_EABI_ATTRIBUTE (Tag_ABI_FP_user_exceptions, 1);
>
> -      /* Tag_ABI_align8_needed.  */
> -      asm_fprintf (asm_out_file, "\t.eabi_attribute 24, 1\n");
> -      /* Tag_ABI_align8_preserved.  */
> -      asm_fprintf (asm_out_file, "\t.eabi_attribute 25, 1\n");
> -      /* Tag_ABI_enum_size.  */
> -      asm_fprintf (asm_out_file, "\t.eabi_attribute 26, %d\n",
> -                  flag_short_enums ? 1 : 2);
> +      EMIT_EABI_ATTRIBUTE (Tag_ABI_FP_number_model,
> +                          flag_finite_math_only ? 1 : 3);
>
> +      EMIT_EABI_ATTRIBUTE (Tag_ABI_align8_needed, 1);
> +      EMIT_EABI_ATTRIBUTE (Tag_ABI_align8_preserved, 1);
> +      EMIT_EABI_ATTRIBUTE (Tag_ABI_enum_size, flag_short_enums ? 1 : 2);
> +
>       /* Tag_ABI_optimization_goals.  */
>       if (optimize_size)
>        val = 4;
> @@ -22296,21 +22309,18 @@
>        val = 1;
>       else
>        val = 6;
> -      asm_fprintf (asm_out_file, "\t.eabi_attribute 30, %d\n", val);
> +      EMIT_EABI_ATTRIBUTE (Tag_ABI_optimization_goals, val);
>
> -      /* Tag_CPU_unaligned_access.  */
> -      asm_fprintf (asm_out_file, "\t.eabi_attribute 34, %d\n",
> -                  unaligned_access);
> +      EMIT_EABI_ATTRIBUTE (Tag_CPU_unaligned_access, unaligned_access);
>
> -      /* Tag_ABI_FP_16bit_format.  */
>       if (arm_fp16_format)
> -       asm_fprintf (asm_out_file, "\t.eabi_attribute 38, %d\n",
> -                    (int)arm_fp16_format);
> +       EMIT_EABI_ATTRIBUTE (Tag_ABI_FP_16bit_format, (int) arm_fp16_format);
>
>       if (arm_lang_output_object_attributes_hook)
>        arm_lang_output_object_attributes_hook();
>     }
> -  default_file_start();
> +
> +  default_file_start ();
>  }
>
>  static void
> Index: gcc/config/arm/arm-c.c
> ===================================================================
> --- gcc/config/arm/arm-c.c      (revision 179247)
> +++ gcc/config/arm/arm-c.c      (working copy)
> @@ -32,8 +32,9 @@
>  static void arm_output_c_attributes(void)
>  {
>   /* Tag_ABI_PCS_wchar_t.  */
> -  asm_fprintf (asm_out_file, "\t.eabi_attribute 18, %d\n",
> -              (int)(TYPE_PRECISION (wchar_type_node) / BITS_PER_UNIT));
> +  asm_fprintf (asm_out_file, "\t.eabi_attribute 18, %d\t%s Tag_ABI_PCS_wchar_t\n",
> +              (int)(TYPE_PRECISION (wchar_type_node) / BITS_PER_UNIT),
> +              ASM_COMMENT_START);
>  }
>
>
>
>