diff mbox

PR66870 PowerPC64 Enable gold linker with split stack

Message ID 55D4DA0B.6000106@ubuntu.com
State New
Headers show

Commit Message

Matthias Klose Aug. 19, 2015, 7:33 p.m. UTC
On 08/18/2015 10:36 PM, Lynn A. Boger wrote:
> As discussed in PR 66870, for ppc64le and ppc64le it is preferred to
>  use the gold linker with gccgo or gcc if the split stack option is enabled.
> Use of the gold linker with the split stack option results in less storage
> allocated for goroutine stacks; if split stack is used without the gold
> linker then some testcase failures can occur.
> 
>   Since the use of the gold linker has not been well tested
> with all gcc compilers on Power, it is only used as the linker if the
> split stack option is used.
> 
> This adds the capability to the configure for gcc and libgo to determine
> if the gold linker is available at build time, either in the path or explicitly
>  configured, and its version supports split stack.  If that is the case then
> defines are set that cause the gold linker to be used by the compiler when
> -fsplit-stack is used.  This applies to ppc64 and ppc64le.  Other platforms
> with split stack work as before.
> 
> 2015-08-18    Lynn Boger <laboger@linux.vnet.ibm.com>
> 
> gcc/
>         PR target/66870
>         config/rs6000/linux64.h: When HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK
>         is defined add -fuse-ld=gold if fsplit-stack and not m32
>         config/rs6000/sysv4.h:  Define TARGET_CAN_SPLIT_STACK based on
>         LIBC version.
>         config.in:  Set up HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK.
>         configure.ac:  When gold linker is available and its version
>         supports split stack on ppc64, ppc64le, define
>         HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK.
>         configure:  Regenerate.
> 
> libgo/
>         PR target/66870
>         configure.ac:  When gccgo for building libgo uses the gold version
>         containing split stack support on ppc64, ppc64le, define
>         LINKER_SUPPORTS_SPLIT_STACK.
>         configure:  Regenerate.
> 
> For platforms other than ppc64 and ppc64le, the configure for gcc
> and libgo behave as before.

why keep the old behaviour for other archs that have split stack support? Is it
really necessary to make this dependent on the target? I'm still using an
unreviewed/unpinged patch to enable gold for gccgo (attached).

Matthias
# DP: Pass -fuse-ld=gold to gccgo on targets supporting -fsplit-stack

gcc/go/                                                                                      
                                                                                             
        * gospec.c (lang_specific_driver): Pass -fuse-ld=gold on targets                     
        supporting -fsplit-stack, unless overwritten.                                        
                                                                                             
gcc/                                                                                         
        * configure.ac: New define HAVE_GOLD_NON_DEFAULT.                                    
        * config.in: Regenerate.

Comments

David Edelsohn Aug. 19, 2015, 7:37 p.m. UTC | #1
On Wed, Aug 19, 2015 at 3:33 PM, Matthias Klose <doko@ubuntu.com> wrote:

> why keep the old behaviour for other archs that have split stack support? Is it
> really necessary to make this dependent on the target? I'm still using an
> unreviewed/unpinged patch to enable gold for gccgo (attached).

I much prefer your patch.

Thanks, David
Lynn A. Boger Aug. 19, 2015, 10:05 p.m. UTC | #2
The split stack support only recently went into the gold
linker for Power so the configure needs to detect if the
gold linker version contains that support.  If the build tries
  to use a gold linker without that support the build
will fail on Power.  My understanding was that the gold
linker support had been there for other platforms.  That is why
the configure check is target dependent and only Power cares
about the version number.

I started out with a simpler patch like you have but after trying to 
build it
on ppc64le, ppc64, and ppc this is what I ended up with and I didn't
want to mess up other platforms so I left those as is.

On 08/19/2015 02:33 PM, Matthias Klose wrote:
> On 08/18/2015 10:36 PM, Lynn A. Boger wrote:
>> As discussed in PR 66870, for ppc64le and ppc64le it is preferred to
>>   use the gold linker with gccgo or gcc if the split stack option is enabled.
>> Use of the gold linker with the split stack option results in less storage
>> allocated for goroutine stacks; if split stack is used without the gold
>> linker then some testcase failures can occur.
>>
>>    Since the use of the gold linker has not been well tested
>> with all gcc compilers on Power, it is only used as the linker if the
>> split stack option is used.
>>
>> This adds the capability to the configure for gcc and libgo to determine
>> if the gold linker is available at build time, either in the path or explicitly
>>   configured, and its version supports split stack.  If that is the case then
>> defines are set that cause the gold linker to be used by the compiler when
>> -fsplit-stack is used.  This applies to ppc64 and ppc64le.  Other platforms
>> with split stack work as before.
>>
>> 2015-08-18    Lynn Boger <laboger@linux.vnet.ibm.com>
>>
>> gcc/
>>          PR target/66870
>>          config/rs6000/linux64.h: When HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK
>>          is defined add -fuse-ld=gold if fsplit-stack and not m32
>>          config/rs6000/sysv4.h:  Define TARGET_CAN_SPLIT_STACK based on
>>          LIBC version.
>>          config.in:  Set up HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK.
>>          configure.ac:  When gold linker is available and its version
>>          supports split stack on ppc64, ppc64le, define
>>          HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK.
>>          configure:  Regenerate.
>>
>> libgo/
>>          PR target/66870
>>          configure.ac:  When gccgo for building libgo uses the gold version
>>          containing split stack support on ppc64, ppc64le, define
>>          LINKER_SUPPORTS_SPLIT_STACK.
>>          configure:  Regenerate.
>>
>> For platforms other than ppc64 and ppc64le, the configure for gcc
>> and libgo behave as before.
> why keep the old behaviour for other archs that have split stack support? Is it
> really necessary to make this dependent on the target? I'm still using an
> unreviewed/unpinged patch to enable gold for gccgo (attached).
>
> Matthias
>
>
Lynn A. Boger Aug. 19, 2015, 11:05 p.m. UTC | #3
Also, I don't think it is sufficient to add the option to enable the
gold linker in gospec.c.  That will only affect links when using gccgo.
You also want to use the gold linker with gcc if -fsplit-stack is used.
That is why I had to add it to a spec in linux64.h, so that -fuse-ld=gold
  is added if fsplit-stack is set for all compilers, not just gccgo.

On 08/19/2015 02:33 PM, Matthias Klose wrote:
> On 08/18/2015 10:36 PM, Lynn A. Boger wrote:
>> As discussed in PR 66870, for ppc64le and ppc64le it is preferred to
>>   use the gold linker with gccgo or gcc if the split stack option is enabled.
>> Use of the gold linker with the split stack option results in less storage
>> allocated for goroutine stacks; if split stack is used without the gold
>> linker then some testcase failures can occur.
>>
>>    Since the use of the gold linker has not been well tested
>> with all gcc compilers on Power, it is only used as the linker if the
>> split stack option is used.
>>
>> This adds the capability to the configure for gcc and libgo to determine
>> if the gold linker is available at build time, either in the path or explicitly
>>   configured, and its version supports split stack.  If that is the case then
>> defines are set that cause the gold linker to be used by the compiler when
>> -fsplit-stack is used.  This applies to ppc64 and ppc64le.  Other platforms
>> with split stack work as before.
>>
>> 2015-08-18    Lynn Boger <laboger@linux.vnet.ibm.com>
>>
>> gcc/
>>          PR target/66870
>>          config/rs6000/linux64.h: When HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK
>>          is defined add -fuse-ld=gold if fsplit-stack and not m32
>>          config/rs6000/sysv4.h:  Define TARGET_CAN_SPLIT_STACK based on
>>          LIBC version.
>>          config.in:  Set up HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK.
>>          configure.ac:  When gold linker is available and its version
>>          supports split stack on ppc64, ppc64le, define
>>          HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK.
>>          configure:  Regenerate.
>>
>> libgo/
>>          PR target/66870
>>          configure.ac:  When gccgo for building libgo uses the gold version
>>          containing split stack support on ppc64, ppc64le, define
>>          LINKER_SUPPORTS_SPLIT_STACK.
>>          configure:  Regenerate.
>>
>> For platforms other than ppc64 and ppc64le, the configure for gcc
>> and libgo behave as before.
> why keep the old behaviour for other archs that have split stack support? Is it
> really necessary to make this dependent on the target? I'm still using an
> unreviewed/unpinged patch to enable gold for gccgo (attached).
>
> Matthias
>
>
diff mbox

Patch

Index: b/src/gcc/go/gospec.c
===================================================================
--- a/src/gcc/go/gospec.c
+++ b/src/gcc/go/gospec.c
@@ -117,6 +117,10 @@  lang_specific_driver (struct cl_decoded_
   /* Whether the -S option was used.  */
   bool saw_opt_S = false;
 
+  /* "-fuse-ld=" if it appears on the command line.  */
+  bool saw_use_ld ATTRIBUTE_UNUSED = false;
+  int need_gold = 0;
+
   /* The first input file with an extension of .go.  */
   const char *first_go_file = NULL;  
 
@@ -217,6 +221,11 @@  lang_specific_driver (struct cl_decoded_
 	    }
 
 	  break;
+
+	case OPT_fuse_ld_bfd:
+	case OPT_fuse_ld_gold:
+	  saw_use_ld = true;
+	  break;
 	}
     }
 
@@ -226,8 +235,14 @@  lang_specific_driver (struct cl_decoded_
   shared_libgcc = 0;
 #endif
 
+#if defined(TARGET_CAN_SPLIT_STACK) && defined(HAVE_GOLD_NON_DEFAULT)
+  if (!saw_use_ld)
+    need_gold = 1;
+#endif
+
   /* Make sure to have room for the trailing NULL argument.  */
-  num_args = argc + need_math + shared_libgcc + (library > 0) * 5 + 10;
+  num_args = argc + need_math + shared_libgcc + need_gold +
+    (library > 0) * 5 + 10;
   new_decoded_options = XNEWVEC (struct cl_decoded_option, num_args);
 
   i = 0;
@@ -244,6 +259,14 @@  lang_specific_driver (struct cl_decoded_
 		       &new_decoded_options[j]);
       j++;
     }
+#ifdef HAVE_GOLD_NON_DEFAULT
+  if (need_gold)
+    {
+      generate_option (OPT_fuse_ld_gold, NULL, 1, CL_DRIVER,
+		       &new_decoded_options[j]);
+      j++;
+    }
+#endif
 #endif
 
   /* NOTE: We start at 1 now, not 0.  */
Index: b/src/gcc/config.in
===================================================================
--- a/src/gcc/config.in
+++ b/src/gcc/config.in
@@ -1277,6 +1277,12 @@ 
 #endif
 
 
+/* Define if the gold linker is available as a non-default */
+#ifndef USED_FOR_TARGET
+#undef HAVE_GOLD_NON_DEFAULT
+#endif
+
+
 /* Define if you have the iconv() function. */
 #ifndef USED_FOR_TARGET
 #undef HAVE_ICONV
Index: b/src/gcc/configure.ac
===================================================================
--- a/src/gcc/configure.ac
+++ b/src/gcc/configure.ac
@@ -2225,6 +2225,12 @@  if test x$gcc_cv_ld != x; then
 fi
 AC_MSG_RESULT($ld_is_gold)
 
+# Check to see if ld is used, and gold is available
+if test x$ld_is_gold = xno && which ${gcc_cv_ld}.gold >/dev/null 2>&1; then
+  AC_DEFINE(HAVE_GOLD_NON_DEFAULT, 1,
+  	    [Define if the gold linker is available as a non-default])
+fi
+
 ORIGINAL_LD_FOR_TARGET=$gcc_cv_ld
 AC_SUBST(ORIGINAL_LD_FOR_TARGET)
 case "$ORIGINAL_LD_FOR_TARGET" in