Message ID | 55DF7A72.2040208@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On Thu, Aug 27, 2015 at 2:00 PM, Lynn A. Boger <laboger@linux.vnet.ibm.com> wrote: > Here is an updated patch, with a summary of the differences from my previous > patch: > > - In my previous patch gcc configure was verifying the gold linker even if > it was the > default linker, but that is not necessary since in that case -fuse-ld=gold > does not > need to be set. Gold version checking is now only done if the alternate > linker is gold > and the default linker is not. > - The STACK_SPLIT_STACK spec define found in gcc/gcc.c now adds > -fuse-ld=gold > if the gcc configure determines the alternate gold linker has split stack > support. > - A case statement is now used in gcc configure to verify the gold version, > to make it > easier for other platforms to add their checks if necessary. I don't know > if other > platforms require this checking; Matthias' patch did not check the version. > For powerpc64 > big and little endian we have to check the gold linker version because the > split > stack support was added recently and older gold linkers won't work. > - The version checking of the gold linker was removed from the libgo > configure > since gcc configure has already decided if it is correct. > - TARGET_CAN_SPLIT_STACK_64BIT is now defined in sysv4.h if the glibc > version > is correct for split stack for powerpc64 big and little endian. This define > is used in > go/gospec.c in the same way that TARGET_CAN_SPLIT_STACK is used now but, > additionally verifies that it is a 64 bit compile. This is necessary > because split > stack support is not available for ppc 32 bit big endian in gcc or the gold > linker. > > Bootstrapped and tested on x86_64, ppc64le, ppc64 (ran m32, m64 tests) > > > 2015-08-27 Lynn Boger <laboger@linux.vnet.ibm.com> > > gcc/ > PR target/66870 > config/rs6000/sysv4.h: Define TARGET_CAN_SPLIT_STACK_64BIT > based on LIBC version. > config.in: Set up HAVE_GOLD_ALTERNATE. > configure.ac: Define HAVE_GOLD_ALTERNATE if the version of the > gold > linker supports split stack. > configure: Regenerate. > gcc.c: Add -fuse-ld=gold to STACK_SPLIT_SPEC if > HAVE_GOLD_ALTERNATE > is defined. > go/gospec.c: (lang_specific_driver): Use > TARGET_CAN_SPLIT_STACK_64BIT > to control setting of fsplit-stack and u,pthread_create options for > 64 bit > compiles. I'm not sure who is going to approve this patch overall, but I'd rather the gospec.c changes weren't so #ifdef heavy. I would set m32 unconditionally. Then write something like supports_split_stack = 0; #ifdef TARGET_CAN_SPLIT_STACK supports_split_stack = 1; #endif #ifdef TARGET_CAN_SPLIT_STACK64 if (m32) supports_split_stack = 0; #endif Then change the #ifdef TARGET_CAN_SPLIT_STACK code to test supports_split_stack. Thanks. Ian
On Thu, Aug 27, 2015 at 5:00 PM, Lynn A. Boger <laboger@linux.vnet.ibm.com> wrote: > Here is an updated patch, with a summary of the differences from my previous > patch: > > - In my previous patch gcc configure was verifying the gold linker even if > it was the > default linker, but that is not necessary since in that case -fuse-ld=gold > does not > need to be set. Gold version checking is now only done if the alternate > linker is gold > and the default linker is not. > - The STACK_SPLIT_STACK spec define found in gcc/gcc.c now adds > -fuse-ld=gold > if the gcc configure determines the alternate gold linker has split stack > support. > - A case statement is now used in gcc configure to verify the gold version, > to make it > easier for other platforms to add their checks if necessary. I don't know > if other > platforms require this checking; Matthias' patch did not check the version. > For powerpc64 > big and little endian we have to check the gold linker version because the > split > stack support was added recently and older gold linkers won't work. > - The version checking of the gold linker was removed from the libgo > configure > since gcc configure has already decided if it is correct. > - TARGET_CAN_SPLIT_STACK_64BIT is now defined in sysv4.h if the glibc > version > is correct for split stack for powerpc64 big and little endian. This define > is used in > go/gospec.c in the same way that TARGET_CAN_SPLIT_STACK is used now but, > additionally verifies that it is a 64 bit compile. This is necessary > because split > stack support is not available for ppc 32 bit big endian in gcc or the gold > linker. > > Bootstrapped and tested on x86_64, ppc64le, ppc64 (ran m32, m64 tests) > > > 2015-08-27 Lynn Boger <laboger@linux.vnet.ibm.com> > > gcc/ > PR target/66870 > config/rs6000/sysv4.h: Define TARGET_CAN_SPLIT_STACK_64BIT > based on LIBC version. > config.in: Set up HAVE_GOLD_ALTERNATE. > configure.ac: Define HAVE_GOLD_ALTERNATE if the version of the > gold > linker supports split stack. > configure: Regenerate. > gcc.c: Add -fuse-ld=gold to STACK_SPLIT_SPEC if > HAVE_GOLD_ALTERNATE > is defined. > go/gospec.c: (lang_specific_driver): Use > TARGET_CAN_SPLIT_STACK_64BIT > to control setting of fsplit-stack and u,pthread_create options for > 64 bit > compiles. I don't have authority to approve for most of the patch. I noticed that the test for Gold version number is different than a similar test in another part of configure. And the sed pattern seems to be too restrictive -- it fails to extract a version number for my tests with Gold in a few different Linux distros. This should be addressed instead of silently misidentifying split stack support in some distros. - David
I will make the changes Ian suggested to gospec.c and was planning to fix the sed string in gcc/configure.ac as David suggested. I need some feedback on whether to enable the gold linker at all for split stack on platforms other than Power in gcc/configure.ac. I don't know if there are gold linker versions that should be verified for non-Power platforms. My first patch only enabled it on Power and that is what I think should be done. Those who would like to use gold with split stack for other platforms can enable it under the appropriate conditions. On 09/15/2015 01:18 PM, David Edelsohn wrote: > On Thu, Aug 27, 2015 at 5:00 PM, Lynn A. Boger > <laboger@linux.vnet.ibm.com> wrote: >> Here is an updated patch, with a summary of the differences from my previous >> patch: >> >> - In my previous patch gcc configure was verifying the gold linker even if >> it was the >> default linker, but that is not necessary since in that case -fuse-ld=gold >> does not >> need to be set. Gold version checking is now only done if the alternate >> linker is gold >> and the default linker is not. >> - The STACK_SPLIT_STACK spec define found in gcc/gcc.c now adds >> -fuse-ld=gold >> if the gcc configure determines the alternate gold linker has split stack >> support. >> - A case statement is now used in gcc configure to verify the gold version, >> to make it >> easier for other platforms to add their checks if necessary. I don't know >> if other >> platforms require this checking; Matthias' patch did not check the version. >> For powerpc64 >> big and little endian we have to check the gold linker version because the >> split >> stack support was added recently and older gold linkers won't work. >> - The version checking of the gold linker was removed from the libgo >> configure >> since gcc configure has already decided if it is correct. >> - TARGET_CAN_SPLIT_STACK_64BIT is now defined in sysv4.h if the glibc >> version >> is correct for split stack for powerpc64 big and little endian. This define >> is used in >> go/gospec.c in the same way that TARGET_CAN_SPLIT_STACK is used now but, >> additionally verifies that it is a 64 bit compile. This is necessary >> because split >> stack support is not available for ppc 32 bit big endian in gcc or the gold >> linker. >> >> Bootstrapped and tested on x86_64, ppc64le, ppc64 (ran m32, m64 tests) >> >> >> 2015-08-27 Lynn Boger <laboger@linux.vnet.ibm.com> >> >> gcc/ >> PR target/66870 >> config/rs6000/sysv4.h: Define TARGET_CAN_SPLIT_STACK_64BIT >> based on LIBC version. >> config.in: Set up HAVE_GOLD_ALTERNATE. >> configure.ac: Define HAVE_GOLD_ALTERNATE if the version of the >> gold >> linker supports split stack. >> configure: Regenerate. >> gcc.c: Add -fuse-ld=gold to STACK_SPLIT_SPEC if >> HAVE_GOLD_ALTERNATE >> is defined. >> go/gospec.c: (lang_specific_driver): Use >> TARGET_CAN_SPLIT_STACK_64BIT >> to control setting of fsplit-stack and u,pthread_create options for >> 64 bit >> compiles. > I don't have authority to approve for most of the patch. > > I noticed that the test for Gold version number is different than a > similar test in another part of configure. And the sed pattern seems > to be too restrictive -- it fails to extract a version number for my > tests with Gold in a few different Linux distros. This should be > addressed instead of silently misidentifying split stack support in > some distros. > > - David > >
On Tue, Sep 15, 2015 at 11:24 AM, Lynn A. Boger <laboger@linux.vnet.ibm.com> wrote: > > I need some feedback on whether to enable the gold linker at > all for split stack on platforms other than Power in gcc/configure.ac. > I don't know if there are gold linker versions that should be verified for > non-Power platforms. My first patch only enabled it on Power and that > is what I think should be done. Those who would like to use gold > with split stack for other platforms can enable it under the appropriate > conditions. I think that is fine for now. If you want to enable gold for i386/x86_64 with -fsplit-stack, that is also fine. Ian
Index: gcc/config/rs6000/sysv4.h =================================================================== --- gcc/config/rs6000/sysv4.h (revision 227266) +++ gcc/config/rs6000/sysv4.h (working copy) @@ -946,6 +946,14 @@ ncrtn.o%s" #undef TARGET_ASAN_SHADOW_OFFSET #define TARGET_ASAN_SHADOW_OFFSET rs6000_asan_shadow_offset +/* On ppc64 and ppc64le, split stack is only support for + 64 bit. */ +#undef TARGET_CAN_SPLIT_STACK_64BIT +#if TARGET_GLIBC_MAJOR > 2 \ + || (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 18) +#define TARGET_CAN_SPLIT_STACK_64BIT +#endif + /* This target uses the sysv4.opt file. */ #define TARGET_USES_SYSV4_OPT 1 Index: gcc/config.in =================================================================== --- gcc/config.in (revision 227266) +++ gcc/config.in (working copy) @@ -1296,6 +1296,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: gcc/configure.ac =================================================================== --- gcc/configure.ac (revision 227266) +++ gcc/configure.ac (working copy) @@ -2247,6 +2247,37 @@ if test x$gcc_cv_ld != x; then fi AC_MSG_RESULT($ld_is_gold) +AC_MSG_CHECKING(gold linker available as non default) +# Check to see if default ld is not gold, but gold is +# available. If gcc was configured with gold then +# nothing more is needed. +# +if test x$ld_is_gold = xno && which ${gcc_cv_ld}.gold >/dev/null 2>&1; then + case $target in +# check that the gold version contains the split stack support +# on powerpc64 big and little endian + powerpc64*-*-*) + ld_gold=`which ${gcc_cv_ld}.gold` + gold_vers=`$ld_gold --version | sed 1q | sed -n -e 's/.*Binutils.* \([[0-9]][[0-9]]*\.[[^)]]*\)).*$/\1/p'` + case "$gold_vers" in + 2.25.[[1-9]]*|2.2[[6-9]][[.0-9]]*|2.[[3-9]][[.0-9]]*|[[3-9]].[[.0-9]]*) gold_non_default=yes + ;; + *) gold_non_default=no + ;; + esac + ;; +# Version check needed on other platforms that care about gold with split stack? + *) + gold_non_default=yes + ;; + esac + if test $gold_non_default = yes; then + AC_DEFINE(HAVE_GOLD_NON_DEFAULT, 1, + [Define if the gold linker is available as a non-default]) + fi +fi +AC_MSG_RESULT($gold_non_default) + ORIGINAL_LD_FOR_TARGET=$gcc_cv_ld AC_SUBST(ORIGINAL_LD_FOR_TARGET) case "$ORIGINAL_LD_FOR_TARGET" in Index: gcc/gcc.c =================================================================== --- gcc/gcc.c (revision 227266) +++ gcc/gcc.c (working copy) @@ -666,7 +666,11 @@ proper position among the other output files. */ libgcc. This is not yet a real spec, though it could become one; it is currently just stuffed into LINK_SPEC. FIXME: This wrapping only works with GNU ld and gold. */ +#ifdef HAVE_GOLD_NON_DEFAULT +#define STACK_SPLIT_SPEC " %{fsplit-stack: -fuse-ld=gold --wrap=pthread_create}" +#else #define STACK_SPLIT_SPEC " %{fsplit-stack: --wrap=pthread_create}" +#endif #ifndef LIBASAN_SPEC #define STATIC_LIBASAN_LIBS \ Index: gcc/go/gospec.c =================================================================== --- gcc/go/gospec.c (revision 227266) +++ gcc/go/gospec.c (working copy) @@ -117,6 +117,11 @@ lang_specific_driver (struct cl_decoded_option **i /* Whether the -S option was used. */ bool saw_opt_S = false; +#ifdef TARGET_CAN_SPLIT_STACK_64BIT + /* Whether the -m32 option was used. */ + bool saw_opt_m32 = false; +#endif + /* The first input file with an extension of .go. */ const char *first_go_file = NULL; @@ -152,6 +157,12 @@ lang_specific_driver (struct cl_decoded_option **i library = (library == 0) ? 1 : library; break; +#ifdef TARGET_CAN_SPLIT_STACK_64BIT + case OPT_m32: + saw_opt_m32 = true; + break; +#endif + case OPT_pg: case OPT_p: saw_profile_flag = true; @@ -237,8 +248,12 @@ lang_specific_driver (struct cl_decoded_option **i new_decoded_options[j++] = decoded_options[i++]; /* If we are linking, pass -fsplit-stack if it is supported. */ -#ifdef TARGET_CAN_SPLIT_STACK +#if defined(TARGET_CAN_SPLIT_STACK) || defined(TARGET_CAN_SPLIT_STACK_64BIT) +#ifdef TARGET_CAN_SPLIT_STACK_64BIT + if ((library >= 0) && !saw_opt_m32) +#else if (library >= 0) +#endif { generate_option (OPT_fsplit_stack, NULL, 1, CL_DRIVER, &new_decoded_options[j]); @@ -381,13 +396,17 @@ lang_specific_driver (struct cl_decoded_option **i generate_option (OPT_shared_libgcc, NULL, 1, CL_DRIVER, &new_decoded_options[j++]); -#ifdef TARGET_CAN_SPLIT_STACK +#if defined(TARGET_CAN_SPLIT_STACK) || defined(TARGET_CAN_SPLIT_STACK_64BIT) /* libgcc wraps pthread_create to support split stack, however, due to relative ordering of -lpthread and -lgcc, we can't just mark __real_pthread_create in libgcc as non-weak. But we need to link in pthread_create from pthread if we are statically linking, so we work- around by passing -u pthread_create to the linker. */ +#ifdef TARGET_CAN_SPLIT_STACK_64BIT + if (static_link && !saw_opt_m32) +#else if (static_link) +#endif { generate_option (OPT_Wl_, "-u,pthread_create", 1, CL_DRIVER, &new_decoded_options[j]);