On Tue, Jan 5, 2010 at 11:08 AM, Ian Lance Taylor wrote: > "H.J. Lu" writes: > >> diff --git a/configure.ac b/configure.ac >> index 407ab59..b349633 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -311,10 +311,11 @@ esac >>  # Handle --enable-gold. >> >>  AC_ARG_ENABLE(gold, >> -[  --enable-gold           use gold instead of ld], >> +[  --enable-gold[[=ARG]]     build gold [[ARG={yes,both}]]], >>  ENABLE_GOLD=$enableval, >>  ENABLE_GOLD=no) >> -if test "${ENABLE_GOLD}" = "yes"; then >> +case "${ENABLE_GOLD}" in >> +yes|both) >>    # Check for ELF target. >>    is_elf=no >>    case "${target}" in >> @@ -334,11 +335,17 @@ if test "${ENABLE_GOLD}" = "yes"; then >>      # Check for target supported by gold. >>      case "${target}" in >>        i?86-*-* | x86_64-*-* | sparc*-*-* | powerpc*-*-* | arm*-*-*) >> -        configdirs="`echo " ${configdirs} " | sed -e 's/ ld / gold /'`" >> +        if test "${ENABLE_GOLD}" = both; then >> +          configdirs="$configdirs gold" >> +     else >> +          configdirs="`echo " ${configdirs} " | sed -e 's/ ld / gold /'`" >> +     fi >>          ;; >>      esac >>    fi >> -fi >> +  ENABLE_GOLD=yes >> +  ;; >> +esac > > You should have an error case here to ensure that --enable-gold only > has the values "yes", "both", or "no".  --enable-gold=foo should give > an error. > > This patch uses two configure options: one to build both gold and ld, > and one to select which linker is the default.  Why not use just one > configure option?  Perhaps something like: >    --enable-gold              Build only gold, gold is default >    --disable-gold [default]   Build only GNU ld, GNU ld is default >    --enable-gold=no           Same >    --enable-gold=both         Build both gold and GNU ld, gold is default >    --enable-gold=both/gold    Same >    --enable-gold=both/bfd     Build both gold and GNU ld, GNU ld is default Done. > But of course this approach is conditional on whether there should be > some way to select the linker to use at runtime. > > > >> diff --git a/gold/Makefile.am b/gold/Makefile.am >> index 8d8b617..85b103b 100644 >> --- a/gold/Makefile.am >> +++ b/gold/Makefile.am >> @@ -163,12 +163,20 @@ check: libgold.a >> >>  install-exec-local: ld-new$(EXEEXT) >>       $(mkinstalldirs) $(DESTDIR)$(bindir) $(DESTDIR)$(tooldir)/bin >> -     n=`echo ld | sed '$(transform)'`; \ >> +     n=`echo @installed_linker@ | sed '$(transform)'`; \ >>       $(INSTALL_PROGRAM) ld-new$(EXEEXT) $(DESTDIR)$(bindir)/$${n}$(EXEEXT); \ >> -     if test "$(bindir)" != "$(tooldir)/bin"; then \ >> -       rm -f $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT); \ >> -       ln $(DESTDIR)$(bindir)/$${n}$(EXEEXT) $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT) >/dev/null 2>/dev/null \ >> +     if test "@linker@" = "ld.gold"; then \ >> +       if test "@installed_linker@" != "ld"; then \ >> +         ld=`echo ld | sed '$(transform)'`; \ >> +         rm -f $(DESTDIR)$(bindir)/$${ld}$(EXEEXT); \ >> +         ln $(DESTDIR)$(bindir)/$${n}$(EXEEXT) $(DESTDIR)$(bindir)/$${ld}$(EXEEXT) >/dev/null 2>/dev/null \ >> +         || $(INSTALL_PROGRAM) ld-new$(EXEEXT) $(DESTDIR)$(bindir)/$${ld}$(EXEEXT); \ >> +       fi; \ >> +       if test "$(bindir)" != "$(tooldir)/bin"; then \ >> +         rm -f $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT); \ >> +         ln $(DESTDIR)$(bindir)/$${n}$(EXEEXT) $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT) >/dev/null 2>/dev/null \ >>           || $(INSTALL_PROGRAM) ld-new$(EXEEXT) $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT); \ >> +       fi; \ >>       fi > > In Makefile.am you don't need to use @@ quoting in rules.  You can > just use $(installed_linker) and $(linker). > Done. > >> diff --git a/gold/configure.ac b/gold/configure.ac >> index 85e23f9..10389a9 100644 >> --- a/gold/configure.ac >> +++ b/gold/configure.ac >> @@ -38,6 +38,29 @@ AC_DEFINE_UNQUOTED(TARGET_SYSTEM_ROOT, "$sysroot", >>  AC_DEFINE_UNQUOTED(TARGET_SYSTEM_ROOT_RELOCATABLE, $sysroot_relocatable, >>    [Whether the system root can be relocated]) >> >> +AC_ARG_ENABLE(gold, >> +[  --enable-gold[[=ARG]]     build gold [[ARG={yes,both}]]], >> +[if test "${enableval}" = "both"; then >> +   bfd_linker=ld.bfd >> +   installed_linker=ld.gold >> + else >> +   bfd_linker=ld.gold >> +   installed_linker=ld >> + fi], >> +[bfd_linker=ld.bfd >> + installed_linker=ld]) >> +AC_SUBST(installed_linker) > > It seems rather weird to set a variable named bfd_linker to be > ld.gold.  What does that mean? Rewritten. > >> +AC_ARG_ENABLE(linker, >> +[  --enable-linker=[[ARG]]   specify the default linker [[ARG={bfd,gold}]]], >> +[if test "${enableval}" = "gold"; then >> +   linker=ld.gold >> + else >> +   linker=$bfd_linker >> + fi], >> +[linker=$bfd_linker]) >> +AC_SUBST(linker) > > You should give an error if --enable-linker is used with an > unrecognized value. Done. > >> --- a/ld/Makefile.am >> +++ b/ld/Makefile.am >> @@ -95,7 +95,7 @@ CXX_FOR_TARGET = ` \ >>      fi; \ >>    fi` >> >> -transform = s/^ld-new$$/ld/;@program_transform_name@ >> +transform = s/^ld-new$$/@installed_linker@/;$(program_transform_name) >>  bin_PROGRAMS = ld-new >>  info_TEXINFOS = ld.texinfo >>  ld_TEXINFOS = configdoc.texi > > $(installed_linker).  Although actually as far as I can tell this line > is completely unnecessary.  Only constant strings are passed to > $(transform), and those constant strings never include ld-new. > I'd like to keep it in this patch. We can have a separate patch to remove it. >> -install-exec-local: ld-new$(EXEEXT) >> +install-exec-local: ld-new$(EXEEXT) install-binPROGRAMS >>       $(mkinstalldirs) $(DESTDIR)$(tooldir)/bin >> -     n=`echo ld | sed '$(transform)'`; \ >> -     if [ "$(bindir)/$$n$(EXEEXT)" != "$(tooldir)/bin/ld$(EXEEXT)" ]; then \ >> -       rm -f $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT); \ >> -       ln $(DESTDIR)$(bindir)/$$n$(EXEEXT) $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT) >/dev/null 2>/dev/null \ >> -       || $(LIBTOOL) --mode=install $(INSTALL_PROGRAM) ld-new$(EXEEXT) $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT); \ >> +     n=`echo @installed_linker@ | sed '$(transform)'`; \ >> +     if test "@linker@" = "ld.bfd"; then \ >> +       if test "@installed_linker@" != "ld"; then \ >> +         ld=`echo ld | sed '$(transform)'`; \ >> +         rm -f $(DESTDIR)$(bindir)/$$ld$(EXEEXT); \ >> +         ln $(DESTDIR)$(bindir)/$$n$(EXEEXT) $(DESTDIR)$(bindir)/$$ld$(EXEEXT) >/dev/null 2>/dev/null \ >> +         || $(LIBTOOL) --mode=install $(INSTALL_PROGRAM) ld-new$(EXEEXT) $(DESTDIR)$(bindir)/$$ld$(EXEEXT); \ >> +       fi; \ >> +       if test "$(bindir)/$$n$(EXEEXT)" != "$(tooldir)/bin/ld$(EXEEXT)"; then \ >> +         rm -f $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT); \ >> +         ln $(DESTDIR)$(bindir)/$$n$(EXEEXT) $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT) >/dev/null 2>/dev/null \ >> +         || $(LIBTOOL) --mode=install $(INSTALL_PROGRAM) ld-new$(EXEEXT) $(DESTDIR)$(tooldir)/bin/ld$(EXEEXT); \ >> +       fi; \ >>       fi > > Here also use $(VAR) rather than @VAR@. Done. > >> diff --git a/ld/configure.in b/ld/configure.in >> index c4655f5..9786953 100644 >> --- a/ld/configure.in >> +++ b/ld/configure.in >> @@ -69,6 +69,29 @@ AC_SUBST(use_sysroot) >>  AC_SUBST(TARGET_SYSTEM_ROOT) >>  AC_SUBST(TARGET_SYSTEM_ROOT_DEFINE) >> >> +AC_ARG_ENABLE(gold, >> +[  --enable-gold[[=ARG]]     build gold [[ARG={yes,both}]]], >> +[if test "${enableval}" = "both"; then >> +  gold_linker=ld.gold >> +  installed_linker=ld.bfd >> +else >> +  gold_linker=ld.bfd >> +  installed_linker=ld >> +fi], >> +[gold_linker=ld.bfd >> + installed_linker=ld]) >> +AC_SUBST(installed_linker) > > How can gold_linker be set to ld.bfd? > Rewritten. Thanks. -- H.J. --- 2010-01-05 Roland McGrath H.J. Lu * configure.ac (--enable-gold): Support both, both/gold and both/bfd to add gold to configdirs without removing ld. * configure: Regenerated. gold/ 2010-01-05 H.J. Lu * Makefile.am (install-exec-local): Install as $(installed_linker). Install as ld if $(linker) == "ld.gold" and $(installed_linker) != "ld". * Makefile.in: Regenerated. * configure.ac (--enable-gold): Support both, both/gold and both/bfd. * configure: Regenerated. ld/ 2010-01-05 H.J. Lu * Makefile.am (transform): Install as $(installed_linker). (install-exec-local): Depend on install-binPROGRAMS. Install as $(installed_linker). Install as ld if $(linker) == "ld.bfd" and $(installed_linker) != "ld". * Makefile.in: Regenerated. * configure.in (--enable-gold): Support both, both/gold and both/bfd. * configure: Regenerated.