From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16600 invoked by alias); 28 Nov 2012 19:11:42 -0000 Received: (qmail 16587 invoked by uid 22791); 28 Nov 2012 19:11:40 -0000 X-SWARE-Spam-Status: No, hits=-3.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_FV,TW_FW,TW_LG,TW_VP X-Spam-Check-By: sourceware.org Received: from mail-qa0-f54.google.com (HELO mail-qa0-f54.google.com) (209.85.216.54) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 28 Nov 2012 19:11:29 +0000 Received: by mail-qa0-f54.google.com with SMTP id g24so5278370qab.20 for ; Wed, 28 Nov 2012 11:11:28 -0800 (PST) MIME-Version: 1.0 Received: by 10.224.185.212 with SMTP id cp20mr22468582qab.2.1354129888283; Wed, 28 Nov 2012 11:11:28 -0800 (PST) Received: by 10.49.12.210 with HTTP; Wed, 28 Nov 2012 11:11:28 -0800 (PST) In-Reply-To: <50B5D4D9.6000400@gnu.org> References: <50B5D4D9.6000400@gnu.org> Date: Wed, 28 Nov 2012 19:11:00 -0000 Message-ID: Subject: Re: PATCH: Enable both ld and gold in GCC 4.8 From: "H.J. Lu" To: Paolo Bonzini , "Joseph S. Myers" Cc: Matthias Klose , GCC Patches , Diego Novillo Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-11/txt/msg02362.txt.bz2 On Wed, Nov 28, 2012 at 1:09 AM, Paolo Bonzini wrote: > I cannot approve the collect2.c, gcc.c, opts.c, doc/invoke.texi. I'll > include some comments anyway, since the patch needs some more work. > >> 2011-06-27 Doug Kwan >> >> Google ref 41164-p2 >> Backport upstream patch under review. >> >> 2011-01-19 Nick Clifton >> Matthias Klose >> >> * configure.ac (gcc_cv_gold_srcdir): New cached variable - >> contains the location of the gold sources. > > This is gcc_cv_ld_gold_srcdir in configure.ac. I prefer the name in the > changelog. I removed configure.ac change. We can create a separate patch to select gold as the default linker. But this patch will only support -fuse-ld=bfd and -fuse-ld=gold. >> - static const char *const ld_suffix = "ld"; >> - static const char *const plugin_ld_suffix = PLUGIN_LD_SUFFIX; >> - static const char *const real_ld_suffix = "real-ld"; >> + static const char *const ld_suffix = "ld"; >> + static const char *const gold_suffix = "ld.gold"; >> + static const char *const bfd_ld_suffix = "ld.bfd"; >> + static const char *const plugin_ld_suffix = PLUGIN_LD_SUFFIX; >> + static const char *const real_ld_suffix = "real-ld"; > > Please use an array for ld_suffix, gold_suffix, bfd_ld_suffix, > plugin_ld_suffix. Similar for full_ld_suffix and friends. Done. >> - bool use_plugin = false; >> + enum linker_select >> + { >> + DFLT_LINKER, >> + PLUGIN_LINKER, >> + GOLD_LINKER, >> + BFD_LINKER >> + } selected_linker = DFLT_LINKER; > > Please change the names to avoid the "DFLT" abbreviation. For example > USE_DEFAULT_LD, USE_PLUGIN_LD, USE_GOLD, USE_LD. Done. > >> AC_MSG_CHECKING(what linker to use) >> if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext \ >> || test "$gcc_cv_ld" = ../gold/ld-new$build_exeext; then >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >> index 51b6e85..b78f698 100644 >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -425,7 +425,7 @@ Objective-C and Objective-C++ Dialects}. >> -funit-at-a-time -funroll-all-loops -funroll-loops @gol >> -funsafe-loop-optimizations -funsafe-math-optimizations -funswitch-loops @gol >> -fvariable-expansion-in-unroller -fvect-cost-model -fvpt -fweb @gol >> --fwhole-program -fwpa -fuse-linker-plugin @gol >> +-fwhole-program -fwpa -fuse-ld -fuse-linker-plugin @gol >> --param @var{name}=@var{value} >> -O -O0 -O1 -O2 -O3 -Os -Ofast -Og} >> >> @@ -8419,6 +8419,16 @@ the comparison operation before register allocation is complete. >> >> Enabled at levels @option{-O}, @option{-O2}, @option{-O3}, @option{-Os}. >> >> +@item -fuse-ld=gold >> +Use the @command{gold} linker instead of the default linker. >> +This option is only necessary if GCC has been configured with >> +@option{--enable-gold} and @option{--enable-ld=default}. > > This is not really clear. You could have built binutils separately, > would -fuse-ld=gold work in that case? If not, I think that's a bug. > >> +@item -fuse-ld=bfd >> +Use the @command{ld.bfd} linker instead of the default linker. >> +This option is only necessary if GCC has been configured with >> +@option{--enable-gold} and @option{--enable-ld}. > > Same concerns here. Fixed. >> @item -fcprop-registers >> @opindex fcprop-registers >> After register allocation and post-register allocation instruction splitting, >> diff --git a/gcc/exec-tool.in b/gcc/exec-tool.in >> index 8a10775..46aaedc 100644 >> --- a/gcc/exec-tool.in >> +++ b/gcc/exec-tool.in >> @@ -1,6 +1,6 @@ >> #! /bin/sh >> >> -# Copyright (C) 2007, 2008, 2010 Free Software Foundation, Inc. >> +# Copyright (C) 2007, 2008, 2010, 2011 Free Software Foundation, Inc. >> # This file is part of GCC. >> >> # GCC is free software; you can redistribute it and/or modify >> @@ -21,11 +21,13 @@ >> >> ORIGINAL_AS_FOR_TARGET="@ORIGINAL_AS_FOR_TARGET@" >> ORIGINAL_LD_FOR_TARGET="@ORIGINAL_LD_FOR_TARGET@" >> +ORIGINAL_GOLD_FOR_TARGET="@ORIGINAL_GOLD_FOR_TARGET@" >> ORIGINAL_PLUGIN_LD_FOR_TARGET="@ORIGINAL_PLUGIN_LD_FOR_TARGET@" >> ORIGINAL_NM_FOR_TARGET="@ORIGINAL_NM_FOR_TARGET@" >> exeext=@host_exeext@ >> fast_install=@enable_fast_install@ >> objdir=@objdir@ >> +version="1.1" > > Why is the -v behavior now required? Please remove it from this patch. > I rewrote exec-tool.in change. Now you get # ./xgcc -B./ /tmp/x.c -fuse-ld=gold -v ... ./collect-ld --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -fuse-ld=gold /lib/../lib64/crt1.o /lib/../lib64/crti.o ./crtbegin.o -L. -L/lib/../lib64 -L/usr/lib/../lib64 /tmp/cclVWcGz.o -v -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed ./crtend.o /lib/../lib64/crtn.o GNU gold (Linux/GNU Binutils 2.23.51.0.7.20121127) 1.11 /usr/local/bin/ld.gold: fatal error: -f/--auxiliary may not be used without -shared collect2: error: ld returned 1 exit status This is because we pass everything to ld and ld.bfd/ld.gold doesn't understand -fuse-ld=bfd/-fuse-ld=gold. It isn't a problem for collect2 since it will filter-out -fuse-ld=bfd/-fuse-ld=gold. I will submit a binutils patch to ignore -fuse-ld=bfd/-fuse-ld=gold, similar to -flto options. >> diff --git a/gcc/gcc.c b/gcc/gcc.c >> index 13e93e5..1702d2c 100644 >> --- a/gcc/gcc.c >> +++ b/gcc/gcc.c >> @@ -705,6 +705,9 @@ proper position among the other output files. */ >> LINK_PLUGIN_SPEC \ >> "%{flto|flto=*:%> %{flto} %{flto=*} %l " LINK_PIE_SPEC \ >> + "%{fuse-ld=gold:%{fuse-ld=bfd:%e-fuse-ld=gold and -fuse-ld=bfd may not be used together}} \ >> + %{fuse-ld=gold:-use-gold} \ >> + %{fuse-ld=bfd:-use-ld}" \ > > Why do we need to use separate spellings of the options in gcc and collect2? > This is a bug in the original patch. If we pass -fuse-ld=bfd/-fuse-ld=gold to linker, linker will complain. -use-gold/-use-ld are valid linker options: -u SYMBOL, --undefined SYMBOL It is wrong to abuse -use-gold/-use-ld. Here is the updated patch. OK for trunk? Thanks. -- H.J. ---- 2012-11-28 Nick Clifton Matthias Klose Doug Kwan H.J. Lu * collect2.c (main): Support -fuse-ld=bfd and -fuse-ld=gold. * exec-tool.in: Likewise. * common.opt: Add fuse-ld=bfd and fuse-ld=gold. * gcc.c (LINK_COMMAND_SPEC): Pass -fuse-ld=* to collect2. * opts.c (comman_handle_option): Ignore -fuse-ld=bfd and -fuse-ld=gold. * doc/invoke.texi: Document Da-fuse-ld=bfd and -fuse-ld=gold. diff --git a/gcc/collect2.c b/gcc/collect2.c index 49c4030..19bdc29 100644 --- a/gcc/collect2.c +++ b/gcc/collect2.c @@ -842,8 +842,21 @@ maybe_run_lto_and_relink (char **lto_ld_argv, char **object_lst, int main (int argc, char **argv) { - static const char *const ld_suffix = "ld"; - static const char *const plugin_ld_suffix = PLUGIN_LD_SUFFIX; + enum linker_select + { + USE_DEFAULT_LD, + USE_PLUGIN_LD, + USE_GOLD_LD, + USE_BFD_LD, + USE_LD_MAX + } selected_linker = USE_DEFAULT_LD; + static const char *const ld_suffixes[USE_LD_MAX] = + { + "ld", + PLUGIN_LD_SUFFIX, + "ld.gold", + "ld.bfd" + }; static const char *const real_ld_suffix = "real-ld"; static const char *const collect_ld_suffix = "collect-ld"; static const char *const nm_suffix = "nm"; @@ -854,16 +867,13 @@ main (int argc, char **argv) static const char *const strip_suffix = "strip"; static const char *const gstrip_suffix = "gstrip"; + const char *full_ld_suffixes[USE_LD_MAX]; #ifdef CROSS_DIRECTORY_STRUCTURE /* If we look for a program in the compiler directories, we just use the short name, since these directories are already system-specific. But it we look for a program in the system directories, we need to qualify the program name with the target machine. */ - const char *const full_ld_suffix = - concat(target_machine, "-", ld_suffix, NULL); - const char *const full_plugin_ld_suffix = - concat(target_machine, "-", plugin_ld_suffix, NULL); const char *const full_nm_suffix = concat (target_machine, "-", nm_suffix, NULL); const char *const full_gnm_suffix = @@ -877,13 +887,11 @@ main (int argc, char **argv) const char *const full_gstrip_suffix = concat (target_machine, "-", gstrip_suffix, NULL); #else - const char *const full_ld_suffix = ld_suffix; - const char *const full_plugin_ld_suffix = plugin_ld_suffix; - const char *const full_nm_suffix = nm_suffix; - const char *const full_gnm_suffix = gnm_suffix; #ifdef LDD_SUFFIX const char *const full_ldd_suffix = ldd_suffix; #endif + const char *const full_nm_suffix = nm_suffix; + const char *const full_gnm_suffix = gnm_suffix; const char *const full_strip_suffix = strip_suffix; const char *const full_gstrip_suffix = gstrip_suffix; #endif /* CROSS_DIRECTORY_STRUCTURE */ @@ -900,6 +908,7 @@ main (int argc, char **argv) char **ld1_argv; const char **ld1; bool use_plugin = false; + bool use_collect_ld = false; /* The kinds of symbols we will have to consider when scanning the outcome of a first pass link. This is ALL to start with, then might @@ -919,6 +928,15 @@ main (int argc, char **argv) int first_file; int num_c_args; char **old_argv; + int i; + + for (i = 0; i < USE_LD_MAX; i++) + full_ld_suffixes[i] +#ifdef CROSS_DIRECTORY_STRUCTURE + = concat(target_machine, "-", ld_suffixes[i], NULL); +#else + = ld_suffixes[i]; +#endif p = argv[0] + strlen (argv[0]); while (p != argv[0] && !IS_DIR_SEPARATOR (p[-1])) @@ -980,7 +998,6 @@ main (int argc, char **argv) are called. We also look for the -flto or -flto-partition=none flag to know what LTO mode we are in. */ { - int i; bool no_partition = false; for (i = 1; argv[i] != NULL; i ++) @@ -998,7 +1015,14 @@ main (int argc, char **argv) { use_plugin = true; lto_mode = LTO_MODE_NONE; + if (selected_linker == USE_DEFAULT_LD) + selected_linker = USE_PLUGIN_LD; } + else if (strcmp (argv[i], "-fuse-ld=bfd") == 0) + selected_linker = USE_BFD_LD; + else if (strcmp (argv[i], "-fuse-ld=gold") == 0) + selected_linker = USE_GOLD_LD; + #ifdef COLLECT_EXPORT_LIST /* since -brtl, -bexport, -b64 are not position dependent also check for them here */ @@ -1095,21 +1119,18 @@ main (int argc, char **argv) ld_file_name = find_a_file (&cpath, real_ld_suffix); /* Likewise for `collect-ld'. */ if (ld_file_name == 0) - ld_file_name = find_a_file (&cpath, collect_ld_suffix); + { + ld_file_name = find_a_file (&cpath, collect_ld_suffix); + use_collect_ld = ld_file_name != 0; + } /* Search the compiler directories for `ld'. We have protection against recursive calls in find_a_file. */ if (ld_file_name == 0) - ld_file_name = find_a_file (&cpath, - use_plugin - ? plugin_ld_suffix - : ld_suffix); + ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker]); /* Search the ordinary system bin directories for `ld' (if native linking) or `TARGET-ld' (if cross). */ if (ld_file_name == 0) - ld_file_name = find_a_file (&path, - use_plugin - ? full_plugin_ld_suffix - : full_ld_suffix); + ld_file_name = find_a_file (&path, full_ld_suffixes[selected_linker]); #ifdef REAL_NM_FILE_NAME nm_file_name = find_a_file (&path, REAL_NM_FILE_NAME); @@ -1266,6 +1287,13 @@ main (int argc, char **argv) "configuration"); #endif } + else if (!use_collect_ld + && strncmp (arg, "-fuse-ld=", 9) == 0) + { + /* Do not pass -fuse-ld={bfd|gold} to the linker. */ + ld1--; + ld2--; + } #ifdef TARGET_AIX_VERSION else { diff --git a/gcc/common.opt b/gcc/common.opt index 4c8bd11..7b36ba3 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2171,6 +2171,12 @@ funwind-tables Common Report Var(flag_unwind_tables) Optimization Just generate unwind tables for exception handling +fuse-ld=bfd +Common Var(flag_use_ld_bfd) Negative(fuse-ld=gold) Undocumented + +fuse-ld=gold +Common Var(flag_use_ld_gold) Negative(fuse-ld=bfd) Undocumented + fuse-linker-plugin Common Undocumented diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 51b6e85..3558f43 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -425,7 +425,7 @@ Objective-C and Objective-C++ Dialects}. -funit-at-a-time -funroll-all-loops -funroll-loops @gol -funsafe-loop-optimizations -funsafe-math-optimizations -funswitch-loops @gol -fvariable-expansion-in-unroller -fvect-cost-model -fvpt -fweb @gol --fwhole-program -fwpa -fuse-linker-plugin @gol +-fwhole-program -fwpa -fuse-ld=@var{linker} -fuse-linker-plugin @gol --param @var{name}=@var{value} -O -O0 -O1 -O2 -O3 -Os -Ofast -Og} @@ -8419,6 +8419,12 @@ the comparison operation before register allocation is complete. Enabled at levels @option{-O}, @option{-O2}, @option{-O3}, @option{-Os}. +@item -fuse-ld=gold +Use the @command{gold} linker instead of the default linker. + +@item -fuse-ld=bfd +Use the @command{ld.bfd} linker instead of the default linker. + @item -fcprop-registers @opindex fcprop-registers After register allocation and post-register allocation instruction splitting, diff --git a/gcc/exec-tool.in b/gcc/exec-tool.in index 8a10775..8f9ae27 100644 --- a/gcc/exec-tool.in +++ b/gcc/exec-tool.in @@ -36,13 +36,28 @@ case "$invoked" in dir=gas ;; collect-ld) - # when using a linker plugin, gcc will always pass '-plugin' as the - # first or second option to the linker. - if test x"$1" = "x-plugin" || test x"$2" = "x-plugin"; then - original=$ORIGINAL_PLUGIN_LD_FOR_TARGET - else - original=$ORIGINAL_LD_FOR_TARGET - fi + # Check -fuse-ld=bfd and -fuse-ld=gold + case " $* " in + *\ -fuse-ld=bfd\ *) + if test -x ${ORIGINAL_LD_FOR_TARGET}.bfd; then + original=${ORIGINAL_LD_FOR_TARGET}.bfd; + fi + ;; + *\ -fuse-ld=gold\ *) + if test -x ${ORIGINAL_LD_FOR_TARGET}.gold; then + original=${ORIGINAL_LD_FOR_TARGET}.gold; + fi + ;; + *) + # when using a linker plugin, gcc will always pass '-plugin' as the + # first or second option to the linker. + if test x"$1" = "x-plugin" || test x"$2" = "x-plugin"; then + original=$ORIGINAL_PLUGIN_LD_FOR_TARGET + else + original=$ORIGINAL_LD_FOR_TARGET + fi + ;; + esac prog=ld-new$exeext dir=ld id=ld diff --git a/gcc/gcc.c b/gcc/gcc.c index 13e93e5..e0fee40 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -705,7 +705,8 @@ proper position among the other output files. */ LINK_PLUGIN_SPEC \ "%{flto|flto=*:%max_errors = value; break; + case OPT_fuse_ld_bfd: + case OPT_fuse_ld_gold: case OPT_fuse_linker_plugin: /* No-op. Used by the driver and passed to us because it starts with f.*/ break;