[Patch AArch64] Use default_elf_asm_named_section instead of special cased hook

[Patch AArch64] Use default_elf_asm_named_section instead of special cased hook

[Ramana Radhakrishnan]

Hi,

The hook for TARGET_ASM_NAMED_SECTION was defined separately in
the backend around the time frame for GCC 4.7 under the assumption
that '@' would be used as a comment character in the binutils port.

This was indeed true in versions of the AArch64 port that never made it
into the FSF tree. However this practice was killed
before the binutils port made it upstream. Doing archaeology into
various revisions including the first commit of upstream binutils, 
talking to Marcus about the history of this and actually testing the
first commit of binutils, I can confidently say that the upstream binutils
port never had any use of '@' as a comment character for AArch64
However we never got rid of the special cased handling in
GCC and the duplication of code in the AArch64 backend.

This was found when I was playing with Virtual Table verification
on ARM and AArch64 and discovered ICEs which were similar but manifested
in different places begging the obvious question.

Tested on aarch64-none-elf with no regressions.

Ok to apply ?

Ramana

	* config/aarch64/aarch64-elf.h (TARGET_ASM_NAMED_SECTION): Use
	default_elf_asm_named_section.
	* config/aarch64/aarch64.c (aarch64_elf_asm_named_section): Delete.
---
 gcc/config/aarch64/aarch64-elf.h |  2 +-
 gcc/config/aarch64/aarch64.c     | 74 ----------------------------------------
 2 files changed, 1 insertion(+), 75 deletions(-)

diff --git a/gcc/config/aarch64/aarch64-elf.h b/gcc/config/aarch64/aarch64-elf.h
index 1ce6343..70aa845 100644
--- a/gcc/config/aarch64/aarch64-elf.h
+++ b/gcc/config/aarch64/aarch64-elf.h
@@ -154,7 +154,7 @@ ASM_MABI_SPEC
 #define TYPE_OPERAND_FMT	"%%%s"
 
 #undef TARGET_ASM_NAMED_SECTION
-#define TARGET_ASM_NAMED_SECTION  aarch64_elf_asm_named_section
+#define TARGET_ASM_NAMED_SECTION  default_elf_asm_named_section
 
 /* Stabs debug not required.  */
 #undef DBX_DEBUGGING_INFO
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 4d2126b..0ead7c0 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10784,80 +10784,6 @@ aarch64_shift_truncation_mask (machine_mode mode)
      || aarch64_vect_struct_mode_p (mode)) ? 0 : (GET_MODE_BITSIZE (mode) - 1);
 }
 
-#ifndef TLS_SECTION_ASM_FLAG
-#define TLS_SECTION_ASM_FLAG 'T'
-#endif
-
-void
-aarch64_elf_asm_named_section (const char *name, unsigned int flags,
-			       tree decl ATTRIBUTE_UNUSED)
-{
-  char flagchars[10], *f = flagchars;
-
-  /* If we have already declared this section, we can use an
-     abbreviated form to switch back to it -- unless this section is
-     part of a COMDAT groups, in which case GAS requires the full
-     declaration every time.  */
-  if (!(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
-      && (flags & SECTION_DECLARED))
-    {
-      fprintf (asm_out_file, "\t.section\t%s\n", name);
-      return;
-    }
-
-  if (!(flags & SECTION_DEBUG))
-    *f++ = 'a';
-  if (flags & SECTION_WRITE)
-    *f++ = 'w';
-  if (flags & SECTION_CODE)
-    *f++ = 'x';
-  if (flags & SECTION_SMALL)
-    *f++ = 's';
-  if (flags & SECTION_MERGE)
-    *f++ = 'M';
-  if (flags & SECTION_STRINGS)
-    *f++ = 'S';
-  if (flags & SECTION_TLS)
-    *f++ = TLS_SECTION_ASM_FLAG;
-  if (HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
-    *f++ = 'G';
-  *f = '\0';
-
-  fprintf (asm_out_file, "\t.section\t%s,\"%s\"", name, flagchars);
-
-  if (!(flags & SECTION_NOTYPE))
-    {
-      const char *type;
-      const char *format;
-
-      if (flags & SECTION_BSS)
-	type = "nobits";
-      else
-	type = "progbits";
-
-#ifdef TYPE_OPERAND_FMT
-      format = "," TYPE_OPERAND_FMT;
-#else
-      format = ",@%s";
-#endif
-
-      fprintf (asm_out_file, format, type);
-
-      if (flags & SECTION_ENTSIZE)
-	fprintf (asm_out_file, ",%d", flags & SECTION_ENTSIZE);
-      if (HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
-	{
-	  if (TREE_CODE (decl) == IDENTIFIER_NODE)
-	    fprintf (asm_out_file, ",%s,comdat", IDENTIFIER_POINTER (decl));
-	  else
-	    fprintf (asm_out_file, ",%s,comdat",
-		     IDENTIFIER_POINTER (DECL_COMDAT_GROUP (decl)));
-	}
-    }
-
-  putc ('\n', asm_out_file);
-}
-
 /* Select a format to encode pointers in exception handling data.  */
 int
 aarch64_asm_preferred_eh_data_format (int code ATTRIBUTE_UNUSED, int global)
-- 
1.9.1

Re: [Patch AArch64] Use default_elf_asm_named_section instead of special cased hook

[Marcus Shawcroft]

On 2 October 2015 at 11:08, Ramana Radhakrishnan
<ramana.radhakrishnan@arm.com> wrote:

>         * config/aarch64/aarch64-elf.h (TARGET_ASM_NAMED_SECTION): Use
>         default_elf_asm_named_section.
>         * config/aarch64/aarch64.c (aarch64_elf_asm_named_section): Delete.
> ---
>  gcc/config/aarch64/aarch64-elf.h |  2 +-
>  gcc/config/aarch64/aarch64.c     | 74 ----------------------------------------
>  2 files changed, 1 insertion(+), 75 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64-elf.h b/gcc/config/aarch64/aarch64-elf.h
> index 1ce6343..70aa845 100644
> --- a/gcc/config/aarch64/aarch64-elf.h
> +++ b/gcc/config/aarch64/aarch64-elf.h
> @@ -154,7 +154,7 @@ ASM_MABI_SPEC
>  #define TYPE_OPERAND_FMT       "%%%s"
>
>  #undef TARGET_ASM_NAMED_SECTION
> -#define TARGET_ASM_NAMED_SECTION  aarch64_elf_asm_named_section
> +#define TARGET_ASM_NAMED_SECTION  default_elf_asm_named_section

Isn't it sufficient to simply remove the #define completely and rely
on the default from elfos.h?
/M

Re: [Patch AArch64] Use default_elf_asm_named_section instead of special cased hook

[Ramana Radhakrishnan]

On 02/10/15 13:46, Marcus Shawcroft wrote:
> On 2 October 2015 at 11:08, Ramana Radhakrishnan
> <ramana.radhakrishnan@arm.com> wrote:
> 
>>         * config/aarch64/aarch64-elf.h (TARGET_ASM_NAMED_SECTION): Use
>>         default_elf_asm_named_section.
>>         * config/aarch64/aarch64.c (aarch64_elf_asm_named_section): Delete.
>> ---
>>  gcc/config/aarch64/aarch64-elf.h |  2 +-
>>  gcc/config/aarch64/aarch64.c     | 74 ----------------------------------------
>>  2 files changed, 1 insertion(+), 75 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64-elf.h b/gcc/config/aarch64/aarch64-elf.h
>> index 1ce6343..70aa845 100644
>> --- a/gcc/config/aarch64/aarch64-elf.h
>> +++ b/gcc/config/aarch64/aarch64-elf.h
>> @@ -154,7 +154,7 @@ ASM_MABI_SPEC
>>  #define TYPE_OPERAND_FMT       "%%%s"
>>
>>  #undef TARGET_ASM_NAMED_SECTION
>> -#define TARGET_ASM_NAMED_SECTION  aarch64_elf_asm_named_section
>> +#define TARGET_ASM_NAMED_SECTION  default_elf_asm_named_section
> 
> Isn't it sufficient to simply remove the #define completely and rely
> on the default from elfos.h?

Doh ! you're right - Yeah should be coming in from config/elfos.h given that we don't support anything other than elf in the aarch64 port.

Ok to commit without that hunk ?

regards
Ramana

Re: [Patch AArch64] Use default_elf_asm_named_section instead of special cased hook

[Marcus Shawcroft]

On 2 October 2015 at 14:01, Ramana Radhakrishnan
<ramana.radhakrishnan@foss.arm.com> wrote:

>>>  #undef TARGET_ASM_NAMED_SECTION
>>> -#define TARGET_ASM_NAMED_SECTION  aarch64_elf_asm_named_section
>>> +#define TARGET_ASM_NAMED_SECTION  default_elf_asm_named_section
>>
>> Isn't it sufficient to simply remove the #define completely and rely
>> on the default from elfos.h?
>
> Doh ! you're right - Yeah should be coming in from config/elfos.h given that we don't support anything other than elf in the aarch64 port.
>
> Ok to commit without that hunk ?

Yes. /Marcus

Re: [Patch AArch64] Use default_elf_asm_named_section instead of special cased hook

[Christophe Lyon]

On 2 October 2015 at 15:05, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote:
> On 2 October 2015 at 14:01, Ramana Radhakrishnan
> <ramana.radhakrishnan@foss.arm.com> wrote:
>
>>>>  #undef TARGET_ASM_NAMED_SECTION
>>>> -#define TARGET_ASM_NAMED_SECTION  aarch64_elf_asm_named_section
>>>> +#define TARGET_ASM_NAMED_SECTION  default_elf_asm_named_section
>>>
>>> Isn't it sufficient to simply remove the #define completely and rely
>>> on the default from elfos.h?
>>
>> Doh ! you're right - Yeah should be coming in from config/elfos.h given that we don't support anything other than elf in the aarch64 port.
>>
>> Ok to commit without that hunk ?
>
> Yes. /Marcus

Hi Ramana,

Since this commit, I am seeing build failures when creating lto1:
libbackend.a(aarch64.o):(.data+0x118): undefined reference to
`aarch64_elf_asm_named_section(char const*, unsigned int, tree_node*)'
(targets aarch64_none-linux-gnu and aarch64-none-elf).

I guess I'm missing something obvious?

Christophe.

Re: [Patch AArch64] Use default_elf_asm_named_section instead of special cased hook

[Ramana Radhakrishnan]

On Fri, Oct 2, 2015 at 5:40 PM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 2 October 2015 at 15:05, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote:
>> On 2 October 2015 at 14:01, Ramana Radhakrishnan
>> <ramana.radhakrishnan@foss.arm.com> wrote:
>>
>>>>>  #undef TARGET_ASM_NAMED_SECTION
>>>>> -#define TARGET_ASM_NAMED_SECTION  aarch64_elf_asm_named_section
>>>>> +#define TARGET_ASM_NAMED_SECTION  default_elf_asm_named_section
>>>>
>>>> Isn't it sufficient to simply remove the #define completely and rely
>>>> on the default from elfos.h?
>>>
>>> Doh ! you're right - Yeah should be coming in from config/elfos.h given that we don't support anything other than elf in the aarch64 port.
>>>
>>> Ok to commit without that hunk ?
>>
>> Yes. /Marcus
>
> Hi Ramana,
>
> Since this commit, I am seeing build failures when creating lto1:
> libbackend.a(aarch64.o):(.data+0x118): undefined reference to
> `aarch64_elf_asm_named_section(char const*, unsigned int, tree_node*)'
> (targets aarch64_none-linux-gnu and aarch64-none-elf).
>
> I guess I'm missing something obvious?

I missed a hunk which was fixed up in
https://gcc.gnu.org/ml/gcc-cvs/2015-10/msg00086.html. I don't know why
the email for that hasn't made it to the lists yet. Sorry about the
breakage.

regards
Ramana

>
> Christophe.

Re: [Patch AArch64] Use default_elf_asm_named_section instead of special cased hook

[Andreas Schwab]

	* config/aarch64/aarch64-protos.h (aarch64_elf_asm_named_section):
	Remove declaration.

diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index e8c2ac8..3cdd69b 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -371,7 +371,6 @@ unsigned aarch64_dbx_register_number (unsigned);
 unsigned aarch64_trampoline_size (void);
 void aarch64_asm_output_labelref (FILE *, const char *);
 void aarch64_cpu_cpp_builtins (cpp_reader *);
-void aarch64_elf_asm_named_section (const char *, unsigned, tree);
 const char * aarch64_gen_far_branch (rtx *, int, const char *, const char *);
 const char * aarch64_output_probe_stack_range (rtx, rtx);
 void aarch64_err_no_fpadvsimd (machine_mode, const char *);
-- 
2.9.0


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."