This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Tue, Jan 5, 2010 at 8:23 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > 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. > Here is the updated patch to set ENABLE_GOLD to yes only if target is supported. Any comments? Thanks. -- H.J.
Attachment:
binutils-gold-7.patch
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |