diff mbox

Fix bootstrap with --disable-shared and linker plugin

Message ID 20110108203323.GD28524@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Jan. 8, 2011, 8:33 p.m. UTC
Hi,
my patch to enable linker plugin by default broke bootstrap with --disable-shared since we build
linker plugin as static library. This is pointless.
This patch arranges --enable-shared into lto-plugin conifgury same was as we already do for libiberty.

The patch was confirmed to fix the reported issue and I also tested it with --disable-shared C only
bootstrap.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	PR lto/47225
	* Makefile.def (lto-plugin module): Add @extra_host_lto_plugin_configure_flags.
	* configure.ac: Set extra_host_lto_plugin_configure_flags=--enable-shared
	when doing plugin.
	* Makefile.in: Regenerate.
	* configure: Regenerate.

Comments

Ralf Wildenhues Jan. 9, 2011, 7:10 a.m. UTC | #1
Hi Jan,

* Jan Hubicka wrote on Sat, Jan 08, 2011 at 09:33:23PM CET:
> my patch to enable linker plugin by default broke bootstrap with
> --disable-shared since we build linker plugin as static library. This
> is pointless.  This patch arranges --enable-shared into lto-plugin
> conifgury same was as we already do for libiberty.
> 
> The patch was confirmed to fix the reported issue and I also tested it
> with --disable-shared C only bootstrap.
> 
> Bootstrapped/regtested x86_64-linux, OK?

This kind of subverts the intention of --disable-shared disabling shared
library generation.  But there is precedent with libiberty; and one can
argue both to be internal libraries.  I assume there are other mechanisms
in place actually disabling lto-plugin on systems where shared libraries
are not supported at all?

Other than that, the patch seems OK to me.  Bugzilla seems down at the
moment so I cannot compare with the bug description.

Thanks,
Ralf

> 	PR lto/47225
> 	* Makefile.def (lto-plugin module): Add @extra_host_lto_plugin_configure_flags.
> 	* configure.ac: Set extra_host_lto_plugin_configure_flags=--enable-shared
> 	when doing plugin.
> 	* Makefile.in: Regenerate.
> 	* configure: Regenerate.
Paolo Bonzini Jan. 9, 2011, 12:27 p.m. UTC | #2
On 01/08/2011 09:33 PM, Jan Hubicka wrote:
> 	* Makefile.def (lto-plugin module): Add @extra_host_lto_plugin_configure_flags.
> 	* configure.ac: Set extra_host_lto_plugin_configure_flags=--enable-shared
> 	when doing plugin.

You can add --enable-shared directly in Makefile.def.

Otherwise ok.

Paolo
John Tytgat Jan. 9, 2011, 12:31 p.m. UTC | #3
In message <20110109071047.GE22586@gmx.de>
          Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:

> Hi Jan,
> 
> * Jan Hubicka wrote on Sat, Jan 08, 2011 at 09:33:23PM CET:
> > my patch to enable linker plugin by default broke bootstrap with
> > --disable-shared since we build linker plugin as static library. This
> > is pointless.  This patch arranges --enable-shared into lto-plugin
> > conifgury same was as we already do for libiberty.
> > 
> > The patch was confirmed to fix the reported issue and I also tested it
> > with --disable-shared C only bootstrap.
> > 
> > Bootstrapped/regtested x86_64-linux, OK?
> 
> This kind of subverts the intention of --disable-shared disabling shared
> library generation.  But there is precedent with libiberty; and one can
> argue both to be internal libraries.  I assume there are other mechanisms
> in place actually disabling lto-plugin on systems where shared libraries
> are not supported at all?

The usecase was building a cross-compiler with a target where shared
libraries where not desired but with lto enabled (latter only relevant
for the host).  Since r168593, the lto-plugin was only built as static
library when --disabled-shared was used when configuring the cross-compiler.

John.
Ralf Wildenhues Jan. 9, 2011, 12:47 p.m. UTC | #4
* John Tytgat wrote on Sun, Jan 09, 2011 at 01:31:55PM CET:
> Ralf Wildenhues wrote:
> > * Jan Hubicka wrote on Sat, Jan 08, 2011 at 09:33:23PM CET:
> > > my patch to enable linker plugin by default broke bootstrap with
> > > --disable-shared since we build linker plugin as static library. This
> > > is pointless.  This patch arranges --enable-shared into lto-plugin
> > > conifgury same was as we already do for libiberty.

> > This kind of subverts the intention of --disable-shared disabling shared
> > library generation.  But there is precedent with libiberty; and one can
> > argue both to be internal libraries.  I assume there are other mechanisms
> > in place actually disabling lto-plugin on systems where shared libraries
> > are not supported at all?
> 
> The usecase was building a cross-compiler with a target where shared
> libraries where not desired but with lto enabled (latter only relevant
> for the host).  Since r168593, the lto-plugin was only built as static
> library when --disabled-shared was used when configuring the cross-compiler.

Then with current trunk you could have used
  configure target_configargs=--disable-shared

instead of global --disable-shared.

Cheers,
Ralf
Jan Hubicka Jan. 9, 2011, 2:02 p.m. UTC | #5
> 
> This kind of subverts the intention of --disable-shared disabling shared
> library generation.  But there is precedent with libiberty; and one can
> argue both to be internal libraries.  I assume there are other mechanisms
> in place actually disabling lto-plugin on systems where shared libraries
> are not supported at all?

There is no point building plugin on hosts that do not support shared libraries
since the plugin can not be plugged into linker. 
In these cases we must arrange toplevel configury to not build the plugin at all
or we confuse the detection in GCC configure whether it should or should not
enable -fuse-linker-plugin by default.  That one is based on presence of lto-plugin
Makefile and support for --plugin in the linker command line.
(probably on hosts that do not support dynamic libraries the linker will not be configured
with plugin support, but for in-tree binutils we skip this test and do it solely on
presence of gold or resonably recent GNU LD).

Toplevel configure already contains tests whether the plugin makes sense for
host (enabling it for ELF and cygwin targets only) Is there easy test to see if
shared libs can be built we can put into toplevel configure?
> 
> Other than that, the patch seems OK to me.  Bugzilla seems down at the
> moment so I cannot compare with the bug description.

I will commit the updated patch simply adding --enable-shared to lto-plugin
configury as suggested by paolo once I finish the testing.
This will get us to better shape solving the bootstrap problems reported.

Honza
> 
> Thanks,
> Ralf
> 
> > 	PR lto/47225
> > 	* Makefile.def (lto-plugin module): Add @extra_host_lto_plugin_configure_flags.
> > 	* configure.ac: Set extra_host_lto_plugin_configure_flags=--enable-shared
> > 	when doing plugin.
> > 	* Makefile.in: Regenerate.
> > 	* configure: Regenerate.
Ralf Wildenhues Jan. 9, 2011, 2:54 p.m. UTC | #6
* Jan Hubicka wrote on Sun, Jan 09, 2011 at 03:02:56PM CET:
> > 
> > This kind of subverts the intention of --disable-shared disabling shared
> > library generation.  But there is precedent with libiberty; and one can
> > argue both to be internal libraries.  I assume there are other mechanisms
> > in place actually disabling lto-plugin on systems where shared libraries
> > are not supported at all?
> 
> There is no point building plugin on hosts that do not support shared
> libraries since the plugin can not be plugged into linker. 

Sure.  The question is whether --enable-lto should override
--disable-shared or vice versa.  I agree it is somewhat of a bikeshed
question, but it may be prudent to at least document the intended
semantics.

> Toplevel configure already contains tests whether the plugin makes sense for
> host (enabling it for ELF and cygwin targets only) Is there easy test to see if
> shared libs can be built we can put into toplevel configure?

Well, you can add AC_PROG_LIBTOOL to configure.ac and test
$enable_shared = no afterwards, but that's not what I'd call an "easy
and lightweight test".  ;-)  I'm also not convinced it's a good idea to
introduce at this point in development.

Cheers,
Ralf
John Tytgat Jan. 9, 2011, 4:26 p.m. UTC | #7
In message <20110109124753.GF17091@gmx.de>
          Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:

> * John Tytgat wrote on Sun, Jan 09, 2011 at 01:31:55PM CET:
> > The usecase was building a cross-compiler with a target where shared
> > libraries where not desired but with lto enabled (latter only relevant
> > for the host).  Since r168593, the lto-plugin was only built as static
> > library when --disabled-shared was used when configuring the cross-compiler.
> 
> Then with current trunk you could have used
>   configure target_configargs=--disable-shared
> 
> instead of global --disable-shared.

Excellent, I didn't know about this feature and that looks to me like the
right approach for my cross-compiler usecase.  I've just tried it and
it works.

Thanks,
John.
Jan Hubicka Jan. 9, 2011, 5:59 p.m. UTC | #8
> * Jan Hubicka wrote on Sun, Jan 09, 2011 at 03:02:56PM CET:
> > > 
> > > This kind of subverts the intention of --disable-shared disabling shared
> > > library generation.  But there is precedent with libiberty; and one can
> > > argue both to be internal libraries.  I assume there are other mechanisms
> > > in place actually disabling lto-plugin on systems where shared libraries
> > > are not supported at all?
> > 
> > There is no point building plugin on hosts that do not support shared
> > libraries since the plugin can not be plugged into linker. 
> 
> Sure.  The question is whether --enable-lto should override
> --disable-shared or vice versa.  I agree it is somewhat of a bikeshed
> question, but it may be prudent to at least document the intended
> semantics.

Still the decision needs to be done at toplevel Makefile.  We might opt --disable-shared
to disable lto-plugin too, but it seems that users get this often wrong.

> 
> > Toplevel configure already contains tests whether the plugin makes sense for
> > host (enabling it for ELF and cygwin targets only) Is there easy test to see if
> > shared libs can be built we can put into toplevel configure?
> 
> Well, you can add AC_PROG_LIBTOOL to configure.ac and test
> $enable_shared = no afterwards, but that's not what I'd call an "easy
> and lightweight test".  ;-)  I'm also not convinced it's a good idea to
> introduce at this point in development.

Hmm, heavyweight indeed, but what are the risks of this solution?

Honza
> 
> Cheers,
> Ralf
Ralf Wildenhues Jan. 9, 2011, 6:45 p.m. UTC | #9
* Jan Hubicka wrote on Sun, Jan 09, 2011 at 06:59:40PM CET:
> > 
> > Sure.  The question is whether --enable-lto should override
> > --disable-shared or vice versa.  I agree it is somewhat of a bikeshed
> > question, but it may be prudent to at least document the intended
> > semantics.
> 
> Still the decision needs to be done at toplevel Makefile.  We might opt --disable-shared
> to disable lto-plugin too, but it seems that users get this often wrong.

OK, that seems a good reason then.

> > > Toplevel configure already contains tests whether the plugin makes sense for
> > > host (enabling it for ELF and cygwin targets only) Is there easy test to see if
> > > shared libs can be built we can put into toplevel configure?
> > 
> > Well, you can add AC_PROG_LIBTOOL to configure.ac and test
> > $enable_shared = no afterwards, but that's not what I'd call an "easy
> > and lightweight test".  ;-)  I'm also not convinced it's a good idea to
> > introduce at this point in development.
> 
> Hmm, heavyweight indeed, but what are the risks of this solution?

Not too high I think.  But placement is important, toplevel configure.ac
and Libtool macros will both try to set LD, AR, DLLTOOL, NM, RANLIB,
STRIP, OBJDUMP, and the configure.ac settings need to take precedence.

Cheers,
Ralf
Jan Hubicka Jan. 9, 2011, 10:36 p.m. UTC | #10
> * Jan Hubicka wrote on Sun, Jan 09, 2011 at 06:59:40PM CET:
> > > 
> > > Sure.  The question is whether --enable-lto should override
> > > --disable-shared or vice versa.  I agree it is somewhat of a bikeshed
> > > question, but it may be prudent to at least document the intended
> > > semantics.
> > 
> > Still the decision needs to be done at toplevel Makefile.  We might opt --disable-shared
> > to disable lto-plugin too, but it seems that users get this often wrong.
> 
> OK, that seems a good reason then.
> 
> > > > Toplevel configure already contains tests whether the plugin makes sense for
> > > > host (enabling it for ELF and cygwin targets only) Is there easy test to see if
> > > > shared libs can be built we can put into toplevel configure?
> > > 
> > > Well, you can add AC_PROG_LIBTOOL to configure.ac and test
> > > $enable_shared = no afterwards, but that's not what I'd call an "easy
> > > and lightweight test".  ;-)  I'm also not convinced it's a good idea to
> > > introduce at this point in development.
> > 
> > Hmm, heavyweight indeed, but what are the risks of this solution?
> 
> Not too high I think.  But placement is important, toplevel configure.ac
> and Libtool macros will both try to set LD, AR, DLLTOOL, NM, RANLIB,
> STRIP, OBJDUMP, and the configure.ac settings need to take precedence.

Hmm, do you think you could try to prepare patch?  I have no idea what
placement in configure.ac would make sense.
Currently we do:
ACX_ELF_TARGET_IFELSE([# ELF platforms build the lto-plugin always.
  build_lto_plugin=yes
],[if test x"$default_enable_lto" = x"yes" ; then
    case $target in
      *-apple-darwin* | *-cygwin* | *-mingw*) ;;
      # On other non-ELF platforms, LTO has yet to be validated.
      *) enable_lto=no ;;
    esac
  else
  # Apart from ELF platforms, only Windows and Darwin support LTO so far.
  # It would also be nice to check the binutils support, but we don't
  # have gcc_GAS_CHECK_FEATURE available here.  For now, we'll just
  # warn during gcc/ subconfigure; unless you're bootstrapping with
  # -flto it won't be needed until after installation anyway.
    case $target in
      *-cygwin* | *-mingw* | *-apple-darwin*) ;;
      *) if test x"$enable_lto" = x"yes"; then
        AC_MSG_ERROR([LTO support is not enabled for this target.])
        fi
      ;;
    esac
  fi
  # Among non-ELF, only Windows platforms support the lto-plugin so far.
  # Build it unless LTO was explicitly disabled.
  case $target in
    *-cygwin* | *-mingw*) build_lto_plugin=$enable_lto ;;
    *) ;;
  esac
])
I guess it is place where we want to add check if host is capable of building
shared library...

It is still possible to build libker plugin without plugin enabled LD and I guess
such configuration makes sort of sense.  GCC configure checks if the default linker
has linker support and set -fuse-linker-pugin default accordingly. But when plugin
is build it must be plugin-nable into the LD - there is nothing to check this.

Thanks!
Honza
> 
> Cheers,
> Ralf
John Tytgat Jan. 10, 2011, 12:53 a.m. UTC | #11
In message <4d95f59251.Jo@hobbes.bass-software.com>
          John Tytgat <John.Tytgat@aaug.net> wrote:

> In message <20110109124753.GF17091@gmx.de>
>           Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:
> 
> > * John Tytgat wrote on Sun, Jan 09, 2011 at 01:31:55PM CET:
> > > The usecase was building a cross-compiler with a target where shared
> > > libraries where not desired but with lto enabled (latter only relevant
> > > for the host).  Since r168593, the lto-plugin was only built as static
> > > library when --disabled-shared was used when configuring the cross-compiler.
> > 
> > Then with current trunk you could have used
> >   configure target_configargs=--disable-shared
> > 
> > instead of global --disable-shared.
> 
> Excellent, I didn't know about this feature and that looks to me like the
> right approach for my cross-compiler usecase.  I've just tried it and
> it works.

Actually I declared victory too soon : when using this cross-compiler
build, linking fails because it tries to link with libgcc_s.so which
I don't have built (nor want).

The reason is that my target declares SHLIB_* entries for its t-* file
and that we have in gcc/Makefile.in:

DRIVER_DEFINES = \
  -DSTANDARD_STARTFILE_PREFIX=\"$(unlibsubdir)/\" \
  -DSTANDARD_EXEC_PREFIX=\"$(libdir)/gcc/\" \
  -DSTANDARD_LIBEXEC_PREFIX=\"$(libexecdir)/gcc/\" \
  -DDEFAULT_TARGET_VERSION=\"$(version)\" \
  -DDEFAULT_TARGET_MACHINE=\"$(target_noncanonical)\" \
  -DSTANDARD_BINDIR_PREFIX=\"$(bindir)/\" \
  -DTOOLDIR_BASE_PREFIX=\"$(libsubdir_to_prefix)$(prefix_to_exec_prefix)\" \
  @TARGET_SYSTEM_ROOT_DEFINE@ \
  $(VALGRIND_DRIVER_DEFINES) \
  `test "X$${SHLIB_LINK}" = "X" || test "@enable_shared@" != "yes" || echo "-DENABLE_SHARED_LIBGCC"` \
  -DCONFIGURE_SPECS="\"@CONFIGURE_SPECS@\""

Because of Ralf's suggestion, I no longer give --disable-shared but
target_configargs=--disable-shared so the 2nd test fails now and
ENABLE_SHARED_LIBGCC gets defined so my link spec specifies libgcc_s.so
to be used.

As gcc/configure.ac's --disable-shared option gets described as "don't
provide a shared libgcc", it looks to me that any
--enable-shared/--disable-shared option specified in target_configargs
should be used to pass on to gcc/configure.ac by preference of any
--enable-shared/--disable-shared configure.ac option.

Does that make sense ?

John.
diff mbox

Patch

Index: Makefile.def
===================================================================
--- Makefile.def	(revision 168596)
+++ Makefile.def	(working copy)
@@ -145,7 +145,8 @@  host_modules= { module= libtermcap; no_c
                 missing=maintainer-clean; };
 host_modules= { module= utils; no_check=true; };
 host_modules= { module= gnattools; };
-host_modules= { module= lto-plugin; bootstrap=true; };
+host_modules= { module= lto-plugin; bootstrap=true;
+		extra_configure_flags='@extra_host_lto_plugin_configure_flags@'; };
 
 target_modules = { module= libstdc++-v3;
 		   bootstrap=true;
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 168596)
+++ Makefile.in	(working copy)
@@ -45248,7 +45248,7 @@  configure-lto-plugin:
 	libsrcdir="$$s/lto-plugin"; \
 	$(SHELL) $${libsrcdir}/configure \
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
-	  --target=${target_alias} $${srcdiroption}  \
+	  --target=${target_alias} $${srcdiroption} @extra_host_lto_plugin_configure_flags@ \
 	  || exit 1
 @endif lto-plugin
 
@@ -45282,7 +45282,8 @@  configure-stage1-lto-plugin:
 	$(SHELL) $${libsrcdir}/configure \
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
 	  --target=${target_alias} $${srcdiroption} \
-	  $(STAGE1_CONFIGURE_FLAGS)
+	  $(STAGE1_CONFIGURE_FLAGS) \
+	  @extra_host_lto_plugin_configure_flags@
 @endif lto-plugin-bootstrap
 
 .PHONY: configure-stage2-lto-plugin maybe-configure-stage2-lto-plugin
@@ -45315,7 +45316,8 @@  configure-stage2-lto-plugin:
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
 	  --target=${target_alias} $${srcdiroption} \
 	  --with-build-libsubdir=$(HOST_SUBDIR) \
-	  $(STAGE2_CONFIGURE_FLAGS)
+	  $(STAGE2_CONFIGURE_FLAGS) \
+	  @extra_host_lto_plugin_configure_flags@
 @endif lto-plugin-bootstrap
 
 .PHONY: configure-stage3-lto-plugin maybe-configure-stage3-lto-plugin
@@ -45348,7 +45350,8 @@  configure-stage3-lto-plugin:
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
 	  --target=${target_alias} $${srcdiroption} \
 	  --with-build-libsubdir=$(HOST_SUBDIR) \
-	  $(STAGE3_CONFIGURE_FLAGS)
+	  $(STAGE3_CONFIGURE_FLAGS) \
+	  @extra_host_lto_plugin_configure_flags@
 @endif lto-plugin-bootstrap
 
 .PHONY: configure-stage4-lto-plugin maybe-configure-stage4-lto-plugin
@@ -45381,7 +45384,8 @@  configure-stage4-lto-plugin:
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
 	  --target=${target_alias} $${srcdiroption} \
 	  --with-build-libsubdir=$(HOST_SUBDIR) \
-	  $(STAGE4_CONFIGURE_FLAGS)
+	  $(STAGE4_CONFIGURE_FLAGS) \
+	  @extra_host_lto_plugin_configure_flags@
 @endif lto-plugin-bootstrap
 
 .PHONY: configure-stageprofile-lto-plugin maybe-configure-stageprofile-lto-plugin
@@ -45414,7 +45418,8 @@  configure-stageprofile-lto-plugin:
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
 	  --target=${target_alias} $${srcdiroption} \
 	  --with-build-libsubdir=$(HOST_SUBDIR) \
-	  $(STAGEprofile_CONFIGURE_FLAGS)
+	  $(STAGEprofile_CONFIGURE_FLAGS) \
+	  @extra_host_lto_plugin_configure_flags@
 @endif lto-plugin-bootstrap
 
 .PHONY: configure-stagefeedback-lto-plugin maybe-configure-stagefeedback-lto-plugin
@@ -45447,7 +45452,8 @@  configure-stagefeedback-lto-plugin:
 	  $(HOST_CONFIGARGS) --build=${build_alias} --host=${host_alias} \
 	  --target=${target_alias} $${srcdiroption} \
 	  --with-build-libsubdir=$(HOST_SUBDIR) \
-	  $(STAGEfeedback_CONFIGURE_FLAGS)
+	  $(STAGEfeedback_CONFIGURE_FLAGS) \
+	  @extra_host_lto_plugin_configure_flags@
 @endif lto-plugin-bootstrap
 
 
Index: configure
===================================================================
--- configure	(revision 168596)
+++ configure	(working copy)
@@ -640,6 +640,7 @@  CFLAGS_FOR_TARGET
 DEBUG_PREFIX_CFLAGS_FOR_TARGET
 SYSROOT_CFLAGS_FOR_TARGET
 stage1_languages
+extra_host_lto_plugin_configure_flags
 extra_host_libiberty_configure_flags
 clooginc
 clooglibs
@@ -6237,6 +6238,7 @@  if test -d ${srcdir}/gcc; then
 
   # If LTO is enabled, add the LTO front end.
   extra_host_libiberty_configure_flags=
+  extra_host_lto_plugin_configure_flags=
   if test "$enable_lto" = "yes" ; then
     case ,${enable_languages}, in
       *,lto,*) ;;
@@ -6245,10 +6247,12 @@  if test -d ${srcdir}/gcc; then
     if test "${build_lto_plugin}" = "yes" ; then
       configdirs="$configdirs lto-plugin"
       extra_host_libiberty_configure_flags=--enable-shared
+      extra_host_lto_plugin_configure_flags=--enable-shared
     fi
   fi
 
 
+
   missing_languages=`echo ",$enable_languages," | sed -e s/,all,/,/ -e s/,c,/,/ `
   potential_languages=,c,
 
Index: configure.ac
===================================================================
--- configure.ac	(revision 168596)
+++ configure.ac	(working copy)
@@ -1810,6 +1810,7 @@  if test -d ${srcdir}/gcc; then
 
   # If LTO is enabled, add the LTO front end.
   extra_host_libiberty_configure_flags=
+  extra_host_lto_plugin_configure_flags=
   if test "$enable_lto" = "yes" ; then
     case ,${enable_languages}, in
       *,lto,*) ;;
@@ -1818,9 +1819,11 @@  if test -d ${srcdir}/gcc; then
     if test "${build_lto_plugin}" = "yes" ; then
       configdirs="$configdirs lto-plugin"
       extra_host_libiberty_configure_flags=--enable-shared
+      extra_host_lto_plugin_configure_flags=--enable-shared
     fi
   fi
   AC_SUBST(extra_host_libiberty_configure_flags)
+  AC_SUBST(extra_host_lto_plugin_configure_flags)
 
   missing_languages=`echo ",$enable_languages," | sed -e s/,all,/,/ -e s/,c,/,/ `
   potential_languages=,c,