RFC: ARM: Add comment enumerating emitted .eabi_attribute tags

RFC: ARM: Add comment enumerating emitted .eabi_attribute tags

[Nick Clifton]

Hi Richard, Hi Paul, Hi Ramana,

  I am planning to check in the patch below to add comments to the
  .eabi_attribute directives emitted by the ARM GCC backend.  So for
  example a simple hello world source file might compile to something
  like this:

        .cpu arm7tdmi
        .fpu softvfp
        .eabi_attribute 20, 1   @ Tag_ABI_FP_denormal
        .eabi_attribute 21, 1   @ Tag_ABI_FP_exceptions
        .eabi_attribute 23, 3   @ Tag_ABI_FP_number_model
   
  Apart from making the assembler output easier to understand, the patch
  also makes use of the enumerated tag values in the elf/arm.h header
  file, thus making sure that the compiler stays in sync with the
  header.

  I stuck with using numerical values in the actual .eabi_attribute
  directive as versions of GAS prior to 2.20 require this.
  
  Any comments or objections to this patch ?  If not, I will apply it
  next week.

Cheers
  Nick

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.

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);
 }
 
 

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

[Ramana Radhakrishnan]

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);
>  }
>
>
>
>

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

[Paul Brook]

> 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.

TARGET_AAPCS doesn't exist. If you mean TARGET_AAPCS_BASED then this is a 
runtime expression and can't be used for preprocessor conditionals.  Either 
way I don't think we really care.  The important thing is that we only emit 
the .eabi_attribute directives on EABI based targets.

A more important consideration is that this file does not exist.

Nick: Where are you expecting elf/arm.h come from?  I can't find any evidence 
this exists outside the binutils source tree, and that is not available when 
building gcc.  The BFD_ARCH_SIZE hack is a fair indication that it isn't 
intended to be used elsewhere.

Paul

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

[Ramana Radhakrishnan]

On 27 September 2011 16:55, Paul Brook <paul@codesourcery.com> wrote:
>> 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.
>
> TARGET_AAPCS doesn't exist. If you mean TARGET_AAPCS_BASED then this is a
> runtime expression and can't be used for preprocessor conditionals.  Either
> way I don't think we really care.  The important thing is that we only emit
> the .eabi_attribute directives on EABI based targets.

Bah yes - you are right.

>
> A more important consideration is that this file does not exist.

Amen.

Ramana

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

[Nick Clifton]

Hi Ramana,

>> +/* 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.

Hmm, there does not appear to be any compile-time define that I can 
reliably use for this.

> 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  ?

Err, probably not.  I forgot that the elf/ subdirectory of the include/ 
directory is not shared between gcc and binutils.

> 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  ?

This would be a good idea.

> 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.

Agreed.

I think that there are two possible ways to go:

  1. Create a new file include/arm-eabi-attrs.h to be shared between gcc 
and binutils and alter include/elf/arm.h to use this file.  The problem 
with this idea is that it adds a target specific file to the include/ 
directory, which really should be target agnostic.  (I know that there 
are some xtensa header files there, but IMHO that was a mistake).

   2. Abandon using a header file at all.  Instead use a configure test 
to see if we are using an assembler that supports textual names in a 
.eabi_attribute directive and if so use the names rather than the numbers.

Any preference ?

Cheers
   Nick

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

[Paul Brook]

>    2. Abandon using a header file at all.  Instead use a configure test
> to see if we are using an assembler that supports textual names in a
> .eabi_attribute directive and if so use the names rather than the numbers.

I'm not sure this is mutually exclusive with using a header file, but using 
the names where possible sounds like a nice idea in principle.

There are practical issues with tags introduced in newer EABI revisions (which 
may not be supported by older assemblers).  However you probably need linker 
support anyway and get the new assembler for free in that case.  I wouldn't 
put money on old binutils working reliably with new gcc in the first place.

Paul

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

[Richard Henderson]

On 09/27/2011 07:38 AM, Nick Clifton wrote:
> +#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);			\
> +    }									\

You should only emit the comment for -dA, I think.


r~

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

[Nick Clifton]

Hi Richard,

> You should only emit the comment for -dA, I think.

Really ?  I thought that it was kind of nice to have the comments in the 
default output.

If I go with -dA as the trigger then I assume that it is also OK to add 
the comments if --verbose-asm is enabled, yes ?

Cheers
   Nick