This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PATCH] Don't use .gnu.linkonce with COMDAT

From: Julian Brown <julian at codesourcery dot com>
To: "H. J. Lu" <hjl at lucon dot org>
Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Paul Brook <paul at codesourcery dot com>,Mark Mitchell <mark at codesourcery dot com>
Date: Fri, 06 May 2005 18:10:56 +0100
Subject: Re: [PATCH] Don't use .gnu.linkonce with COMDAT
References: <427BA265.30608@codesourcery.com> <20050506170400.GC19231@lucon.org>

H. J. Lu wrote:

The patch?

Sorry, I am making a habit of forgetting the important bit! Now attached.

Julian

H.J.
On Fri, May 06, 2005 at 05:59:17PM +0100, Julian Brown wrote:
Hi,

This is a combination of various patches to make COMDAT groups work better. The first is from the csl-arm branch:

http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02497.html

This stops .gnu.linkonce from being used on targets which have COMDAT groups.

The next bit is new: rodata sections relating to text sections in COMDAT groups are now put into the same COMDAT group rather than being left stranded. i.e., a function put in a section:

.section .text._Z3fooi,"axG",@progbits,_Z3fooi,comdat

which generates a jump table for a switch statement, now has that table put into a section:

.section .rodata._Z3fooi,"aG",@progbits,_Z3fooi,comdat

rather than just .rodata. This fixes testcase g++.old-deja/g++.other/comdat4.C on x86_64 with the above bit applied.

Thirdly, COMDAT groups have proven buggy on various platforms with various (older) binutils versions. The third part of the patch stops COMDAT groups from being used altogether when the linker version is less than 2.16. This is perhaps slightly too pessimistic (i.e., earlier versions might work OK), but I'm reasonably confident my testing has shown it to be about right. This is based on a patch from H.J. Lu which never seems to have been applied:

http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02774.html

Though the required version has been bumped by several. This will have the effect of disabling COMDAT on some setups where it is currently enabled. I have also renamed the macro HAVE_GAS_COMDAT_GROUP to just HAVE_COMDAT_GROUP to better indicate that it's not just an assembler feature that's being relied on.

I have run tests on powerpc64-unknown-linux-gnu with c,c++ and x86_64-pc-linux-gnu with c,c++,java,objc, with binutils 2.16, with no regressions.

OK to apply?

ChangeLog:
 * gcc/configure.ac: Set ld_vers_major, ld_vers_minor and ld_vers_patch
 for GNU linker. Support linker version x.x.x.x.x. Require GNU linker
 20050308/2.16.0 or newer for comdat group.
 * gcc/configure: Regenerated.
 * gcc/config.in: Regenerated.
 * gcc/varasm.c (default_function_rodata_section): Put .rodata section
 in COMDAT group when necessary.
 (default_elf_asm_named_section): Rename HAVE_GAS_COMDAT_GROUP to
 HAVE_COMDAT_GROUP.
 (default_unique_section_1): Don't use .gnu.linkonce when COMDAT is
 available.

Index: gcc/configure.ac
===================================================================
RCS file: /home/gcc/repos/gcc/gcc/gcc/configure.ac,v
retrieving revision 2.106
diff -c -p -r2.106 configure.ac
*** gcc/configure.ac	27 Apr 2005 20:35:38 -0000	2.106
--- gcc/configure.ac	6 May 2005 15:03:48 -0000
*************** changequote(,)dnl
*** 2103,2110 ****
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)$,\1,p' \
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p' \
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p' \
! 	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p'`
      ld_date=`echo $ld_ver | sed -n 's,^.*\([2-9][0-9][0-9][0-9]\)[-]*\([01][0-9]\)[-]*\([0-3][0-9]\).*$,\1\2\3,p'`
      if test 0"$ld_date" -lt 20020404; then
        if test -n "$ld_date"; then
  	# If there was date string, but was earlier than 2002-04-04, fail
--- 2103,2114 ----
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)$,\1,p' \
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p' \
  	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p' \
! 	-e 's,^.*[ 	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[ 	].*$,\1,p' \
! 	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)[	 ].*$,\1,p'`
      ld_date=`echo $ld_ver | sed -n 's,^.*\([2-9][0-9][0-9][0-9]\)[-]*\([01][0-9]\)[-]*\([0-3][0-9]\).*$,\1\2\3,p'`
+     ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
+     ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
+     ld_vers_patch=`expr "$ld_vers" : '[0-9]*\.[0-9]*\.\([0-9]*\)'`
      if test 0"$ld_date" -lt 20020404; then
        if test -n "$ld_date"; then
  	# If there was date string, but was earlier than 2002-04-04, fail
*************** changequote(,)dnl
*** 2113,2121 ****
  	# If there was no date string nor ld version number, something is wrong
  	gcc_cv_ld_hidden=no
        else
- 	ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
- 	ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
- 	ld_vers_patch=`expr "$ld_vers" : '[0-9]*\.[0-9]*\.\([0-9]*\)'`
  	test -z "$ld_vers_patch" && ld_vers_patch=0
  	if test "$ld_vers_major" -lt 2; then
  	  gcc_cv_ld_hidden=no
--- 2117,2122 ----
*************** AC_DEFINE_UNQUOTED(HAVE_GAS_SHF_MERGE,
*** 2256,2273 ****
  [Define 0/1 if your assembler supports marking sections with SHF_MERGE flag.])
  
  gcc_GAS_CHECK_FEATURE(COMDAT group support, gcc_cv_as_comdat_group,
!  [elf,2,15,91], [--fatal-warnings],
   [.section .text,"axG",@progbits,.foo,comdat])
  if test $gcc_cv_as_comdat_group = yes; then
    gcc_cv_as_comdat_group_percent=no
  else
   gcc_GAS_CHECK_FEATURE(COMDAT group support, gcc_cv_as_comdat_group_percent,
!    [elf,2,15,91], [--fatal-warnings],
     [.section .text,"axG",%progbits,.foo,comdat])
  fi
! AC_DEFINE_UNQUOTED(HAVE_GAS_COMDAT_GROUP,
    [`if test $gcc_cv_as_comdat_group = yes || test $gcc_cv_as_comdat_group_percent = yes; then echo 1; else echo 0; fi`],
! [Define 0/1 if your assembler supports COMDAT group.])
  
  # Thread-local storage - the check is heavily parametrized.
  conftest_s=
--- 2257,2291 ----
  [Define 0/1 if your assembler supports marking sections with SHF_MERGE flag.])
  
  gcc_GAS_CHECK_FEATURE(COMDAT group support, gcc_cv_as_comdat_group,
!  [elf,2,16,0], [--fatal-warnings],
   [.section .text,"axG",@progbits,.foo,comdat])
  if test $gcc_cv_as_comdat_group = yes; then
    gcc_cv_as_comdat_group_percent=no
  else
   gcc_GAS_CHECK_FEATURE(COMDAT group support, gcc_cv_as_comdat_group_percent,
!    [elf,2,16,0], [--fatal-warnings],
     [.section .text,"axG",%progbits,.foo,comdat])
  fi
! if test $in_tree_ld != yes && test x"$ld_vers" != x; then
!   comdat_group=yes
!   if test 0"$ld_date" -lt 20050308; then
!     if test -n "$ld_date"; then
!       # If there was date string, but was earlier than 2005-03-08, fail
!       comdat_group=no
!     elif test "$ld_vers_major" -lt 2; then
!       comdat_group=no
!     elif test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 16; then
!       comdat_group=no
!     fi
!   fi
!   if test $comdat_group = no; then
!     gcc_cv_as_comdat_group=no
!     gcc_cv_as_comdat_group_percent=no
!   fi
! fi
! AC_DEFINE_UNQUOTED(HAVE_COMDAT_GROUP,
    [`if test $gcc_cv_as_comdat_group = yes || test $gcc_cv_as_comdat_group_percent = yes; then echo 1; else echo 0; fi`],
! [Define 0/1 if your assembler and linker support COMDAT groups.])
  
  # Thread-local storage - the check is heavily parametrized.
  conftest_s=
Index: gcc/varasm.c
===================================================================
RCS file: /home/gcc/repos/gcc/gcc/gcc/varasm.c,v
retrieving revision 1.507
diff -c -p -r1.507 varasm.c
*** gcc/varasm.c	27 Apr 2005 21:35:20 -0000	1.507
--- gcc/varasm.c	6 May 2005 15:58:46 -0000
*************** default_function_rodata_section (tree de
*** 618,625 ****
      {
        const char *name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl));
  
        /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo.  */
!       if (DECL_ONE_ONLY (decl) && strncmp (name, ".gnu.linkonce.t.", 16) == 0)
  	{
  	  size_t len = strlen (name) + 1;
  	  char *rname = alloca (len);
--- 618,636 ----
      {
        const char *name = TREE_STRING_POINTER (DECL_SECTION_NAME (decl));
  
+       if (DECL_ONE_ONLY (decl) && HAVE_COMDAT_GROUP)
+         {
+ 	  size_t len = strlen (name) + 1;
+ 	  char* rname = alloca (len + 2);
+           
+ 	  memcpy (rname, ".rodata", 7);
+ 	  memcpy (rname + 7, name + 5, len - 5);
+           named_section_real (rname, SECTION_LINKONCE, decl);
+ 	  return;
+ 	}
        /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo.  */
!       else if (DECL_ONE_ONLY (decl)
! 	       && strncmp (name, ".gnu.linkonce.t.", 16) == 0)
  	{
  	  size_t len = strlen (name) + 1;
  	  char *rname = alloca (len);
*************** default_elf_asm_named_section (const cha
*** 4866,4872 ****
       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_GAS_COMDAT_GROUP && (flags & SECTION_LINKONCE))
        && ! named_section_first_declaration (name))
      {
        fprintf (asm_out_file, "\t.section\t%s\n", name);
--- 4877,4883 ----
       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))
        && ! named_section_first_declaration (name))
      {
        fprintf (asm_out_file, "\t.section\t%s\n", name);
*************** default_elf_asm_named_section (const cha
*** 4887,4893 ****
      *f++ = 'S';
    if (flags & SECTION_TLS)
      *f++ = 'T';
!   if (HAVE_GAS_COMDAT_GROUP && (flags & SECTION_LINKONCE))
      *f++ = 'G';
    *f = '\0';
  
--- 4898,4904 ----
      *f++ = 'S';
    if (flags & SECTION_TLS)
      *f++ = 'T';
!   if (HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
      *f++ = 'G';
    *f = '\0';
  
*************** default_elf_asm_named_section (const cha
*** 4914,4920 ****
  
        if (flags & SECTION_ENTSIZE)
  	fprintf (asm_out_file, ",%d", flags & SECTION_ENTSIZE);
!       if (HAVE_GAS_COMDAT_GROUP && (flags & SECTION_LINKONCE))
  	fprintf (asm_out_file, ",%s,comdat", 
  		 lang_hooks.decls.comdat_group (decl));
      }
--- 4925,4931 ----
  
        if (flags & SECTION_ENTSIZE)
  	fprintf (asm_out_file, ",%d", flags & SECTION_ENTSIZE);
!       if (HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))
  	fprintf (asm_out_file, ",%s,comdat", 
  		 lang_hooks.decls.comdat_group (decl));
      }
*************** default_unique_section (tree decl, int r
*** 5226,5232 ****
  void
  default_unique_section_1 (tree decl, int reloc, int shlib)
  {
!   bool one_only = DECL_ONE_ONLY (decl);
    const char *prefix, *name;
    size_t nlen, plen;
    char *string;
--- 5237,5244 ----
  void
  default_unique_section_1 (tree decl, int reloc, int shlib)
  {
!   /* We only need to use .gnu.linkonce if we don't have COMDAT groups.  */
!   bool one_only = DECL_ONE_ONLY (decl) && !HAVE_COMDAT_GROUP;
    const char *prefix, *name;
    size_t nlen, plen;
    char *string;

Follow-Ups:
- Re: [PATCH] Don't use .gnu.linkonce with COMDAT
  - From: Daniel Jacobowitz

References:
- [PATCH] Don't use .gnu.linkonce with COMDAT
  - From: Julian Brown
- Re: [PATCH] Don't use .gnu.linkonce with COMDAT
  - From: H. J. Lu

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]