PATCH: Support --enable-gold=both --with-linker=[bfd|gold]

H.J. Lu hjl.tools@gmail.com
Wed Jan 6 04:23:00 GMT 2010


On Tue, Jan 5, 2010 at 11:08 AM, Ian Lance Taylor <iant@google.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> 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  <roland@redhat.com>
	    H.J. Lu  <hongjiu.lu@intel.com>

	* 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  <hongjiu.lu@intel.com>

	* 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  <hongjiu.lu@intel.com>

	* 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: binutils-gold-6.patch
Type: text/x-csrc
Size: 7764 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20100106/52526d6c/attachment.bin>


More information about the Binutils mailing list