[build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

The new gcc.dg/lto/20110201-1_0.c test fails on Solaris 2 with Sun ld
(SPARC and x86, 32-bit only):

FAIL: gcc.dg/lto/20110201-1 c_lto_20110201-1_0.o-c_lto_20110201-1_0.o link,  -O0 -flto 
UNRESOLVED: gcc.dg/lto/20110201-1 c_lto_20110201-1_0.o-c_lto_20110201-1_0.o execute  -O0 -flto 

output is:
Undefined			first referenced
 symbol  			    in file
cabs                                c_lto_20110201-1_0.o
ld: fatal: symbol referencing errors. No output written to gcc-dg-lto-20110201-1-01

This is another instance of PR lto/46944: the dg-require-linker-plugin
test assumes that the linker in use supports -plugin when one can
compile a simple test program with -fuse-linker-plugin.  Unfortunately,
this is wrong: while Sun ld doesn't support -plugin, it does have a
-p <auditlib> option, thus treats -plugin as -p lugin and interprets the
next arg as linker input file.  For 32-bit, this seems to work, for a
64-bit link, the linker errors out since it expects 64-bit input files
while lto-plugin.so is a 32-bit shared object, thus the
dg-require-linker-plugin concludes that -plugin support is absent.

It always seemed bogus to me to accept/pass on a linker switch without
checking if the linker in use really supports it, and this failure
finally prompted me to develop a real check.  Obviously, one need not
only test if the linker accepts -plugin, but also if the plugin passed
is really used.  Otherwise, gcc -fuse-linker-plugin will now error out,
as well as collect2 -plugin.

I'm uncertain if the configure check below is acceptable, though: I do
need to build a shared object and cannot use libtool in
gcc/configure.ac.  Therefore I rely on gcc -shared, which only works in
stage2 and up, or if gcc is used as the bootstrap compiler.

The following patch was bootstrapped without regressions on
i386-pc-solaris2.11 both with Sun ld and gld 2.21.  As expected, with
Sun ld it fixed the gcc.dg/lto/20110201-1_0.c failure above, but with
GNU ld, the test still doesn't work.  This is obviously part of the
plugin enhancements that only went into binutils mainline so far (and
will perhaps be backported to 2.21.1).  I'm undecided what's the best
way to handle this: while one could certainly add something like
dg-require-enhanced-linker-plugin or something, this seems
overcomplicated to me.  I'd rather have us require versions of gld or
gold that fully work with -plugin (which ones do?), not only in a subset
of cases, and reject -fuse-linker-plugin if the linker version is too
old.  Everthing else will just confuse users.

Apart from this patch, there's also gfortran.dg/lto/pr41764_0.f which
could use dg-require-linker-plugin, but currently doesn't since the
basic -plugin support in gold doesn't work for it.  If we follow my
suggestion above, the patch could be made to use
dg-require-linker-plugin.  I'll post a followup if we go the route
above.

Ok for mainline?

	Rainer


2011-02-05  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	gcc:
	PR lto/46944
	* configure.ac (ld_plugin): Check for real ld -plugin support
	(HAVE_LD_PLUGIN): Define.
	* configure: Regenerate.
	* config.in: Regenerate.
	* gcc.c (PLUGIN_COND): Only define if HAVE_LD_PLUGIN.
	(PLUGIN_COND_CLOSE): Likewise.
	LINK_PLUGIN_SPEC: Define
	(LINK_COMMAND_SPEC): Use it.
	* collect2.c (main) [!HAVE_LD_PLUGIN]: Error on -plugin.

diff -r b84c1dcc61f1 gcc/collect2.c
--- a/gcc/collect2.c	Sat Feb 05 09:17:53 2011 +0100
+++ b/gcc/collect2.c	Sat Feb 05 15:12:39 2011 +0100
@@ -1,7 +1,7 @@
 /* Collect static initialization info into data structures that can be
    traversed by C++ initialization and finalization routines.
    Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997, 1998,
-   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010
+   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010, 2011
    Free Software Foundation, Inc.
    Contributed by Chris Smith (csmith@convex.com).
    Heavily modified by Michael Meissner (meissner@cygnus.com),
@@ -1215,8 +1215,12 @@
 	  lto_mode = LTO_MODE_NONE;
         else if (! strcmp (argv[i], "-plugin"))
 	  {
+#ifdef HAVE_LD_PLUGIN
 	    use_plugin = true;
 	    lto_mode = LTO_MODE_NONE;
+#else
+	    error ("linker doesn't support -plugin");
+#endif
 	  }
 #ifdef COLLECT_EXPORT_LIST
 	/* since -brtl, -bexport, -b64 are not position dependent
diff -r b84c1dcc61f1 gcc/configure.ac
--- a/gcc/configure.ac	Sat Feb 05 09:17:53 2011 +0100
+++ b/gcc/configure.ac	Sat Feb 05 15:12:39 2011 +0100
@@ -4771,6 +4771,27 @@
 
 if test x"$enable_plugin" = x"yes"; then
 
+  AC_MSG_CHECKING([for -plugin])
+  ld_plugin=no
+  echo 'int main() {return 0;}' > conftest.c
+  ${CC} ${CFLAGS} -c conftest.c > /dev/null 2>&1
+  # Check if plugin is really used.
+  echo 'int onload () {puts ("yes"); return 0;}' > confplugin.c
+  # This depends on gcc, otherwise would have to use libtool.
+  if ${CC} ${CFLAGS} ${LDFLAGS} -shared -static-libgcc -fPIC \
+      -o confplugin.so confplugin.c > /dev/null 2>&1 \
+    && test x$gcc_cv_ld != x \
+    && ld_plugin_out=`$gcc_cv_ld -plugin ./confplugin.so \
+      -o conftest conftest.o -lc 2>/dev/null` \
+    && test x$ld_plugin_out = xyes; then
+    ld_plugin=yes
+  fi
+  if test x$ld_plugin = xyes; then
+    AC_DEFINE(HAVE_LD_PLUGIN, 1,
+      [Define if your linker supports -plugin.])
+  fi
+  AC_MSG_RESULT([$ld_plugin])
+
   AC_MSG_CHECKING([for exported symbols])
   if test "x$export_sym_check" != x; then
     echo "int main() {return 0;} int foobar() {return 0;}" > conftest.c
@@ -4834,7 +4855,7 @@
     if test x"$default_plugin" != x"yes"; then
       AC_MSG_ERROR([
 Building GCC with plugin support requires a host that supports
--fPIC, -shared, -ldl and -rdynamic.])
+-plugin, -fPIC, -shared, -ldl and -rdynamic.])
     fi
   fi
 fi
diff -r b84c1dcc61f1 gcc/gcc.c
--- a/gcc/gcc.c	Sat Feb 05 09:17:53 2011 +0100
+++ b/gcc/gcc.c	Sat Feb 05 15:12:39 2011 +0100
@@ -627,6 +627,7 @@
    not useable.  For GCC 4.6 we don't support slim LTO and thus we can enable
    plugin only when LTO is enabled.  We still honor explicit
    -fuse-linker-plugin.  */
+#ifdef HAVE_LD_PLUGIN
 #ifdef HAVE_LTO_PLUGIN
 #define PLUGIN_COND "!fno-use-linker-plugin:%{flto|flto=*|fuse-linker-plugin"
 #define PLUGIN_COND_CLOSE "}"
@@ -634,7 +635,17 @@
 #define PLUGIN_COND "fuse-linker-plugin"
 #define PLUGIN_COND_CLOSE ""
 #endif
-
+#define LINK_PLUGIN_SPEC \
+    "%{"PLUGIN_COND": \
+    -plugin %(linker_plugin_file) \
+    -plugin-opt=%(lto_wrapper) \
+    -plugin-opt=-fresolution=%u.res \
+    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
+    }"PLUGIN_COND_CLOSE
+#else
+#define LINK_PLUGIN_SPEC "%{fuse-linker-plugin:\
+    %e-fuse-linker-plugin is not supported in this configuration}"
+#endif
 
 /* -u* was put back because both BSD and SysV seem to support it.  */
 /* %{static:} simply prevents an error message if the target machine
@@ -647,14 +658,9 @@
 #ifndef LINK_COMMAND_SPEC
 #define LINK_COMMAND_SPEC "\
 %{!fsyntax-only:%{!c:%{!M:%{!MM:%{!E:%{!S:\
-    %(linker) \
-    %{"PLUGIN_COND": \
-    -plugin %(linker_plugin_file) \
-    -plugin-opt=%(lto_wrapper) \
-    -plugin-opt=-fresolution=%u.res \
-    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
-    }"PLUGIN_COND_CLOSE" \
-    %{flto|flto=*:%<fcompare-debug*} \
+    %(linker) " \
+    LINK_PLUGIN_SPEC \
+    "%{flto|flto=*:%<fcompare-debug*} \
     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
    "%X %{o*} %{e*} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}}\


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Ralf Wildenhues]

Hello Rainer,

Sorry for the delay, I somehow missed this mail.

* Rainer Orth wrote on Mon, Feb 07, 2011 at 01:11:39PM CET:
> This is another instance of PR lto/46944: the dg-require-linker-plugin
> test assumes that the linker in use supports -plugin when one can
> compile a simple test program with -fuse-linker-plugin.  Unfortunately,
> this is wrong: while Sun ld doesn't support -plugin, it does have a
> -p <auditlib> option, thus treats -plugin as -p lugin and interprets the
> next arg as linker input file.  For 32-bit, this seems to work, for a
> 64-bit link, the linker errors out since it expects 64-bit input files
> while lto-plugin.so is a 32-bit shared object, thus the
> dg-require-linker-plugin concludes that -plugin support is absent.
> 
> It always seemed bogus to me to accept/pass on a linker switch without
> checking if the linker in use really supports it, and this failure
> finally prompted me to develop a real check.  Obviously, one need not
> only test if the linker accepts -plugin, but also if the plugin passed
> is really used.  Otherwise, gcc -fuse-linker-plugin will now error out,
> as well as collect2 -plugin.
> 
> I'm uncertain if the configure check below is acceptable, though: I do
> need to build a shared object and cannot use libtool in
> gcc/configure.ac.

You can.  Put
  LT_OUTPUT

in configure.ac before the code that uses './libtool'.
(It would need to be cleaned up from Makefile.in.)

When using libtool you need to ensure that you get all flags you want to
test past libtool: prepend them with '-Wc,'.

I'm not quite sure which compilers this test needs to work with.
It looks however like your patch will not work on some systems
even with GCC, see below for details.

> Therefore I rely on gcc -shared, which only works in
> stage2 and up, or if gcc is used as the bootstrap compiler.

> Ok for mainline?

I'm afraid not.  I only review the build system bits.

> 2011-02-05  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	gcc:
> 	PR lto/46944
> 	* configure.ac (ld_plugin): Check for real ld -plugin support
> 	(HAVE_LD_PLUGIN): Define.
> 	* configure: Regenerate.
> 	* config.in: Regenerate.
> 	* gcc.c (PLUGIN_COND): Only define if HAVE_LD_PLUGIN.
> 	(PLUGIN_COND_CLOSE): Likewise.
> 	LINK_PLUGIN_SPEC: Define
> 	(LINK_COMMAND_SPEC): Use it.
> 	* collect2.c (main) [!HAVE_LD_PLUGIN]: Error on -plugin.

> --- a/gcc/configure.ac	Sat Feb 05 09:17:53 2011 +0100
> +++ b/gcc/configure.ac	Sat Feb 05 15:12:39 2011 +0100
> @@ -4771,6 +4771,27 @@
>  
>  if test x"$enable_plugin" = x"yes"; then
>  
> +  AC_MSG_CHECKING([for -plugin])
> +  ld_plugin=no
> +  echo 'int main() {return 0;}' > conftest.c
> +  ${CC} ${CFLAGS} -c conftest.c > /dev/null 2>&1

Could add $CPPFLAGS too.  I've seen -m32 in there before.

Dropping error output and not showing the commands used in config.log is
fairly ugly for debugging.  It is more helpful to use AC_COMPILE_IFELSE
here.  Below, it is even more important, as the commands are likely to
fail.  The above could simply be
  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [])],
    [ yep that worked, let's try the rest in here.
      ...
    ])

> +  # Check if plugin is really used.
> +  echo 'int onload () {puts ("yes"); return 0;}' > confplugin.c
> +  # This depends on gcc, otherwise would have to use libtool.
> +  if ${CC} ${CFLAGS} ${LDFLAGS} -shared -static-libgcc -fPIC \

Likewise for CPPFLAGS.

This will likely fail on MinGW due to the .so name, and anyway when
cross-compiling.

This might fail on Darwin unless you replace -shared with what the other
test nearby does modifying LDFLAGS.

> +      -o confplugin.so confplugin.c > /dev/null 2>&1 \
> +    && test x$gcc_cv_ld != x \
> +    && ld_plugin_out=`$gcc_cv_ld -plugin ./confplugin.so \
> +      -o conftest conftest.o -lc 2>/dev/null` \

I don't quite understand how this test is supposed to work in a cross
compile setup.  You compile something with $CC and try to load it with
$gcc_cv_ld, but IIUC you are using the test result for the compiler that
you are generating.  What am I missing?  Or is this test simply
irrelevant in cross compile mode anyway?

> +    && test x$ld_plugin_out = xyes; then
> +    ld_plugin=yes
> +  fi

rm -f confplugin.* conftest.$ac_objext conftest.c

> +  if test x$ld_plugin = xyes; then
> +    AC_DEFINE(HAVE_LD_PLUGIN, 1,
> +      [Define if your linker supports -plugin.])
> +  fi
> +  AC_MSG_RESULT([$ld_plugin])
> +
>    AC_MSG_CHECKING([for exported symbols])
>    if test "x$export_sym_check" != x; then
>      echo "int main() {return 0;} int foobar() {return 0;}" > conftest.c
> @@ -4834,7 +4855,7 @@
>      if test x"$default_plugin" != x"yes"; then
>        AC_MSG_ERROR([
>  Building GCC with plugin support requires a host that supports
> --fPIC, -shared, -ldl and -rdynamic.])
> +-plugin, -fPIC, -shared, -ldl and -rdynamic.])
>      fi
>    fi
>  fi

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

After considerable discussion in this thread

	Unreviewed build, lto patch
        http://gcc.gnu.org/ml/gcc-patches/2011-02/msg00868.html

Richard's recommendation was to only use the lto-plugin with known-good
linkers supporting -plugin, i.e. GNU ld or gold >= 2.21.

I'm extracting the binutils version number from gold --version since
it's more familiar than the separate gold version number and provides a
higher resolution than the gold version number that only changes once
per binutils release.

Here's the reworked patch that implements this suggestion.  I couldn't
check with gold yet since it doesn't work on Solaris so far, but
i386-pc-solaris2.11 bootstraps with Sun ld and GNU ld 2.21 work as
expected, i.e. the failure described in PR lto/46944 is gone.

Ok for mainline?

	Rainer


2011-02-05  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	PR lto/46944
	* configure.ac (gcc_cv_gld_major_version, gcc_cv_gld_minor):
	Handle in-tree gold.
	(ld_vers): Extract binutils version for gold.
	(gcc_cv_ld_hidden): Handle gold here.
	(gcc_cv_lto_plugin): Require gld or gold 2.21.
	* configure: Regenerate.
	* gcc.c [HAVE_LTO_PLUGIN] (PLUGIN_COND, PLUGIN_COND_CLOSE): Remove.
	(LINK_PLUGIN_SPEC): Define.
	Extract from LINK_COMMAND_SPEC, integrate PLUGIN_COND,
	PLUGIN_COND_CLOSE.
	[!HAVE_LTO_PLUGIN] (LINK_PLUGIN_SPEC): Define, reject
	-fuse-linker-plugin.
	(LINK_COMMAND_SPEC): Use it.

diff -r bbab4a602b6f gcc/configure.ac
--- a/gcc/configure.ac	Sat Feb 26 09:27:25 2011 +0100
+++ b/gcc/configure.ac	Sat Mar 05 09:36:08 2011 +0100
@@ -1967,7 +1967,8 @@
 esac 
 
 AC_MSG_CHECKING(what linker to use)
-if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext; then
+if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext \
+   || test "$gcc_cv_ld" = ../gold/ld-new$build_exeext; then
 	# Single tree build which includes ld.  We want to prefer it
 	# over whatever linker top-level may have detected, since
 	# we'll use what we're building after installation anyway.
@@ -1978,6 +1979,8 @@
 	    || grep 'EMUL = .*linux' ../ld/Makefile \
 	    || grep 'EMUL = .*lynx' ../ld/Makefile) > /dev/null; then
 	  in_tree_ld_is_elf=yes
+	elif test "$ld_is_gold" = yes; then
+	  in_tree_ld_is_elf=yes
 	fi
 	for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in
 	do
@@ -2192,11 +2195,23 @@
 changequote(,)dnl
 if test $in_tree_ld != yes ; then
   ld_ver=`$gcc_cv_ld --version 2>/dev/null | sed 1q`
-  if test x"$ld_is_gold" = xyes; then
-    gcc_cv_ld_hidden=yes
-  elif echo "$ld_ver" | grep GNU > /dev/null; then
-    ld_vers=`echo $ld_ver | sed -n \
-	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
+  if echo "$ld_ver" | grep GNU > /dev/null; then
+    if test x"$ld_is_gold" = xyes; then
+      # GNU gold --version looks like this:
+      #
+      # GNU gold (GNU Binutils 2.21.51.20110225) 1.11
+      #
+      # We extract the binutils version which is more familiar and specific
+      # than the gold version.
+      ld_vers=`echo $ld_ver | sed -n \
+	  -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)) .*$,\1,p'`
+    else
+      # GNU ld --version looks like this:
+      #
+      # GNU ld (GNU Binutils) 2.21.51.20110225
+      ld_vers=`echo $ld_ver | sed -n \
+	  -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
+    fi
     ld_date=`echo $ld_ver | sed -n 's,^.*\([2-9][0-9][0-9][0-9]\)[-]*\([01][0-9]\)[-]*\([0-3][0-9]\).*$,\1\2\3,p'`
     ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
     ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
@@ -2235,7 +2250,9 @@
   fi
 else
   gcc_cv_ld_hidden=yes
-  if echo "$ld_ver" | grep GNU > /dev/null; then
+  if test x"$ld_is_gold" = xyes; then
+    :
+  elif echo "$ld_ver" | grep GNU > /dev/null; then
     if test 0"$ld_date" -lt 20020404; then
       if test -n "$ld_date"; then
 	# If there was date string, but was earlier than 2002-04-04, fail
@@ -3176,14 +3193,14 @@
 gcc_cv_lto_plugin=no
 if test -f liblto_plugin.la; then
   if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET=" = x"$gcc_cv_ld"; then
-    if test x"$ld_is_gold" = xyes; then
-      gcc_cv_lto_plugin=yes
-    elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then \
+    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
       gcc_cv_lto_plugin=yes
     fi
-  # Check if the linker supports --plugin-opt option
-  elif $ORIGINAL_PLUGIN_LD_FOR_TARGET --help 2>/dev/null | grep plugin-opt > /dev/null; then
-    gcc_cv_lto_plugin=yes
+  elif echo "$ld_ver" | grep GNU > /dev/null; then
+    # Require GNU ld or gold 2.21 for plugin support.
+    if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
+      gcc_cv_lto_plugin=yes
+    fi
   fi
 fi
 if test x"$gcc_cv_lto_plugin" = xyes; then
diff -r bbab4a602b6f gcc/gcc.c
--- a/gcc/gcc.c	Sat Feb 26 09:27:25 2011 +0100
+++ b/gcc/gcc.c	Sat Mar 05 09:36:08 2011 +0100
@@ -623,16 +623,20 @@
 
 /* Conditional to test whether plugin is used or not.
    FIXME: For slim LTO we will need to enable plugin unconditionally.  This
-   still cause problems with PLUGIN_LD != LD and when plugin is built but
+   still causes problems with PLUGIN_LD != LD and when plugin is built but
    not useable.  For GCC 4.6 we don't support slim LTO and thus we can enable
-   plugin only when LTO is enabled.  We still honor explicit
-   -fuse-linker-plugin.  */
+   plugin only when LTO is enabled.  */
 #ifdef HAVE_LTO_PLUGIN
-#define PLUGIN_COND "!fno-use-linker-plugin:%{flto|flto=*|fuse-linker-plugin"
-#define PLUGIN_COND_CLOSE "}"
+#define LINK_PLUGIN_SPEC \
+    "%{!fno-use-linker-plugin:%{flto|flto=*|fuse-linker-plugin: \
+    -plugin %(linker_plugin_file) \
+    -plugin-opt=%(lto_wrapper) \
+    -plugin-opt=-fresolution=%u.res \
+    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
+    }}"
 #else
-#define PLUGIN_COND "fuse-linker-plugin"
-#define PLUGIN_COND_CLOSE ""
+#define LINK_PLUGIN_SPEC "%{fuse-linker-plugin:\
+    %e-fuse-linker-plugin is not supported in this configuration}"
 #endif
 
 
@@ -648,14 +652,9 @@
 #ifndef LINK_COMMAND_SPEC
 #define LINK_COMMAND_SPEC "\
 %{!fsyntax-only:%{!c:%{!M:%{!MM:%{!E:%{!S:\
-    %(linker) \
-    %{"PLUGIN_COND": \
-    -plugin %(linker_plugin_file) \
-    -plugin-opt=%(lto_wrapper) \
-    -plugin-opt=-fresolution=%u.res \
-    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
-    }"PLUGIN_COND_CLOSE" \
-    %{flto|flto=*:%<fcompare-debug*} \
+    %(linker) " \
+    LINK_PLUGIN_SPEC \
+    "%{flto|flto=*:%<fcompare-debug*} \
     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
    "%X %{o*} %{e*} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}}\


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Paolo Bonzini]

On 03/10/2011 12:26 PM, Rainer Orth wrote:
> +      # GNU gold (GNU Binutils 2.21.51.20110225) 1.11
> +      #
> +      # We extract the binutils version which is more familiar and specific
> +      # than the gold version.
> +      ld_vers=`echo $ld_ver | sed -n \
> +	  -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)) .*$,\1,p'`
                  ^^                                 ^^

Perhaps changing these to [^)]* is better.  Build parts are okay with 
that change.

Patch

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Thu, 10 Mar 2011, Rainer Orth wrote:

> After considerable discussion in this thread
> 
> 	Unreviewed build, lto patch
>         http://gcc.gnu.org/ml/gcc-patches/2011-02/msg00868.html
> 
> Richard's recommendation was to only use the lto-plugin with known-good
> linkers supporting -plugin, i.e. GNU ld or gold >= 2.21.
> 
> I'm extracting the binutils version number from gold --version since
> it's more familiar than the separate gold version number and provides a
> higher resolution than the gold version number that only changes once
> per binutils release.
> 
> Here's the reworked patch that implements this suggestion.  I couldn't
> check with gold yet since it doesn't work on Solaris so far, but
> i386-pc-solaris2.11 bootstraps with Sun ld and GNU ld 2.21 work as
> expected, i.e. the failure described in PR lto/46944 is gone.
> 
> Ok for mainline?

If I read this patch correctly then

 1) it doesn't change the condition under which lto-plugin/ is built
    (good)

 2) it makes -fuse-linker-plugin the default for and only for
    known good linkers (GNU binutils >= 2.21) (good)

 3) it makes it impossible to use -fuse-linker-plugin explicitly
    for other linkers or linkers that were not installed during
    configuring gcc (bad - esp. the latter)

can you please try avoiding 3) at this stage?  Or is the whole
point of this patch 3) to be able to fix PR46944?

Ideally we'd reject broken linkers at runtime, but that would
require some major collect2 massaging (eventually falling back
to collect2 or simply reporting an error).

That said, I'm not 100% happy with 3) at this point (though
2) is very desirable).

Can we to fix 46944 change the dejagnu require-linker-plugin
to check if a linker plugin is used by default and not add
-fuse-linker-plugin?

Thanks,
Richard.

> 	Rainer
> 
> 
> 2011-02-05  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	PR lto/46944
> 	* configure.ac (gcc_cv_gld_major_version, gcc_cv_gld_minor):
> 	Handle in-tree gold.
> 	(ld_vers): Extract binutils version for gold.
> 	(gcc_cv_ld_hidden): Handle gold here.
> 	(gcc_cv_lto_plugin): Require gld or gold 2.21.
> 	* configure: Regenerate.
> 	* gcc.c [HAVE_LTO_PLUGIN] (PLUGIN_COND, PLUGIN_COND_CLOSE): Remove.
> 	(LINK_PLUGIN_SPEC): Define.
> 	Extract from LINK_COMMAND_SPEC, integrate PLUGIN_COND,
> 	PLUGIN_COND_CLOSE.
> 	[!HAVE_LTO_PLUGIN] (LINK_PLUGIN_SPEC): Define, reject
> 	-fuse-linker-plugin.
> 	(LINK_COMMAND_SPEC): Use it.
> 
> diff -r bbab4a602b6f gcc/configure.ac
> --- a/gcc/configure.ac	Sat Feb 26 09:27:25 2011 +0100
> +++ b/gcc/configure.ac	Sat Mar 05 09:36:08 2011 +0100
> @@ -1967,7 +1967,8 @@
>  esac 
>  
>  AC_MSG_CHECKING(what linker to use)
> -if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext; then
> +if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext \
> +   || test "$gcc_cv_ld" = ../gold/ld-new$build_exeext; then
>  	# Single tree build which includes ld.  We want to prefer it
>  	# over whatever linker top-level may have detected, since
>  	# we'll use what we're building after installation anyway.
> @@ -1978,6 +1979,8 @@
>  	    || grep 'EMUL = .*linux' ../ld/Makefile \
>  	    || grep 'EMUL = .*lynx' ../ld/Makefile) > /dev/null; then
>  	  in_tree_ld_is_elf=yes
> +	elif test "$ld_is_gold" = yes; then
> +	  in_tree_ld_is_elf=yes
>  	fi
>  	for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in
>  	do
> @@ -2192,11 +2195,23 @@
>  changequote(,)dnl
>  if test $in_tree_ld != yes ; then
>    ld_ver=`$gcc_cv_ld --version 2>/dev/null | sed 1q`
> -  if test x"$ld_is_gold" = xyes; then
> -    gcc_cv_ld_hidden=yes
> -  elif echo "$ld_ver" | grep GNU > /dev/null; then
> -    ld_vers=`echo $ld_ver | sed -n \
> -	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
> +  if echo "$ld_ver" | grep GNU > /dev/null; then
> +    if test x"$ld_is_gold" = xyes; then
> +      # GNU gold --version looks like this:
> +      #
> +      # GNU gold (GNU Binutils 2.21.51.20110225) 1.11
> +      #
> +      # We extract the binutils version which is more familiar and specific
> +      # than the gold version.
> +      ld_vers=`echo $ld_ver | sed -n \
> +	  -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)) .*$,\1,p'`
> +    else
> +      # GNU ld --version looks like this:
> +      #
> +      # GNU ld (GNU Binutils) 2.21.51.20110225
> +      ld_vers=`echo $ld_ver | sed -n \
> +	  -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
> +    fi
>      ld_date=`echo $ld_ver | sed -n 's,^.*\([2-9][0-9][0-9][0-9]\)[-]*\([01][0-9]\)[-]*\([0-3][0-9]\).*$,\1\2\3,p'`
>      ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
>      ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
> @@ -2235,7 +2250,9 @@
>    fi
>  else
>    gcc_cv_ld_hidden=yes
> -  if echo "$ld_ver" | grep GNU > /dev/null; then
> +  if test x"$ld_is_gold" = xyes; then
> +    :
> +  elif echo "$ld_ver" | grep GNU > /dev/null; then
>      if test 0"$ld_date" -lt 20020404; then
>        if test -n "$ld_date"; then
>  	# If there was date string, but was earlier than 2002-04-04, fail
> @@ -3176,14 +3193,14 @@
>  gcc_cv_lto_plugin=no
>  if test -f liblto_plugin.la; then
>    if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET=" = x"$gcc_cv_ld"; then
> -    if test x"$ld_is_gold" = xyes; then
> -      gcc_cv_lto_plugin=yes
> -    elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then \
> +    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
>        gcc_cv_lto_plugin=yes
>      fi
> -  # Check if the linker supports --plugin-opt option
> -  elif $ORIGINAL_PLUGIN_LD_FOR_TARGET --help 2>/dev/null | grep plugin-opt > /dev/null; then
> -    gcc_cv_lto_plugin=yes
> +  elif echo "$ld_ver" | grep GNU > /dev/null; then
> +    # Require GNU ld or gold 2.21 for plugin support.
> +    if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
> +      gcc_cv_lto_plugin=yes
> +    fi
>    fi
>  fi
>  if test x"$gcc_cv_lto_plugin" = xyes; then
> diff -r bbab4a602b6f gcc/gcc.c
> --- a/gcc/gcc.c	Sat Feb 26 09:27:25 2011 +0100
> +++ b/gcc/gcc.c	Sat Mar 05 09:36:08 2011 +0100
> @@ -623,16 +623,20 @@
>  
>  /* Conditional to test whether plugin is used or not.
>     FIXME: For slim LTO we will need to enable plugin unconditionally.  This
> -   still cause problems with PLUGIN_LD != LD and when plugin is built but
> +   still causes problems with PLUGIN_LD != LD and when plugin is built but
>     not useable.  For GCC 4.6 we don't support slim LTO and thus we can enable
> -   plugin only when LTO is enabled.  We still honor explicit
> -   -fuse-linker-plugin.  */
> +   plugin only when LTO is enabled.  */
>  #ifdef HAVE_LTO_PLUGIN
> -#define PLUGIN_COND "!fno-use-linker-plugin:%{flto|flto=*|fuse-linker-plugin"
> -#define PLUGIN_COND_CLOSE "}"
> +#define LINK_PLUGIN_SPEC \
> +    "%{!fno-use-linker-plugin:%{flto|flto=*|fuse-linker-plugin: \
> +    -plugin %(linker_plugin_file) \
> +    -plugin-opt=%(lto_wrapper) \
> +    -plugin-opt=-fresolution=%u.res \
> +    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
> +    }}"
>  #else
> -#define PLUGIN_COND "fuse-linker-plugin"
> -#define PLUGIN_COND_CLOSE ""
> +#define LINK_PLUGIN_SPEC "%{fuse-linker-plugin:\
> +    %e-fuse-linker-plugin is not supported in this configuration}"
>  #endif
>  
>  
> @@ -648,14 +652,9 @@
>  #ifndef LINK_COMMAND_SPEC
>  #define LINK_COMMAND_SPEC "\
>  %{!fsyntax-only:%{!c:%{!M:%{!MM:%{!E:%{!S:\
> -    %(linker) \
> -    %{"PLUGIN_COND": \
> -    -plugin %(linker_plugin_file) \
> -    -plugin-opt=%(lto_wrapper) \
> -    -plugin-opt=-fresolution=%u.res \
> -    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
> -    }"PLUGIN_COND_CLOSE" \
> -    %{flto|flto=*:%<fcompare-debug*} \
> +    %(linker) " \
> +    LINK_PLUGIN_SPEC \
> +    "%{flto|flto=*:%<fcompare-debug*} \
>      %{flto} %{flto=*} %l " LINK_PIE_SPEC \
>     "%X %{o*} %{e*} %{N} %{n} %{r}\
>      %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}}\
> 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

> If I read this patch correctly then
>
>  1) it doesn't change the condition under which lto-plugin/ is built
>     (good)

Right.

>  2) it makes -fuse-linker-plugin the default for and only for
>     known good linkers (GNU binutils >= 2.21) (good)

Indeed.

>  3) it makes it impossible to use -fuse-linker-plugin explicitly
>     for other linkers or linkers that were not installed during
>     configuring gcc (bad - esp. the latter)
>
> can you please try avoiding 3) at this stage?  Or is the whole
> point of this patch 3) to be able to fix PR46944?

That was my goal: the Solaris linker accepts -p <auditlib>, which causes
confusion when it is called with -plugin.

> Ideally we'd reject broken linkers at runtime, but that would
> require some major collect2 massaging (eventually falling back
> to collect2 or simply reporting an error).

What about making LTO_PLUGIN 3-valued?

2	linker used has full -plugin support, i.e. gld/gold >= 2.21
1       linker has some -plugin support, i.e. gold >= 2.20 < 2.21
0       linker has no known plugin support, i.e. everything else, in
        particular vendor linkers

We'd default to -fuse-linker-plugin for 2, accept it if given explicitly
for 1, and reject it for 0.

This would be similar to the first version of my patch, with the
difference that we don't try to determine the level of -plugin support
from trying to run the configured linker with -plugin and check if it
works, but instead hardcode that knowledge.

> That said, I'm not 100% happy with 3) at this point (though
> 2) is very desirable).
>
> Can we to fix 46944 change the dejagnu require-linker-plugin
> to check if a linker plugin is used by default and not add
> -fuse-linker-plugin?

That might be involved since it requires parsing gcc -Wl,-debug output.
I suppose the solution outlined above is simpler and thus more robust.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Thu, 10 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> > If I read this patch correctly then
> >
> >  1) it doesn't change the condition under which lto-plugin/ is built
> >     (good)
> 
> Right.
> 
> >  2) it makes -fuse-linker-plugin the default for and only for
> >     known good linkers (GNU binutils >= 2.21) (good)
> 
> Indeed.
> 
> >  3) it makes it impossible to use -fuse-linker-plugin explicitly
> >     for other linkers or linkers that were not installed during
> >     configuring gcc (bad - esp. the latter)
> >
> > can you please try avoiding 3) at this stage?  Or is the whole
> > point of this patch 3) to be able to fix PR46944?
> 
> That was my goal: the Solaris linker accepts -p <auditlib>, which causes
> confusion when it is called with -plugin.
> 
> > Ideally we'd reject broken linkers at runtime, but that would
> > require some major collect2 massaging (eventually falling back
> > to collect2 or simply reporting an error).
> 
> What about making LTO_PLUGIN 3-valued?
> 
> 2	linker used has full -plugin support, i.e. gld/gold >= 2.21
> 1       linker has some -plugin support, i.e. gold >= 2.20 < 2.21
> 0       linker has no known plugin support, i.e. everything else, in
>         particular vendor linkers
> 
> We'd default to -fuse-linker-plugin for 2, accept it if given explicitly
> for 1, and reject it for 0.
> 
> This would be similar to the first version of my patch, with the
> difference that we don't try to determine the level of -plugin support
> from trying to run the configured linker with -plugin and check if it
> works, but instead hardcode that knowledge.
> 
> > That said, I'm not 100% happy with 3) at this point (though
> > 2) is very desirable).
> >
> > Can we to fix 46944 change the dejagnu require-linker-plugin
> > to check if a linker plugin is used by default and not add
> > -fuse-linker-plugin?
> 
> That might be involved since it requires parsing gcc -Wl,-debug output.
> I suppose the solution outlined above is simpler and thus more robust.

It might be as simple as

int res;
int main() { int x; asm ("mov res, %0" : x(g)); return x; }

which should fail to link with the plugin only (but yes, requies
target dependent assembly ...).

Or, use -save-temps and check for the existence of the resolution
file for int main() {}.  It should be named t.res for gcc -o t t.c -flto.

Richard.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

>> > Can we to fix 46944 change the dejagnu require-linker-plugin
>> > to check if a linker plugin is used by default and not add
>> > -fuse-linker-plugin?
>> 
>> That might be involved since it requires parsing gcc -Wl,-debug output.
>> I suppose the solution outlined above is simpler and thus more robust.
>
> It might be as simple as
>
> int res;
> int main() { int x; asm ("mov res, %0" : x(g)); return x; }
>
> which should fail to link with the plugin only (but yes, requies
> target dependent assembly ...).

... which I'd consider far too complicated/hard to maintain to consider.

> Or, use -save-temps and check for the existence of the resolution
> file for int main() {}.  It should be named t.res for gcc -o t t.c -flto.

Only with -save-temps, otherwise it's some random file in /var/tmp.  But
even so the file is removed immediately.

And even if we decide to fix PR lto/46944 like this, we're still left
with the problem of users invoking gcc with -fuse-linker-plugin and
getting either strange errors or no effect instead of a clear
diagnostic.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Thu, 10 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> >> > Can we to fix 46944 change the dejagnu require-linker-plugin
> >> > to check if a linker plugin is used by default and not add
> >> > -fuse-linker-plugin?
> >> 
> >> That might be involved since it requires parsing gcc -Wl,-debug output.
> >> I suppose the solution outlined above is simpler and thus more robust.
> >
> > It might be as simple as
> >
> > int res;
> > int main() { int x; asm ("mov res, %0" : x(g)); return x; }
> >
> > which should fail to link with the plugin only (but yes, requies
> > target dependent assembly ...).
> 
> ... which I'd consider far too complicated/hard to maintain to consider.
> 
> > Or, use -save-temps and check for the existence of the resolution
> > file for int main() {}.  It should be named t.res for gcc -o t t.c -flto.
> 
> Only with -save-temps, otherwise it's some random file in /var/tmp.  But
> even so the file is removed immediately.

The following seems to work for me

Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp       (revision 170868)
+++ gcc/testsuite/lib/target-supports.exp       (working copy)
@@ -1011,9 +1011,20 @@ proc check_effective_target_static_libgf
 }
 
 proc check_linker_plugin_available { } {
-  return [check_no_compiler_messages_nocache linker_plugin executable {
+  set result [eval check_compile { linker_plugin object {
      int main() { return 0; }
-  } "-flto -fuse-linker-plugin"]
+  } "-flto" } ]
+  set lines [lindex $result 0]
+  set output [lindex $result 1]
+  set result [gcc_target_compile $output linker_plugin[pid] executable { 
additional_flags=-flto additional_flags=-flto-partition=none 
additional_flags=-save-temps } ]
+  remote_file build delete $output
+  remote_file build delete linker_plugin[pid]
+  remote_file build delete linker_plugin[pid].s
+  if [file exists linker_plugin[pid].res] {
+    remote_file build delete linker_plugin[pid].res
+    return 1
+  }
+  return 0
 }
 
 # Return 1 if the target supports executing 750CL paired-single 
instructions, 0

it leaves an argument file in /tmp but otherwise nothing.

Eventually scanning -v output for '-plugin' is better.

> And even if we decide to fix PR lto/46944 like this, we're still left
> with the problem of users invoking gcc with -fuse-linker-plugin and
> getting either strange errors or no effect instead of a clear
> diagnostic.

Sure.  But just disabling linker-plugin support for almost everyone
doesn't sound like a good solution either.

I'm ok with fixing 46944 somehow at this point, as well as making
-fuse-linker-plugin the default only for known good linker versions.

Richard.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

>> Only with -save-temps, otherwise it's some random file in /var/tmp.  But
>> even so the file is removed immediately.
>
> The following seems to work for me
>
> Index: gcc/testsuite/lib/target-supports.exp
> ===================================================================
> --- gcc/testsuite/lib/target-supports.exp       (revision 170868)
> +++ gcc/testsuite/lib/target-supports.exp       (working copy)
> @@ -1011,9 +1011,20 @@ proc check_effective_target_static_libgf
>  }
>  
>  proc check_linker_plugin_available { } {
> -  return [check_no_compiler_messages_nocache linker_plugin executable {
> +  set result [eval check_compile { linker_plugin object {
>       int main() { return 0; }
> -  } "-flto -fuse-linker-plugin"]
> +  } "-flto" } ]
> +  set lines [lindex $result 0]
> +  set output [lindex $result 1]
> +  set result [gcc_target_compile $output linker_plugin[pid] executable { 
> additional_flags=-flto additional_flags=-flto-partition=none 
> additional_flags=-save-temps } ]
> +  remote_file build delete $output
> +  remote_file build delete linker_plugin[pid]
> +  remote_file build delete linker_plugin[pid].s
> +  if [file exists linker_plugin[pid].res] {
> +    remote_file build delete linker_plugin[pid].res
> +    return 1
> +  }
> +  return 0
>  }

Still doesn't work for me if compiling and linking manually with GNU ld
2.21 on Solaris 11/x86: no .res file is being created, although
liblto_plugin.so is used.

> Eventually scanning -v output for '-plugin' is better.

This can only work if we make sure that -plugin is only passed to
linkers that properly handle it.

>> And even if we decide to fix PR lto/46944 like this, we're still left
>> with the problem of users invoking gcc with -fuse-linker-plugin and
>> getting either strange errors or no effect instead of a clear
>> diagnostic.
>
> Sure.  But just disabling linker-plugin support for almost everyone
> doesn't sound like a good solution either.

Why do you say so?  The tri-state solution I've outlined only disables it
completely for non-GNU linkers.  The plugin is used automatically for
gld/gold 2.21+ and for gold 2.20* if -fuse-linker-plugin is given.

I don't see the `almost everyone' here, on the contrary: the situation
is identical to what we have now, with the exception that we don't try
to pass -plugin to linkers we don't know they can somehow (even with
restrictions) handle it.  That's what PR lto/46944 is primarily about.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Fri, 11 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> >> Only with -save-temps, otherwise it's some random file in /var/tmp.  But
> >> even so the file is removed immediately.
> >
> > The following seems to work for me
> >
> > Index: gcc/testsuite/lib/target-supports.exp
> > ===================================================================
> > --- gcc/testsuite/lib/target-supports.exp       (revision 170868)
> > +++ gcc/testsuite/lib/target-supports.exp       (working copy)
> > @@ -1011,9 +1011,20 @@ proc check_effective_target_static_libgf
> >  }
> >  
> >  proc check_linker_plugin_available { } {
> > -  return [check_no_compiler_messages_nocache linker_plugin executable {
> > +  set result [eval check_compile { linker_plugin object {
> >       int main() { return 0; }
> > -  } "-flto -fuse-linker-plugin"]
> > +  } "-flto" } ]
> > +  set lines [lindex $result 0]
> > +  set output [lindex $result 1]
> > +  set result [gcc_target_compile $output linker_plugin[pid] executable { 
> > additional_flags=-flto additional_flags=-flto-partition=none 
> > additional_flags=-save-temps } ]
> > +  remote_file build delete $output
> > +  remote_file build delete linker_plugin[pid]
> > +  remote_file build delete linker_plugin[pid].s
> > +  if [file exists linker_plugin[pid].res] {
> > +    remote_file build delete linker_plugin[pid].res
> > +    return 1
> > +  }
> > +  return 0
> >  }
> 
> Still doesn't work for me if compiling and linking manually with GNU ld
> 2.21 on Solaris 11/x86: no .res file is being created, although
> liblto_plugin.so is used.

"Work" as in, it detects if -fuse-linker-plugin is enabled by default.
Which it isn't for you?

> > Eventually scanning -v output for '-plugin' is better.
> 
> This can only work if we make sure that -plugin is only passed to
> linkers that properly handle it.

Sure, but your version check patch part would ensure that, if not
overridden by -fuse-linker-plugin.

> >> And even if we decide to fix PR lto/46944 like this, we're still left
> >> with the problem of users invoking gcc with -fuse-linker-plugin and
> >> getting either strange errors or no effect instead of a clear
> >> diagnostic.
> >
> > Sure.  But just disabling linker-plugin support for almost everyone
> > doesn't sound like a good solution either.
> 
> Why do you say so?  The tri-state solution I've outlined only disables it
> completely for non-GNU linkers.  The plugin is used automatically for
> gld/gold 2.21+ and for gold 2.20* if -fuse-linker-plugin is given.
> 
> I don't see the `almost everyone' here, on the contrary: the situation
> is identical to what we have now, with the exception that we don't try
> to pass -plugin to linkers we don't know they can somehow (even with
> restrictions) handle it.  That's what PR lto/46944 is primarily about.

Can you update your patch with the tri-state solution?

Thanks,
Richard.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

>> Still doesn't work for me if compiling and linking manually with GNU ld
>> 2.21 on Solaris 11/x86: no .res file is being created, although
>> liblto_plugin.so is used.
>
> "Work" as in, it detects if -fuse-linker-plugin is enabled by default.
> Which it isn't for you?

I'm using gld 2.21, and -flto automatically uses the linker plugin, as
seen with -v.  Despite that, -plugin-opt=-fresolution=ldl.res is passed
to collect2/ld, but no ldl.res file is created.  In truss, I see a stat
of that file, but nothing more.

>> > Eventually scanning -v output for '-plugin' is better.
>> 
>> This can only work if we make sure that -plugin is only passed to
>> linkers that properly handle it.
>
> Sure, but your version check patch part would ensure that, if not
> overridden by -fuse-linker-plugin.

No, -fuse-linker-plugin wouldn't override here: that option is only
accepted if we know that the linker has *some* -plugin support.

>> Why do you say so?  The tri-state solution I've outlined only disables it
>> completely for non-GNU linkers.  The plugin is used automatically for
>> gld/gold 2.21+ and for gold 2.20* if -fuse-linker-plugin is given.
>> 
>> I don't see the `almost everyone' here, on the contrary: the situation
>> is identical to what we have now, with the exception that we don't try
>> to pass -plugin to linkers we don't know they can somehow (even with
>> restrictions) handle it.  That's what PR lto/46944 is primarily about.
>
> Can you update your patch with the tri-state solution?

Sure if the solution is deemed acceptable.  There isn't much point in
following that route if you see problems up front.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Fri, 11 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> >> Still doesn't work for me if compiling and linking manually with GNU ld
> >> 2.21 on Solaris 11/x86: no .res file is being created, although
> >> liblto_plugin.so is used.
> >
> > "Work" as in, it detects if -fuse-linker-plugin is enabled by default.
> > Which it isn't for you?
> 
> I'm using gld 2.21, and -flto automatically uses the linker plugin, as
> seen with -v.  Despite that, -plugin-opt=-fresolution=ldl.res is passed
> to collect2/ld, but no ldl.res file is created.  In truss, I see a stat
> of that file, but nothing more.

Interesting - it works for me with both GNU ld and gold from binutils 
2.21.

> >> > Eventually scanning -v output for '-plugin' is better.
> >> 
> >> This can only work if we make sure that -plugin is only passed to
> >> linkers that properly handle it.
> >
> > Sure, but your version check patch part would ensure that, if not
> > overridden by -fuse-linker-plugin.
> 
> No, -fuse-linker-plugin wouldn't override here: that option is only
> accepted if we know that the linker has *some* -plugin support.

Yeah, of course - that's desirable.

> >> Why do you say so?  The tri-state solution I've outlined only disables it
> >> completely for non-GNU linkers.  The plugin is used automatically for
> >> gld/gold 2.21+ and for gold 2.20* if -fuse-linker-plugin is given.
> >> 
> >> I don't see the `almost everyone' here, on the contrary: the situation
> >> is identical to what we have now, with the exception that we don't try
> >> to pass -plugin to linkers we don't know they can somehow (even with
> >> restrictions) handle it.  That's what PR lto/46944 is primarily about.
> >
> > Can you update your patch with the tri-state solution?
> 
> Sure if the solution is deemed acceptable.  There isn't much point in
> following that route if you see problems up front.

If that solution avoids 3) then yes, I'm fine with going that route.
Both 1) and 2) are very desirable anyway.

Thanks,
Richard.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

>> I'm using gld 2.21, and -flto automatically uses the linker plugin, as
>> seen with -v.  Despite that, -plugin-opt=-fresolution=ldl.res is passed
>> to collect2/ld, but no ldl.res file is created.  In truss, I see a stat
>> of that file, but nothing more.
>
> Interesting - it works for me with both GNU ld and gold from binutils 
> 2.21.

Strange indeed.  Maybe related to using xgcc -B./ from a build tree?
gold still doesn't fully work for me, perhaps it does for this example.
I'll give that a try too.

>> > Can you update your patch with the tri-state solution?
>> 
>> Sure if the solution is deemed acceptable.  There isn't much point in
>> following that route if you see problems up front.
>
> If that solution avoids 3) then yes, I'm fine with going that route.
> Both 1) and 2) are very desirable anyway.

Ok, I'll update the patch over the weekend.

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

>> > Can you update your patch with the tri-state solution?
>> 
>> Sure if the solution is deemed acceptable.  There isn't much point in
>> following that route if you see problems up front.
>
> If that solution avoids 3) then yes, I'm fine with going that route.
> Both 1) and 2) are very desirable anyway.

Here's the updated patch, which also incorporates Paolo's suggestion.
Bootstrapped without regressions on i386-pc-solaris2.11 with Sun as/ld
and GNU as/ld 2.21.  I couldn't really test the middle ground (gold 2.20
with limited plugin support) since even mainline gold doesn't work for
me yet.  What I've done is rebuild xgcc after manually changing
HAVE_LTO_PLUGIN to 1 in auto-host.h.  After I realized that I needed to
remove the existing specs file, that variant worked as expected in that
-plugin is only passed to the linker with an explicit
-fuse-linker-plugin.

I had to make a last-minute change when I realized that two other uses
of #ifdef HAVE_LTO_PLUGIN needed to be updated since that macro is now
always defined.  I'm running fresh bootstraps to make sure nothing broke
that way.

Ok for mainline if that passes, and perhaps also the 4.6 branch?

Thanks.
	Rainer


2011-02-05  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	PR lto/46944
	* configure.ac (gcc_cv_gld_major_version, gcc_cv_gld_minor):
	Handle in-tree gold.
	(ld_vers): Extract binutils version for gold.
	(gcc_cv_ld_hidden): Handle gold here.
	(gcc_cv_lto_plugin): Determine level of linker plugin support.
	* configure: Regenerate.
	* config.in: Regenerate.
	* gcc.c: Only use LTO plugin if HAVE_LTO_PLUGIN > 0, reject
	-fuse-linker-plugin otherwise.
	(LINK_PLUGIN_SPEC): Define.  Extract from LINK_COMMAND_SPEC.
	(LINK_COMMAND_SPEC): Use it.
	(main): Only look for LTOPLUGINSONAME if HAVE_LTO_PLUGIN > 0.

diff -r 71f0a0dc3338 gcc/configure.ac
--- a/gcc/configure.ac	Mon Mar 14 19:57:51 2011 +0100
+++ b/gcc/configure.ac	Mon Mar 14 19:59:05 2011 +0100
@@ -1967,7 +1967,8 @@
 esac 
 
 AC_MSG_CHECKING(what linker to use)
-if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext; then
+if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext \
+   || test "$gcc_cv_ld" = ../gold/ld-new$build_exeext; then
 	# Single tree build which includes ld.  We want to prefer it
 	# over whatever linker top-level may have detected, since
 	# we'll use what we're building after installation anyway.
@@ -1978,6 +1979,8 @@
 	    || grep 'EMUL = .*linux' ../ld/Makefile \
 	    || grep 'EMUL = .*lynx' ../ld/Makefile) > /dev/null; then
 	  in_tree_ld_is_elf=yes
+	elif test "$ld_is_gold" = yes; then
+	  in_tree_ld_is_elf=yes
 	fi
 	for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in
 	do
@@ -2192,11 +2195,23 @@
 changequote(,)dnl
 if test $in_tree_ld != yes ; then
   ld_ver=`$gcc_cv_ld --version 2>/dev/null | sed 1q`
-  if test x"$ld_is_gold" = xyes; then
-    gcc_cv_ld_hidden=yes
-  elif echo "$ld_ver" | grep GNU > /dev/null; then
-    ld_vers=`echo $ld_ver | sed -n \
-	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
+  if echo "$ld_ver" | grep GNU > /dev/null; then
+    if test x"$ld_is_gold" = xyes; then
+      # GNU gold --version looks like this:
+      #
+      # GNU gold (GNU Binutils 2.21.51.20110225) 1.11
+      #
+      # We extract the binutils version which is more familiar and specific
+      # than the gold version.
+      ld_vers=`echo $ld_ver | sed -n \
+	  -e 's,^[^)]*[	 ]\([0-9][0-9]*\.[0-9][0-9]*[^)]*\)) .*$,\1,p'`
+    else
+      # GNU ld --version looks like this:
+      #
+      # GNU ld (GNU Binutils) 2.21.51.20110225
+      ld_vers=`echo $ld_ver | sed -n \
+	  -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
+    fi
     ld_date=`echo $ld_ver | sed -n 's,^.*\([2-9][0-9][0-9][0-9]\)[-]*\([01][0-9]\)[-]*\([0-3][0-9]\).*$,\1\2\3,p'`
     ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
     ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
@@ -2235,7 +2250,9 @@
   fi
 else
   gcc_cv_ld_hidden=yes
-  if echo "$ld_ver" | grep GNU > /dev/null; then
+  if test x"$ld_is_gold" = xyes; then
+    :
+  elif echo "$ld_ver" | grep GNU > /dev/null; then
     if test 0"$ld_date" -lt 20020404; then
       if test -n "$ld_date"; then
 	# If there was date string, but was earlier than 2002-04-04, fail
@@ -3173,23 +3190,27 @@
 fi
 
 AC_MSG_CHECKING(linker plugin support)
-gcc_cv_lto_plugin=no
+gcc_cv_lto_plugin=0
 if test -f liblto_plugin.la; then
   if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET=" = x"$gcc_cv_ld"; then
-    if test x"$ld_is_gold" = xyes; then
-      gcc_cv_lto_plugin=yes
-    elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then \
-      gcc_cv_lto_plugin=yes
+    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
+      gcc_cv_lto_plugin=2
+    elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
+      gcc_cv_lto_plugin=1
+
     fi
-  # Check if the linker supports --plugin-opt option
-  elif $ORIGINAL_PLUGIN_LD_FOR_TARGET --help 2>/dev/null | grep plugin-opt > /dev/null; then
-    gcc_cv_lto_plugin=yes
+  elif echo "$ld_ver" | grep GNU > /dev/null; then
+    # Require GNU ld or gold 2.21+ for plugin support by default.
+    if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
+      gcc_cv_lto_plugin=2
+    # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.
+    elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
+      gcc_cv_lto_plugin=1
+    fi
   fi
 fi
-if test x"$gcc_cv_lto_plugin" = xyes; then
-  AC_DEFINE(HAVE_LTO_PLUGIN, 1,
-[Define if your linker supports plugin.])
-fi
+AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc_cv_lto_plugin,
+  [Define to the level of your linker's plugin support.])
 AC_MSG_RESULT($gcc_cv_lto_plugin)
 
 case "$target" in
diff -r 71f0a0dc3338 gcc/gcc.c
--- a/gcc/gcc.c	Mon Mar 14 19:57:51 2011 +0100
+++ b/gcc/gcc.c	Mon Mar 14 19:59:05 2011 +0100
@@ -621,19 +621,37 @@
 # endif
 #endif
 
-/* Conditional to test whether plugin is used or not.
+/* Conditional to test whether the LTO plugin is used or not.
    FIXME: For slim LTO we will need to enable plugin unconditionally.  This
    still cause problems with PLUGIN_LD != LD and when plugin is built but
    not useable.  For GCC 4.6 we don't support slim LTO and thus we can enable
    plugin only when LTO is enabled.  We still honor explicit
-   -fuse-linker-plugin.  */
-#ifdef HAVE_LTO_PLUGIN
+   -fuse-linker-plugin if the linker used understands -plugin.  */
+
+/* The linker has some plugin support.  */
+#if HAVE_LTO_PLUGIN > 0
+/* The linker used has full plugin support, use LTO plugin by default.  */
+#if HAVE_LTO_PLUGIN == 2
 #define PLUGIN_COND "!fno-use-linker-plugin:%{flto|flto=*|fuse-linker-plugin"
 #define PLUGIN_COND_CLOSE "}"
 #else
+/* The linker used has limited plugin support, use LTO plugin with explicit
+   -fuse-linker-plugin.  */
 #define PLUGIN_COND "fuse-linker-plugin"
 #define PLUGIN_COND_CLOSE ""
 #endif
+#define LINK_PLUGIN_SPEC \
+    "%{"PLUGIN_COND": \
+    -plugin %(linker_plugin_file) \
+    -plugin-opt=%(lto_wrapper) \
+    -plugin-opt=-fresolution=%u.res \
+    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
+    }"PLUGIN_COND_CLOSE
+#else
+/* The linker used doesn't support -plugin, reject -fuse-linker-plugin.  */
+#define LINK_PLUGIN_SPEC "%{fuse-linker-plugin:\
+    %e-fuse-linker-plugin is not supported in this configuration}"
+#endif
 
 
 /* -u* was put back because both BSD and SysV seem to support it.  */
@@ -648,14 +666,9 @@
 #ifndef LINK_COMMAND_SPEC
 #define LINK_COMMAND_SPEC "\
 %{!fsyntax-only:%{!c:%{!M:%{!MM:%{!E:%{!S:\
-    %(linker) \
-    %{"PLUGIN_COND": \
-    -plugin %(linker_plugin_file) \
-    -plugin-opt=%(lto_wrapper) \
-    -plugin-opt=-fresolution=%u.res \
-    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
-    }"PLUGIN_COND_CLOSE" \
-    %{flto|flto=*:%<fcompare-debug*} \
+    %(linker) " \
+    LINK_PLUGIN_SPEC \
+    "%{flto|flto=*:%<fcompare-debug*} \
     %{flto} %{flto=*} %l " LINK_PIE_SPEC \
    "%X %{o*} %{e*} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}}\
@@ -6815,11 +6828,13 @@
   if (num_linker_inputs > 0 && !seen_error () && print_subprocess_help < 2)
     {
       int tmp = execution_count;
-#ifdef HAVE_LTO_PLUGIN
+#if HAVE_LTO_PLUGIN > 0
+#if HAVE_LTO_PLUGIN == 2
       const char *fno_use_linker_plugin = "fno-use-linker-plugin";
 #else
       const char *fuse_linker_plugin = "fuse-linker-plugin";
 #endif
+#endif
 
       /* We'll use ld if we can't find collect2.  */
       if (! strcmp (linker_name_spec, "collect2"))
@@ -6829,7 +6844,8 @@
 	    linker_name_spec = "ld";
 	}
 
-#ifdef HAVE_LTO_PLUGIN
+#if HAVE_LTO_PLUGIN > 0
+#if HAVE_LTO_PLUGIN == 2
       if (!switch_matches (fno_use_linker_plugin,
 			   fno_use_linker_plugin + strlen (fno_use_linker_plugin), 0))
 #else
@@ -6843,6 +6859,7 @@
 	  if (!linker_plugin_file_spec)
 	    fatal_error ("-fuse-linker-plugin, but " LTOPLUGINSONAME " not found");
 	}
+#endif
       lto_gcc_spec = argv[0];
 
       /* Rebuild the COMPILER_PATH and LIBRARY_PATH environment variables


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Mon, 14 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> >> > Can you update your patch with the tri-state solution?
> >> 
> >> Sure if the solution is deemed acceptable.  There isn't much point in
> >> following that route if you see problems up front.
> >
> > If that solution avoids 3) then yes, I'm fine with going that route.
> > Both 1) and 2) are very desirable anyway.
> 
> Here's the updated patch, which also incorporates Paolo's suggestion.
> Bootstrapped without regressions on i386-pc-solaris2.11 with Sun as/ld
> and GNU as/ld 2.21.  I couldn't really test the middle ground (gold 2.20
> with limited plugin support) since even mainline gold doesn't work for
> me yet.  What I've done is rebuild xgcc after manually changing
> HAVE_LTO_PLUGIN to 1 in auto-host.h.  After I realized that I needed to
> remove the existing specs file, that variant worked as expected in that
> -plugin is only passed to the linker with an explicit
> -fuse-linker-plugin.
> 
> I had to make a last-minute change when I realized that two other uses
> of #ifdef HAVE_LTO_PLUGIN needed to be updated since that macro is now
> always defined.  I'm running fresh bootstraps to make sure nothing broke
> that way.
> 
> Ok for mainline if that passes, and perhaps also the 4.6 branch?

I'm ok with this for mainline - Paolo, can you double-check the
autofoo stuff?  As for 4.6 I'd like to give it a day or two on
trunk to allow people to report problems.

Thanks,
Richard.

> Thanks.
> 	Rainer
> 
> 
> 2011-02-05  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	PR lto/46944
> 	* configure.ac (gcc_cv_gld_major_version, gcc_cv_gld_minor):
> 	Handle in-tree gold.
> 	(ld_vers): Extract binutils version for gold.
> 	(gcc_cv_ld_hidden): Handle gold here.
> 	(gcc_cv_lto_plugin): Determine level of linker plugin support.
> 	* configure: Regenerate.
> 	* config.in: Regenerate.
> 	* gcc.c: Only use LTO plugin if HAVE_LTO_PLUGIN > 0, reject
> 	-fuse-linker-plugin otherwise.
> 	(LINK_PLUGIN_SPEC): Define.  Extract from LINK_COMMAND_SPEC.
> 	(LINK_COMMAND_SPEC): Use it.
> 	(main): Only look for LTOPLUGINSONAME if HAVE_LTO_PLUGIN > 0.
> 
> diff -r 71f0a0dc3338 gcc/configure.ac
> --- a/gcc/configure.ac	Mon Mar 14 19:57:51 2011 +0100
> +++ b/gcc/configure.ac	Mon Mar 14 19:59:05 2011 +0100
> @@ -1967,7 +1967,8 @@
>  esac 
>  
>  AC_MSG_CHECKING(what linker to use)
> -if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext; then
> +if test "$gcc_cv_ld" = ../ld/ld-new$build_exeext \
> +   || test "$gcc_cv_ld" = ../gold/ld-new$build_exeext; then
>  	# Single tree build which includes ld.  We want to prefer it
>  	# over whatever linker top-level may have detected, since
>  	# we'll use what we're building after installation anyway.
> @@ -1978,6 +1979,8 @@
>  	    || grep 'EMUL = .*linux' ../ld/Makefile \
>  	    || grep 'EMUL = .*lynx' ../ld/Makefile) > /dev/null; then
>  	  in_tree_ld_is_elf=yes
> +	elif test "$ld_is_gold" = yes; then
> +	  in_tree_ld_is_elf=yes
>  	fi
>  	for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in
>  	do
> @@ -2192,11 +2195,23 @@
>  changequote(,)dnl
>  if test $in_tree_ld != yes ; then
>    ld_ver=`$gcc_cv_ld --version 2>/dev/null | sed 1q`
> -  if test x"$ld_is_gold" = xyes; then
> -    gcc_cv_ld_hidden=yes
> -  elif echo "$ld_ver" | grep GNU > /dev/null; then
> -    ld_vers=`echo $ld_ver | sed -n \
> -	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
> +  if echo "$ld_ver" | grep GNU > /dev/null; then
> +    if test x"$ld_is_gold" = xyes; then
> +      # GNU gold --version looks like this:
> +      #
> +      # GNU gold (GNU Binutils 2.21.51.20110225) 1.11
> +      #
> +      # We extract the binutils version which is more familiar and specific
> +      # than the gold version.
> +      ld_vers=`echo $ld_ver | sed -n \
> +	  -e 's,^[^)]*[	 ]\([0-9][0-9]*\.[0-9][0-9]*[^)]*\)) .*$,\1,p'`
> +    else
> +      # GNU ld --version looks like this:
> +      #
> +      # GNU ld (GNU Binutils) 2.21.51.20110225
> +      ld_vers=`echo $ld_ver | sed -n \
> +	  -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
> +    fi
>      ld_date=`echo $ld_ver | sed -n 's,^.*\([2-9][0-9][0-9][0-9]\)[-]*\([01][0-9]\)[-]*\([0-3][0-9]\).*$,\1\2\3,p'`
>      ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
>      ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
> @@ -2235,7 +2250,9 @@
>    fi
>  else
>    gcc_cv_ld_hidden=yes
> -  if echo "$ld_ver" | grep GNU > /dev/null; then
> +  if test x"$ld_is_gold" = xyes; then
> +    :
> +  elif echo "$ld_ver" | grep GNU > /dev/null; then
>      if test 0"$ld_date" -lt 20020404; then
>        if test -n "$ld_date"; then
>  	# If there was date string, but was earlier than 2002-04-04, fail
> @@ -3173,23 +3190,27 @@
>  fi
>  
>  AC_MSG_CHECKING(linker plugin support)
> -gcc_cv_lto_plugin=no
> +gcc_cv_lto_plugin=0
>  if test -f liblto_plugin.la; then
>    if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET=" = x"$gcc_cv_ld"; then
> -    if test x"$ld_is_gold" = xyes; then
> -      gcc_cv_lto_plugin=yes
> -    elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then \
> -      gcc_cv_lto_plugin=yes
> +    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
> +      gcc_cv_lto_plugin=2
> +    elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
> +      gcc_cv_lto_plugin=1
> +
>      fi
> -  # Check if the linker supports --plugin-opt option
> -  elif $ORIGINAL_PLUGIN_LD_FOR_TARGET --help 2>/dev/null | grep plugin-opt > /dev/null; then
> -    gcc_cv_lto_plugin=yes
> +  elif echo "$ld_ver" | grep GNU > /dev/null; then
> +    # Require GNU ld or gold 2.21+ for plugin support by default.
> +    if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
> +      gcc_cv_lto_plugin=2
> +    # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.
> +    elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
> +      gcc_cv_lto_plugin=1
> +    fi
>    fi
>  fi
> -if test x"$gcc_cv_lto_plugin" = xyes; then
> -  AC_DEFINE(HAVE_LTO_PLUGIN, 1,
> -[Define if your linker supports plugin.])
> -fi
> +AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc_cv_lto_plugin,
> +  [Define to the level of your linker's plugin support.])
>  AC_MSG_RESULT($gcc_cv_lto_plugin)
>  
>  case "$target" in
> diff -r 71f0a0dc3338 gcc/gcc.c
> --- a/gcc/gcc.c	Mon Mar 14 19:57:51 2011 +0100
> +++ b/gcc/gcc.c	Mon Mar 14 19:59:05 2011 +0100
> @@ -621,19 +621,37 @@
>  # endif
>  #endif
>  
> -/* Conditional to test whether plugin is used or not.
> +/* Conditional to test whether the LTO plugin is used or not.
>     FIXME: For slim LTO we will need to enable plugin unconditionally.  This
>     still cause problems with PLUGIN_LD != LD and when plugin is built but
>     not useable.  For GCC 4.6 we don't support slim LTO and thus we can enable
>     plugin only when LTO is enabled.  We still honor explicit
> -   -fuse-linker-plugin.  */
> -#ifdef HAVE_LTO_PLUGIN
> +   -fuse-linker-plugin if the linker used understands -plugin.  */
> +
> +/* The linker has some plugin support.  */
> +#if HAVE_LTO_PLUGIN > 0
> +/* The linker used has full plugin support, use LTO plugin by default.  */
> +#if HAVE_LTO_PLUGIN == 2
>  #define PLUGIN_COND "!fno-use-linker-plugin:%{flto|flto=*|fuse-linker-plugin"
>  #define PLUGIN_COND_CLOSE "}"
>  #else
> +/* The linker used has limited plugin support, use LTO plugin with explicit
> +   -fuse-linker-plugin.  */
>  #define PLUGIN_COND "fuse-linker-plugin"
>  #define PLUGIN_COND_CLOSE ""
>  #endif
> +#define LINK_PLUGIN_SPEC \
> +    "%{"PLUGIN_COND": \
> +    -plugin %(linker_plugin_file) \
> +    -plugin-opt=%(lto_wrapper) \
> +    -plugin-opt=-fresolution=%u.res \
> +    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
> +    }"PLUGIN_COND_CLOSE
> +#else
> +/* The linker used doesn't support -plugin, reject -fuse-linker-plugin.  */
> +#define LINK_PLUGIN_SPEC "%{fuse-linker-plugin:\
> +    %e-fuse-linker-plugin is not supported in this configuration}"
> +#endif
>  
>  
>  /* -u* was put back because both BSD and SysV seem to support it.  */
> @@ -648,14 +666,9 @@
>  #ifndef LINK_COMMAND_SPEC
>  #define LINK_COMMAND_SPEC "\
>  %{!fsyntax-only:%{!c:%{!M:%{!MM:%{!E:%{!S:\
> -    %(linker) \
> -    %{"PLUGIN_COND": \
> -    -plugin %(linker_plugin_file) \
> -    -plugin-opt=%(lto_wrapper) \
> -    -plugin-opt=-fresolution=%u.res \
> -    %{!nostdlib:%{!nodefaultlibs:%:pass-through-libs(%(link_gcc_c_sequence))}} \
> -    }"PLUGIN_COND_CLOSE" \
> -    %{flto|flto=*:%<fcompare-debug*} \
> +    %(linker) " \
> +    LINK_PLUGIN_SPEC \
> +    "%{flto|flto=*:%<fcompare-debug*} \
>      %{flto} %{flto=*} %l " LINK_PIE_SPEC \
>     "%X %{o*} %{e*} %{N} %{n} %{r}\
>      %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}}\
> @@ -6815,11 +6828,13 @@
>    if (num_linker_inputs > 0 && !seen_error () && print_subprocess_help < 2)
>      {
>        int tmp = execution_count;
> -#ifdef HAVE_LTO_PLUGIN
> +#if HAVE_LTO_PLUGIN > 0
> +#if HAVE_LTO_PLUGIN == 2
>        const char *fno_use_linker_plugin = "fno-use-linker-plugin";
>  #else
>        const char *fuse_linker_plugin = "fuse-linker-plugin";
>  #endif
> +#endif
>  
>        /* We'll use ld if we can't find collect2.  */
>        if (! strcmp (linker_name_spec, "collect2"))
> @@ -6829,7 +6844,8 @@
>  	    linker_name_spec = "ld";
>  	}
>  
> -#ifdef HAVE_LTO_PLUGIN
> +#if HAVE_LTO_PLUGIN > 0
> +#if HAVE_LTO_PLUGIN == 2
>        if (!switch_matches (fno_use_linker_plugin,
>  			   fno_use_linker_plugin + strlen (fno_use_linker_plugin), 0))
>  #else
> @@ -6843,6 +6859,7 @@
>  	  if (!linker_plugin_file_spec)
>  	    fatal_error ("-fuse-linker-plugin, but " LTOPLUGINSONAME " not found");
>  	}
> +#endif
>        lto_gcc_spec = argv[0];
>  
>        /* Rebuild the COMPILER_PATH and LIBRARY_PATH environment variables
> 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

>> Ok for mainline if that passes, and perhaps also the 4.6 branch?
>
> I'm ok with this for mainline - Paolo, can you double-check the
> autofoo stuff?  As for 4.6 I'd like to give it a day or two on
> trunk to allow people to report problems.

Paolo, do you have any comments on the configure.ac part of the patch?

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Paolo Bonzini]

On 03/16/2011 10:23 AM, Rainer Orth wrote:
> >  I'm ok with this for mainline - Paolo, can you double-check the
> >  autofoo stuff?  As for 4.6 I'd like to give it a day or two on
> >  trunk to allow people to report problems.
>
> Paolo, do you have any comments on the configure.ac part of the patch?

It's okay.

I hope Binutils will never switch to 3.x, but that's not your fault.

Paolo

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Paolo Bonzini <bonzini@gnu.org> writes:

>> Paolo, do you have any comments on the configure.ac part of the patch?
>
> It's okay.

Thanks, installed on mainline then.

> I hope Binutils will never switch to 3.x, but that's not your fault.

True: although there are some precautions for binutils > 2, I doubt they
are consistent.  Only if we could deprecate all binutils 2.x releases,
the GCC configure could be considerably simplified.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

>> Ok for mainline if that passes, and perhaps also the 4.6 branch?
>
> I'm ok with this for mainline - Paolo, can you double-check the
> autofoo stuff?  As for 4.6 I'd like to give it a day or two on
> trunk to allow people to report problems.

The patch has been on mainline for two days now and I'm not aware of any
problems.  Unless someone objects, I'm going to apply it to 4.6 branch
later today.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Fri, Mar 18, 2011 at 10:26 AM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
> Richard Guenther <rguenther@suse.de> writes:
>
>>> Ok for mainline if that passes, and perhaps also the 4.6 branch?
>>
>> I'm ok with this for mainline - Paolo, can you double-check the
>> autofoo stuff?  As for 4.6 I'd like to give it a day or two on
>> trunk to allow people to report problems.
>
> The patch has been on mainline for two days now and I'm not aware of any
> problems.  Unless someone objects, I'm going to apply it to 4.6 branch
> later today.

It seemed to have disabled linker-plugin support for old binutils
with --with-plugin-ld=/usr/local/bin/gold, explicit -fuse-linker-plugin says
it is not supported.  The system linker does not have plugin support
(nor gold).  /usr/local has gold from binutils 2.20.50.xxx.  So it seems that

    # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.
    elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a
"$ld_vers_minor" -eq 20; then
      gcc_cv_lto_plugin=1
    fi

only allows explicit -fuse-linker-plugin for recent gold but based on checking
the system linker version (instead of that of --with-plugin-ld).  I'd say
allow explicit -fuse-linker-plugin always if --with-plugin-ld is specified.

Richard.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <richard.guenther@gmail.com> writes:

> It seemed to have disabled linker-plugin support for old binutils
> with --with-plugin-ld=/usr/local/bin/gold, explicit -fuse-linker-plugin says
> it is not supported.  The system linker does not have plugin support
> (nor gold).  /usr/local has gold from binutils 2.20.50.xxx.  So it seems that
>
>     # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.
>     elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a
> "$ld_vers_minor" -eq 20; then
>       gcc_cv_lto_plugin=1
>     fi
>
> only allows explicit -fuse-linker-plugin for recent gold but based on checking
> the system linker version (instead of that of --with-plugin-ld).  I'd say
> allow explicit -fuse-linker-plugin always if --with-plugin-ld is specified.

Agreed.  I'll have a look and refrain from installing on 4.6 until this
is resolved.  I wasn't even aware of --with-plugin-ld, which isn't
documented in install.texi ;-(  This stuff seems insanely complicated to
me, though: which user is supposed to deal with such complications?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Fri, 18 Mar 2011, Rainer Orth wrote:

> Richard Guenther <richard.guenther@gmail.com> writes:
> 
> > It seemed to have disabled linker-plugin support for old binutils
> > with --with-plugin-ld=/usr/local/bin/gold, explicit -fuse-linker-plugin says
> > it is not supported.  The system linker does not have plugin support
> > (nor gold).  /usr/local has gold from binutils 2.20.50.xxx.  So it seems that
> >
> >     # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.
> >     elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a
> > "$ld_vers_minor" -eq 20; then
> >       gcc_cv_lto_plugin=1
> >     fi
> >
> > only allows explicit -fuse-linker-plugin for recent gold but based on checking
> > the system linker version (instead of that of --with-plugin-ld).  I'd say
> > allow explicit -fuse-linker-plugin always if --with-plugin-ld is specified.
> 
> Agreed.  I'll have a look and refrain from installing on 4.6 until this
> is resolved.  I wasn't even aware of --with-plugin-ld, which isn't
> documented in install.texi ;-(  This stuff seems insanely complicated to
> me, though: which user is supposed to deal with such complications?

It's sort of historic ... with plugin support for GNU ld we can probably
drop this flag for 4.7 (eventually I'd even like to enforce the use
of the plugin all the time or disable LTO, just for the sake of reducing
the testing and bugreporting matrix ...).

I just noticed the above with the LTO setup of our SPEC testers
which are using somewhat old system tools.

Thanks,
Richard.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

> It's sort of historic ... with plugin support for GNU ld we can probably

Could you or someone else who understands this stuff please document it
in install.texi then?  Thanks.

> drop this flag for 4.7 (eventually I'd even like to enforce the use
> of the plugin all the time or disable LTO, just for the sake of reducing
> the testing and bugreporting matrix ...).

This would mean no more LTO without GNU ld/gold, then?  Isn't this going
too far?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Fri, 18 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> > It's sort of historic ... with plugin support for GNU ld we can probably
> 
> Could you or someone else who understands this stuff please document it
> in install.texi then?  Thanks.

I'll try.

> > drop this flag for 4.7 (eventually I'd even like to enforce the use
> > of the plugin all the time or disable LTO, just for the sake of reducing
> > the testing and bugreporting matrix ...).
> 
> This would mean no more LTO without GNU ld/gold, then?  Isn't this going
> too far?

I'm not sure - the collect2 path has known issues that are hard
to address and result in unexpected behavior, so it's probably
not a good service to our users to serve them half-broken LTO.

But I suppose we can discuss this when we also see a clear benefit
in removing the corresponding code in gcc.

Richard.
kk

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <rguenther@suse.de> writes:

> On Fri, 18 Mar 2011, Rainer Orth wrote:
>
>> Richard Guenther <richard.guenther@gmail.com> writes:
>> 
>> > It seemed to have disabled linker-plugin support for old binutils
>> > with --with-plugin-ld=/usr/local/bin/gold, explicit -fuse-linker-plugin says
>> > it is not supported.  The system linker does not have plugin support
>> > (nor gold).  /usr/local has gold from binutils 2.20.50.xxx.  So it seems that
>> >
>> >     # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.
>> >     elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a
>> > "$ld_vers_minor" -eq 20; then
>> >       gcc_cv_lto_plugin=1
>> >     fi
>> >
>> > only allows explicit -fuse-linker-plugin for recent gold but based on checking
>> > the system linker version (instead of that of --with-plugin-ld).  I'd say
>> > allow explicit -fuse-linker-plugin always if --with-plugin-ld is specified.
>> 
>> Agreed.  I'll have a look and refrain from installing on 4.6 until this
>> is resolved.  I wasn't even aware of --with-plugin-ld, which isn't
>> documented in install.texi ;-(  This stuff seems insanely complicated to
>> me, though: which user is supposed to deal with such complications?
>
> It's sort of historic ... with plugin support for GNU ld we can probably
> drop this flag for 4.7 (eventually I'd even like to enforce the use
> of the plugin all the time or disable LTO, just for the sake of reducing
> the testing and bugreporting matrix ...).
>
> I just noticed the above with the LTO setup of our SPEC testers
> which are using somewhat old system tools.

Here's the patch I came up with.  It is on top of the previous one, so
if we want to backport to 4.6 later, both are necessary.

It also fixes a typo in the in_tree_ld case, which can never have worked.

Tested by i386-pc-solaris2.11 bootstraps with either Sun ld or gld 2.21,
and by configuring with Sun ld --with-plugin-ld=gld-2.21 and obvserving
HAVE_LTO_PLUGIN being set to 1.

Ok for mainline now and 4.6 after some soak time?

Thanks.
	Rainer


2011-03-19  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* configure.ac (gcc_cv_lto_plugin): Fix typo.
	Allow -fuse-linker-plugin for non-default plugin linker.
	* configure: Regenerate.

diff -r 916a52719d80 gcc/configure.ac
--- a/gcc/configure.ac	Sat Mar 19 08:54:19 2011 +0100
+++ b/gcc/configure.ac	Sun Mar 20 13:37:38 2011 +0100
@@ -3192,14 +3192,14 @@
 AC_MSG_CHECKING(linker plugin support)
 gcc_cv_lto_plugin=0
 if test -f liblto_plugin.la; then
-  if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET=" = x"$gcc_cv_ld"; then
+  if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"; then
     if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
       gcc_cv_lto_plugin=2
     elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
       gcc_cv_lto_plugin=1
 
     fi
-  elif echo "$ld_ver" | grep GNU > /dev/null; then
+  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld" && echo "$ld_ver" | grep GNU > /dev/null; then
     # Require GNU ld or gold 2.21+ for plugin support by default.
     if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
       gcc_cv_lto_plugin=2
@@ -3207,6 +3207,10 @@
     elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
       gcc_cv_lto_plugin=1
     fi
+  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" != x"$gcc_cv_ld"; then
+    # Allow -fuse-linker-plugin if plugin linker differs from
+    # default/specified linker.
+    gcc_cv_lto_plugin=1
   fi
 fi
 AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc_cv_lto_plugin,


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Paolo Bonzini]

On 03/21/2011 11:18 AM, Rainer Orth wrote:
> Richard Guenther<rguenther@suse.de>  writes:
>
>> On Fri, 18 Mar 2011, Rainer Orth wrote:
>>
>>> Richard Guenther<richard.guenther@gmail.com>  writes:
>>>
>>>> It seemed to have disabled linker-plugin support for old binutils
>>>> with --with-plugin-ld=/usr/local/bin/gold, explicit -fuse-linker-plugin says
>>>> it is not supported.  The system linker does not have plugin support
>>>> (nor gold).  /usr/local has gold from binutils 2.20.50.xxx.  So it seems that
>>>>
>>>>      # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.
>>>>      elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a
>>>> "$ld_vers_minor" -eq 20; then
>>>>        gcc_cv_lto_plugin=1
>>>>      fi
>>>>
>>>> only allows explicit -fuse-linker-plugin for recent gold but based on checking
>>>> the system linker version (instead of that of --with-plugin-ld).  I'd say
>>>> allow explicit -fuse-linker-plugin always if --with-plugin-ld is specified.
>>>
>>> Agreed.  I'll have a look and refrain from installing on 4.6 until this
>>> is resolved.  I wasn't even aware of --with-plugin-ld, which isn't
>>> documented in install.texi ;-(  This stuff seems insanely complicated to
>>> me, though: which user is supposed to deal with such complications?
>>
>> It's sort of historic ... with plugin support for GNU ld we can probably
>> drop this flag for 4.7 (eventually I'd even like to enforce the use
>> of the plugin all the time or disable LTO, just for the sake of reducing
>> the testing and bugreporting matrix ...).
>>
>> I just noticed the above with the LTO setup of our SPEC testers
>> which are using somewhat old system tools.
>
> Here's the patch I came up with.  It is on top of the previous one, so
> if we want to backport to 4.6 later, both are necessary.
>
> It also fixes a typo in the in_tree_ld case, which can never have worked.
>
> Tested by i386-pc-solaris2.11 bootstraps with either Sun ld or gld 2.21,
> and by configuring with Sun ld --with-plugin-ld=gld-2.21 and obvserving
> HAVE_LTO_PLUGIN being set to 1.
>
> Ok for mainline now and 4.6 after some soak time?
>
> Thanks.
> 	Rainer
>
>
> 2011-03-19  Rainer Orth<ro@CeBiTec.Uni-Bielefeld.DE>
>
> 	* configure.ac (gcc_cv_lto_plugin): Fix typo.
> 	Allow -fuse-linker-plugin for non-default plugin linker.
> 	* configure: Regenerate.
>
> diff -r 916a52719d80 gcc/configure.ac
> --- a/gcc/configure.ac	Sat Mar 19 08:54:19 2011 +0100
> +++ b/gcc/configure.ac	Sun Mar 20 13:37:38 2011 +0100
> @@ -3192,14 +3192,14 @@
>   AC_MSG_CHECKING(linker plugin support)
>   gcc_cv_lto_plugin=0
>   if test -f liblto_plugin.la; then
> -  if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET=" = x"$gcc_cv_ld"; then
> +  if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"; then
>       if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
>         gcc_cv_lto_plugin=2
>       elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
>         gcc_cv_lto_plugin=1
>
>       fi
> -  elif echo "$ld_ver" | grep GNU>  /dev/null; then
> +  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"&&  echo "$ld_ver" | grep GNU>  /dev/null; then
>       # Require GNU ld or gold 2.21+ for plugin support by default.
>       if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
>         gcc_cv_lto_plugin=2
> @@ -3207,6 +3207,10 @@
>       elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
>         gcc_cv_lto_plugin=1
>       fi
> +  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" != x"$gcc_cv_ld"; then
> +    # Allow -fuse-linker-plugin if plugin linker differs from
> +    # default/specified linker.
> +    gcc_cv_lto_plugin=1
>     fi
>   fi
>   AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc_cv_lto_plugin,

I haven't followed the thread enough to understand the issue, but the 
patch is trivial enough that I can defer to Richard about it.

Paolo

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Mon, 21 Mar 2011, Rainer Orth wrote:

> Richard Guenther <rguenther@suse.de> writes:
> 
> > On Fri, 18 Mar 2011, Rainer Orth wrote:
> >
> >> Richard Guenther <richard.guenther@gmail.com> writes:
> >> 
> >> > It seemed to have disabled linker-plugin support for old binutils
> >> > with --with-plugin-ld=/usr/local/bin/gold, explicit -fuse-linker-plugin says
> >> > it is not supported.  The system linker does not have plugin support
> >> > (nor gold).  /usr/local has gold from binutils 2.20.50.xxx.  So it seems that
> >> >
> >> >     # Allow -fuse-linker-plugin to enable plugin support in GNU gold 2.20.
> >> >     elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a
> >> > "$ld_vers_minor" -eq 20; then
> >> >       gcc_cv_lto_plugin=1
> >> >     fi
> >> >
> >> > only allows explicit -fuse-linker-plugin for recent gold but based on checking
> >> > the system linker version (instead of that of --with-plugin-ld).  I'd say
> >> > allow explicit -fuse-linker-plugin always if --with-plugin-ld is specified.
> >> 
> >> Agreed.  I'll have a look and refrain from installing on 4.6 until this
> >> is resolved.  I wasn't even aware of --with-plugin-ld, which isn't
> >> documented in install.texi ;-(  This stuff seems insanely complicated to
> >> me, though: which user is supposed to deal with such complications?
> >
> > It's sort of historic ... with plugin support for GNU ld we can probably
> > drop this flag for 4.7 (eventually I'd even like to enforce the use
> > of the plugin all the time or disable LTO, just for the sake of reducing
> > the testing and bugreporting matrix ...).
> >
> > I just noticed the above with the LTO setup of our SPEC testers
> > which are using somewhat old system tools.
> 
> Here's the patch I came up with.  It is on top of the previous one, so
> if we want to backport to 4.6 later, both are necessary.
> 
> It also fixes a typo in the in_tree_ld case, which can never have worked.
> 
> Tested by i386-pc-solaris2.11 bootstraps with either Sun ld or gld 2.21,
> and by configuring with Sun ld --with-plugin-ld=gld-2.21 and obvserving
> HAVE_LTO_PLUGIN being set to 1.
> 
> Ok for mainline now and 4.6 after some soak time?

Ok for trunk and 4.6.1.

Thanks,
Richard.

> Thanks.
> 	Rainer
> 
> 
> 2011-03-19  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	* configure.ac (gcc_cv_lto_plugin): Fix typo.
> 	Allow -fuse-linker-plugin for non-default plugin linker.
> 	* configure: Regenerate.
> 
> diff -r 916a52719d80 gcc/configure.ac
> --- a/gcc/configure.ac	Sat Mar 19 08:54:19 2011 +0100
> +++ b/gcc/configure.ac	Sun Mar 20 13:37:38 2011 +0100
> @@ -3192,14 +3192,14 @@
>  AC_MSG_CHECKING(linker plugin support)
>  gcc_cv_lto_plugin=0
>  if test -f liblto_plugin.la; then
> -  if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET=" = x"$gcc_cv_ld"; then
> +  if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"; then
>      if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
>        gcc_cv_lto_plugin=2
>      elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
>        gcc_cv_lto_plugin=1
>  
>      fi
> -  elif echo "$ld_ver" | grep GNU > /dev/null; then
> +  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld" && echo "$ld_ver" | grep GNU > /dev/null; then
>      # Require GNU ld or gold 2.21+ for plugin support by default.
>      if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
>        gcc_cv_lto_plugin=2
> @@ -3207,6 +3207,10 @@
>      elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
>        gcc_cv_lto_plugin=1
>      fi
> +  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" != x"$gcc_cv_ld"; then
> +    # Allow -fuse-linker-plugin if plugin linker differs from
> +    # default/specified linker.
> +    gcc_cv_lto_plugin=1
>    fi
>  fi
>  AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc_cv_lto_plugin,
> 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Michael Matz]

Hi,

[sorry for breaking the threading I've deleted the mails I'm answering 
already]

In any case, citing from 
  http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01250.html

> Here's the patch I came up with.  It is on top of the previous one, so 
> if we want to backport to 4.6 later, both are necessary.
> 
> It also fixes a typo in the in_tree_ld case, which can never have 
> worked.
> 
> Tested by i386-pc-solaris2.11 bootstraps with either Sun ld or gld 2.21, 
> and by configuring with Sun ld --with-plugin-ld=gld-2.21 and obvserving 
> HAVE_LTO_PLUGIN being set to 1.
...
> @@ -3207,6 +3207,10 @@
>      elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
>        gcc_cv_lto_plugin=1
>      fi
> +  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" != x"$gcc_cv_ld"; then
> +    # Allow -fuse-linker-plugin if plugin linker differs from
> +    # default/specified linker.
> +    gcc_cv_lto_plugin=1
>    fi
>  fi

And this '1' is a problem.  Even if I specify 
--with-plugin-ld=some-good-ld (i.e. it can be reasonably assumed that I 
know what I'm doing) the above forces me to still have to use 
-fuse-linker-plugin when I really want to use it.  This is because the 
introduction of three levels of HAVE_LTO_PLUGIN: 0 (-fuse-linker-plugin 
not allowed), 1 (allowed but not default for -flto), 2 (allowed and 
default to on with -flto).

I think if the plugin linker is different from the normal linker we should 
set HAVE_LTO_PLUGIN to 2.


Ciao,
Michael.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Sat, Mar 26, 2011 at 3:51 AM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> [sorry for breaking the threading I've deleted the mails I'm answering
> already]
>
> In any case, citing from
>  http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01250.html
>
>> Here's the patch I came up with.  It is on top of the previous one, so
>> if we want to backport to 4.6 later, both are necessary.
>>
>> It also fixes a typo in the in_tree_ld case, which can never have
>> worked.
>>
>> Tested by i386-pc-solaris2.11 bootstraps with either Sun ld or gld 2.21,
>> and by configuring with Sun ld --with-plugin-ld=gld-2.21 and obvserving
>> HAVE_LTO_PLUGIN being set to 1.
> ...
>> @@ -3207,6 +3207,10 @@
>>      elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
>>        gcc_cv_lto_plugin=1
>>      fi
>> +  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" != x"$gcc_cv_ld"; then
>> +    # Allow -fuse-linker-plugin if plugin linker differs from
>> +    # default/specified linker.
>> +    gcc_cv_lto_plugin=1
>>    fi
>>  fi
>
> And this '1' is a problem.  Even if I specify
> --with-plugin-ld=some-good-ld (i.e. it can be reasonably assumed that I
> know what I'm doing) the above forces me to still have to use
> -fuse-linker-plugin when I really want to use it.  This is because the
> introduction of three levels of HAVE_LTO_PLUGIN: 0 (-fuse-linker-plugin
> not allowed), 1 (allowed but not default for -flto), 2 (allowed and
> default to on with -flto).
>
> I think if the plugin linker is different from the normal linker we should
> set HAVE_LTO_PLUGIN to 2.

I think we should do the linker version checks which relate to linker-plugin
use on the plugin-linker instead.  So if I specify a separate but known
buggy linker I don't want it to be used by default.

Richard.

>
> Ciao,
> Michael.
>

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <richard.guenther@gmail.com> writes:

>> I think if the plugin linker is different from the normal linker we should
>> set HAVE_LTO_PLUGIN to 2.
>
> I think we should do the linker version checks which relate to linker-plugin
> use on the plugin-linker instead.  So if I specify a separate but known

I'll see if this is reasonably doable...

> buggy linker I don't want it to be used by default.

... but how about a different route for 4.7?  I've never really
understood the point of --with-plugin-ld.  Given that we have known-good
linkers, why not simply remove that wart?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Mon, Mar 28, 2011 at 12:14 PM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
> Richard Guenther <richard.guenther@gmail.com> writes:
>
>>> I think if the plugin linker is different from the normal linker we should
>>> set HAVE_LTO_PLUGIN to 2.
>>
>> I think we should do the linker version checks which relate to linker-plugin
>> use on the plugin-linker instead.  So if I specify a separate but known
>
> I'll see if this is reasonably doable...
>
>> buggy linker I don't want it to be used by default.
>
> ... but how about a different route for 4.7?  I've never really
> understood the point of --with-plugin-ld.  Given that we have known-good
> linkers, why not simply remove that wart?

I sort-of agree, but for example for weird hosts like solaris we might want
to give people the option to not replace their system linker unless they
are using LTO.  We could of course direct them to --with-ld instead, replacing
the linker for a specific compiler only.

What I'd like to remove for 4.7 is the -fuse-linker-plugin
flag and non-linker-plugin operation (but that requires some more discussion).
At least we should warn people if no linker with appropriate plugin-support
was detected, as LTO support might be fragile then.

Richard.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Richard Guenther <richard.guenther@gmail.com> writes:

> I think we should do the linker version checks which relate to linker-plugin
> use on the plugin-linker instead.  So if I specify a separate but known
> buggy linker I don't want it to be used by default.

Here's a patch that does this.  I'm not at all happy with the patch
since it partially duplicates the logic to determine linker version
numbers.  While this could (and probably should) be generalized along
the lines of gcc_GAS_CHECK_FEATURE and gcc_GAS_VERSION_GTE_IFELSE, even
that wouldn't help immediately since such autoconf macros would still
$gcc_cv_ld.  As far as I can see, all those linker checks could
massively benefit from an overhaul to use gcc_LD_CHECK_FEATURE
etc.macros, but that's not something I want to attack.  It's especially
messy that there are two sets of version variables for in-tree and
external linkers.  Probably fodder for the build maintainers.

Anyway, here's what I've got.  Tested by configuring with

* no --with-ld arg (i.e. /usr/ccs/bin/ld)

* --with-ld=/path/to/gld-2.21 --with-gnu-ld

* --with-plugin-ld=/path/to/gld-2.21

* --with-ld=/path/to/gld-2.21 --with-gnu-ld --with-plugin-ld=/usr/ccs/bin/ld

and checking HAVE_LTO_PLUGIN in auto-host.h (0, 2, 2, 0).

I haven't found if there are provisions for in-tree gold, though, and
still cannot test that.

Ok for mainline?

Could the whole bunch eventually be backported to the 4.6 branch?

	http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00759.html
        http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01890.html
        http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01250.html

and this one?

Thanks.
	Rainer


2011-04-02  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* configure.ac (gcc_cv_lto_plugin): Determine lto plugin support
	from plugin linker.
	* configure: Regenerate.

diff --git a/gcc/configure.ac b/gcc/configure.ac
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3192,14 +3192,40 @@ fi
 AC_MSG_CHECKING(linker plugin support)
 gcc_cv_lto_plugin=0
 if test -f liblto_plugin.la; then
+  save_ld_ver="$ld_ver"
+  save_ld_vers_major="$ld_vers_major"
+  save_ld_vers_minor="$ld_vers_minor"
+  save_ld_is_gold="$ld_is_gold"
+
+  ld_is_gold=no
+
   if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"; then
-    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
-      gcc_cv_lto_plugin=2
-    elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
-      gcc_cv_lto_plugin=1
-
+    ld_ver="GNU ld"
+    # FIXME: ld_is_gold?
+    ld_vers_major="$gcc_cv_gld_major_version"
+    ld_vers_minor="$gcc_cv_gld_minor_version"
+  else
+    # Determine plugin linker version.
+    # FIXME: Partial duplicate from above, generalize.
+changequote(,)dnl
+    ld_ver=`$ORIGINAL_PLUGIN_LD_FOR_TARGET --version 2>/dev/null | sed 1q`
+    if echo "$ld_ver" | grep GNU > /dev/null; then
+      if echo "$ld_ver" | grep "GNU gold" > /dev/null; then
+        ld_is_gold=yes
+        ld_vers=`echo $ld_ver | sed -n \
+    	    -e 's,^[^)]*[	 ]\([0-9][0-9]*\.[0-9][0-9]*[^)]*\)) .*$,\1,p'`
+      else
+        ld_vers=`echo $ld_ver | sed -n \
+    	    -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
+      fi
+      ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
+      ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
     fi
-  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld" && echo "$ld_ver" | grep GNU > /dev/null; then
+changequote([,])dnl
+  fi
+
+  # Determine plugin support.
+  if echo "$ld_ver" | grep GNU > /dev/null; then
     # Require GNU ld or gold 2.21+ for plugin support by default.
     if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
       gcc_cv_lto_plugin=2
@@ -3207,11 +3233,12 @@ if test -f liblto_plugin.la; then
     elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
       gcc_cv_lto_plugin=1
     fi
-  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" != x"$gcc_cv_ld"; then
-    # Allow -fuse-linker-plugin if plugin linker differs from
-    # default/specified linker.
-    gcc_cv_lto_plugin=1
   fi
+
+  ld_ver="$save_ld_ver"
+  ld_vers_major="$save_ld_vers_major"
+  ld_vers_minor="$save_ld_vers_minor"
+  ld_is_gold="$save_ld_is_gold"
 fi
 AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc_cv_lto_plugin,
   [Define to the level of your linker's plugin support.])


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> Richard Guenther <richard.guenther@gmail.com> writes:
>
>> I think we should do the linker version checks which relate to linker-plugin
>> use on the plugin-linker instead.  So if I specify a separate but known
>> buggy linker I don't want it to be used by default.
>
> Here's a patch that does this.  I'm not at all happy with the patch
> since it partially duplicates the logic to determine linker version
> numbers.  While this could (and probably should) be generalized along
> the lines of gcc_GAS_CHECK_FEATURE and gcc_GAS_VERSION_GTE_IFELSE, even
> that wouldn't help immediately since such autoconf macros would still
> $gcc_cv_ld.  As far as I can see, all those linker checks could
> massively benefit from an overhaul to use gcc_LD_CHECK_FEATURE
> etc.macros, but that's not something I want to attack.  It's especially
> messy that there are two sets of version variables for in-tree and
> external linkers.  Probably fodder for the build maintainers.

This patch has remained unreviewed for a week now:

	http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00226.html

Any takers?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Ralf Wildenhues]

Hello Rainer,

* Rainer Orth wrote on Mon, Apr 04, 2011 at 06:15:28PM CEST:
> Here's a patch that does this.  I'm not at all happy with the patch
> since it partially duplicates the logic to determine linker version
> numbers.  While this could (and probably should) be generalized along
> the lines of gcc_GAS_CHECK_FEATURE and gcc_GAS_VERSION_GTE_IFELSE, even
> that wouldn't help immediately since such autoconf macros would still
> $gcc_cv_ld.  As far as I can see, all those linker checks could
> massively benefit from an overhaul to use gcc_LD_CHECK_FEATURE
> etc.macros, but that's not something I want to attack.  It's especially
> messy that there are two sets of version variables for in-tree and
> external linkers.  Probably fodder for the build maintainers.
> 
> Anyway, here's what I've got.  Tested by configuring with
> 
> * no --with-ld arg (i.e. /usr/ccs/bin/ld)
> 
> * --with-ld=/path/to/gld-2.21 --with-gnu-ld
> 
> * --with-plugin-ld=/path/to/gld-2.21
> 
> * --with-ld=/path/to/gld-2.21 --with-gnu-ld --with-plugin-ld=/usr/ccs/bin/ld
> 
> and checking HAVE_LTO_PLUGIN in auto-host.h (0, 2, 2, 0).
> 
> I haven't found if there are provisions for in-tree gold, though, and
> still cannot test that.

I'm not quite sure I understand this statement.  I built a combined tree
with gold enabled a while ago (must've been several months now).
I might be misunderstanding this.

> Ok for mainline?

I think I'll need to take another look after questions are answered.

> 2011-04-02  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	* configure.ac (gcc_cv_lto_plugin): Determine lto plugin support
> 	from plugin linker.
> 	* configure: Regenerate.

> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -3192,14 +3192,40 @@ fi
>  AC_MSG_CHECKING(linker plugin support)
>  gcc_cv_lto_plugin=0
>  if test -f liblto_plugin.la; then
> +  save_ld_ver="$ld_ver"
> +  save_ld_vers_major="$ld_vers_major"
> +  save_ld_vers_minor="$ld_vers_minor"
> +  save_ld_is_gold="$ld_is_gold"
> +
> +  ld_is_gold=no
> +
>    if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"; then
> -    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
> -      gcc_cv_lto_plugin=2
> -    elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
> -      gcc_cv_lto_plugin=1
> -
> +    ld_ver="GNU ld"
> +    # FIXME: ld_is_gold?

What about this FIXME?  Did you test gold?  Is it not relevant here?
Can the FIXME go?

> +    ld_vers_major="$gcc_cv_gld_major_version"
> +    ld_vers_minor="$gcc_cv_gld_minor_version"
> +  else
> +    # Determine plugin linker version.
> +    # FIXME: Partial duplicate from above, generalize.
> +changequote(,)dnl
> +    ld_ver=`$ORIGINAL_PLUGIN_LD_FOR_TARGET --version 2>/dev/null | sed 1q`
> +    if echo "$ld_ver" | grep GNU > /dev/null; then
> +      if echo "$ld_ver" | grep "GNU gold" > /dev/null; then
> +        ld_is_gold=yes
> +        ld_vers=`echo $ld_ver | sed -n \
> +    	    -e 's,^[^)]*[	 ]\([0-9][0-9]*\.[0-9][0-9]*[^)]*\)) .*$,\1,p'`
> +      else
> +        ld_vers=`echo $ld_ver | sed -n \
> +    	    -e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
> +      fi
> +      ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
> +      ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`

Can you try the expr statements quickly on Tru64?  If not, I can do it
for you ('info Autoconf --index expr' is long and tells of many woes).

>      fi
> -  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld" && echo "$ld_ver" | grep GNU > /dev/null; then
> +changequote([,])dnl
> +  fi
> +
> +  # Determine plugin support.
> +  if echo "$ld_ver" | grep GNU > /dev/null; then
>      # Require GNU ld or gold 2.21+ for plugin support by default.
>      if test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -ge 21; then
>        gcc_cv_lto_plugin=2
> @@ -3207,11 +3233,12 @@ if test -f liblto_plugin.la; then
>      elif test "$ld_is_gold" = yes -a "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -eq 20; then
>        gcc_cv_lto_plugin=1
>      fi
> -  elif test x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" != x"$gcc_cv_ld"; then
> -    # Allow -fuse-linker-plugin if plugin linker differs from
> -    # default/specified linker.
> -    gcc_cv_lto_plugin=1
>    fi
> +
> +  ld_ver="$save_ld_ver"
> +  ld_vers_major="$save_ld_vers_major"
> +  ld_vers_minor="$save_ld_vers_minor"
> +  ld_is_gold="$save_ld_is_gold"
>  fi
>  AC_DEFINE_UNQUOTED(HAVE_LTO_PLUGIN, $gcc_cv_lto_plugin,
>    [Define to the level of your linker's plugin support.])

Thanks, and sorry for the delay,
Ralf

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Hi Ralf,

>> I haven't found if there are provisions for in-tree gold, though, and
>> still cannot test that.
>
> I'm not quite sure I understand this statement.  I built a combined tree
> with gold enabled a while ago (must've been several months now).
> I might be misunderstanding this.

I suppose I've been unclear: I'm specificially referring to the current
code for setting gcc_cv_lto_plugin: in the in-tree case, there's nothing
that deals with in-tree gold.

>>    if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"; then
>> -    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
>> -      gcc_cv_lto_plugin=2
>> -    elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
>> -      gcc_cv_lto_plugin=1
>> -
>> +    ld_ver="GNU ld"
>> +    # FIXME: ld_is_gold?
>
> What about this FIXME?  Did you test gold?  Is it not relevant here?
> Can the FIXME go?

I cannot test gold since it doesn't yet work on Solaris: cf. binutils PR
gold/12525.  We made some progress on that front, but the PR is
currently stalled and I had other things on my plate that prevented me
from pushing it.  As I said, the current code (before my patch) doesn't
handle in-tree gold, so I don't feel obliged to change that, especially
since I'm in no good position to test.

>> +      ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
>> +      ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
>
> Can you try the expr statements quickly on Tru64?  If not, I can do it
> for you ('info Autoconf --index expr' is long and tells of many woes).

I just tried with /bin/expr and ld_vers set to 2.20.1.  OTOH, this isn't
relevant for two reasons: this code is identical to the one determining
ld_vers_major/ld_vers_minor further up in gcc/configure.ac, and GNU ld
(as well as GNU as) aren't currently supported on Tru64 UNIX and I
seriously doubt that will change over the remaining livetime of the
platform.

> Thanks, and sorry for the delay,

No worries.  I'd just like to get this series of patches out of my queue
(and eventually backported to the 4.6 branch if all issues are sorted
out).  Maybe one of the build maintainers finds some time to handle the
current mess that are the linker tests in gcc/configure.ac, compared to
what we do for the assembler.

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Hi Ralf,

it's been a week since I answered your questions on this patch.  Could
you please have a look?

Thanks.
        Rainer


>>> I haven't found if there are provisions for in-tree gold, though, and
>>> still cannot test that.
>>
>> I'm not quite sure I understand this statement.  I built a combined tree
>> with gold enabled a while ago (must've been several months now).
>> I might be misunderstanding this.
>
> I suppose I've been unclear: I'm specificially referring to the current
> code for setting gcc_cv_lto_plugin: in the in-tree case, there's nothing
> that deals with in-tree gold.
>
>>>    if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"; then
>>> -    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
>>> -      gcc_cv_lto_plugin=2
>>> -    elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
>>> -      gcc_cv_lto_plugin=1
>>> -
>>> +    ld_ver="GNU ld"
>>> +    # FIXME: ld_is_gold?
>>
>> What about this FIXME?  Did you test gold?  Is it not relevant here?
>> Can the FIXME go?
>
> I cannot test gold since it doesn't yet work on Solaris: cf. binutils PR
> gold/12525.  We made some progress on that front, but the PR is
> currently stalled and I had other things on my plate that prevented me
> from pushing it.  As I said, the current code (before my patch) doesn't
> handle in-tree gold, so I don't feel obliged to change that, especially
> since I'm in no good position to test.
>
>>> +      ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
>>> +      ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
>>
>> Can you try the expr statements quickly on Tru64?  If not, I can do it
>> for you ('info Autoconf --index expr' is long and tells of many woes).
>
> I just tried with /bin/expr and ld_vers set to 2.20.1.  OTOH, this isn't
> relevant for two reasons: this code is identical to the one determining
> ld_vers_major/ld_vers_minor further up in gcc/configure.ac, and GNU ld
> (as well as GNU as) aren't currently supported on Tru64 UNIX and I
> seriously doubt that will change over the remaining livetime of the
> platform.
>
>> Thanks, and sorry for the delay,
>
> No worries.  I'd just like to get this series of patches out of my queue
> (and eventually backported to the 4.6 branch if all issues are sorted
> out).  Maybe one of the build maintainers finds some time to handle the
> current mess that are the linker tests in gcc/configure.ac, compared to
> what we do for the assembler.
>
> Thanks.
> 	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Ralf Wildenhues]

Hello Rainer,

* Rainer Orth wrote on Tue, Apr 26, 2011 at 05:28:19PM CEST:
> it's been a week since I answered your questions on this patch.  Could
> you please have a look?

Sorry for the delay.  I'm practically AFK until the weekend or maybe
next weekend, whenever I have connectivity again after relocating;
if another build maintainer could take a look that would be nice,
otherwise I will get to it ASAP.

Thanks,
Ralf

> >>> I haven't found if there are provisions for in-tree gold, though, and
> >>> still cannot test that.
> >>
> >> I'm not quite sure I understand this statement.  I built a combined tree
> >> with gold enabled a while ago (must've been several months now).
> >> I might be misunderstanding this.
> >
> > I suppose I've been unclear: I'm specificially referring to the current
> > code for setting gcc_cv_lto_plugin: in the in-tree case, there's nothing
> > that deals with in-tree gold.
> >
> >>>    if test $in_tree_ld = yes -a x"$ORIGINAL_PLUGIN_LD_FOR_TARGET" = x"$gcc_cv_ld"; then
> >>> -    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2; then
> >>> -      gcc_cv_lto_plugin=2
> >>> -    elif test "$ld_is_gold" = yes -a "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -eq 20; then
> >>> -      gcc_cv_lto_plugin=1
> >>> -
> >>> +    ld_ver="GNU ld"
> >>> +    # FIXME: ld_is_gold?
> >>
> >> What about this FIXME?  Did you test gold?  Is it not relevant here?
> >> Can the FIXME go?
> >
> > I cannot test gold since it doesn't yet work on Solaris: cf. binutils PR
> > gold/12525.  We made some progress on that front, but the PR is
> > currently stalled and I had other things on my plate that prevented me
> > from pushing it.  As I said, the current code (before my patch) doesn't
> > handle in-tree gold, so I don't feel obliged to change that, especially
> > since I'm in no good position to test.
> >
> >>> +      ld_vers_major=`expr "$ld_vers" : '\([0-9]*\)'`
> >>> +      ld_vers_minor=`expr "$ld_vers" : '[0-9]*\.\([0-9]*\)'`
> >>
> >> Can you try the expr statements quickly on Tru64?  If not, I can do it
> >> for you ('info Autoconf --index expr' is long and tells of many woes).
> >
> > I just tried with /bin/expr and ld_vers set to 2.20.1.  OTOH, this isn't
> > relevant for two reasons: this code is identical to the one determining
> > ld_vers_major/ld_vers_minor further up in gcc/configure.ac, and GNU ld
> > (as well as GNU as) aren't currently supported on Tru64 UNIX and I
> > seriously doubt that will change over the remaining livetime of the
> > platform.
> >
> >> Thanks, and sorry for the delay,
> >
> > No worries.  I'd just like to get this series of patches out of my queue
> > (and eventually backported to the 4.6 branch if all issues are sorted
> > out).  Maybe one of the build maintainers finds some time to handle the
> > current mess that are the linker tests in gcc/configure.ac, compared to
> > what we do for the assembler.

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Hello Ralf,

> * Rainer Orth wrote on Tue, Apr 26, 2011 at 05:28:19PM CEST:
>> it's been a week since I answered your questions on this patch.  Could
>> you please have a look?
>
> Sorry for the delay.  I'm practically AFK until the weekend or maybe
> next weekend, whenever I have connectivity again after relocating;
> if another build maintainer could take a look that would be nice,
> otherwise I will get to it ASAP.

no worries.  I just wanted to make sure that the patch doesn't fall
through the cracks.  If another build maintainer could have a look, that
would be great, otherwise I can easily wait another week or two.

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Paolo Bonzini]

On 04/04/2011 06:15 PM, Rainer Orth wrote:
> I haven't found if there are provisions for in-tree gold, though, and
> still cannot test that.

In-tree gold definitely works (or should).

Paolo

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Paolo Bonzini <bonzini@gnu.org> writes:

> On 04/04/2011 06:15 PM, Rainer Orth wrote:
>> I haven't found if there are provisions for in-tree gold, though, and
>> still cannot test that.
>
> In-tree gold definitely works (or should).

My problem is simply that gold doesn't work on Solaris at all, either
in-tree or externally.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Rainer Orth]

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> Richard Guenther <richard.guenther@gmail.com> writes:
>
>> I think we should do the linker version checks which relate to linker-plugin
>> use on the plugin-linker instead.  So if I specify a separate but known
>> buggy linker I don't want it to be used by default.
>
> Here's a patch that does this.  I'm not at all happy with the patch
> since it partially duplicates the logic to determine linker version
> numbers.  While this could (and probably should) be generalized along
> the lines of gcc_GAS_CHECK_FEATURE and gcc_GAS_VERSION_GTE_IFELSE, even
> that wouldn't help immediately since such autoconf macros would still
> $gcc_cv_ld.  As far as I can see, all those linker checks could
> massively benefit from an overhaul to use gcc_LD_CHECK_FEATURE
> etc.macros, but that's not something I want to attack.  It's especially
> messy that there are two sets of version variables for in-tree and
> external linkers.  Probably fodder for the build maintainers.
>
> Anyway, here's what I've got.  Tested by configuring with
>
> * no --with-ld arg (i.e. /usr/ccs/bin/ld)
>
> * --with-ld=/path/to/gld-2.21 --with-gnu-ld
>
> * --with-plugin-ld=/path/to/gld-2.21
>
> * --with-ld=/path/to/gld-2.21 --with-gnu-ld --with-plugin-ld=/usr/ccs/bin/ld
>
> and checking HAVE_LTO_PLUGIN in auto-host.h (0, 2, 2, 0).
>
> I haven't found if there are provisions for in-tree gold, though, and
> still cannot test that.
[...]
> Could the whole bunch eventually be backported to the 4.6 branch?
>
> 	http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00759.html
>         http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01890.html
>         http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01250.html
>
> and this one?

This question remains: is this series appropriate for the 4.6 branch or
should it stay on mainline only?

> 2011-04-02  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>
> 	* configure.ac (gcc_cv_lto_plugin): Determine lto plugin support
> 	from plugin linker.
> 	* configure: Regenerate.

This patch has now been approved by Paolo in private mail, installed on
mainline.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [build, lto] Only accept -fuse-linker-plugin if linker supports -plugin (PR lto/46944)

[Richard Guenther]

On Mon, 30 May 2011, Rainer Orth wrote:

> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
> 
> > Richard Guenther <richard.guenther@gmail.com> writes:
> >
> >> I think we should do the linker version checks which relate to linker-plugin
> >> use on the plugin-linker instead.  So if I specify a separate but known
> >> buggy linker I don't want it to be used by default.
> >
> > Here's a patch that does this.  I'm not at all happy with the patch
> > since it partially duplicates the logic to determine linker version
> > numbers.  While this could (and probably should) be generalized along
> > the lines of gcc_GAS_CHECK_FEATURE and gcc_GAS_VERSION_GTE_IFELSE, even
> > that wouldn't help immediately since such autoconf macros would still
> > $gcc_cv_ld.  As far as I can see, all those linker checks could
> > massively benefit from an overhaul to use gcc_LD_CHECK_FEATURE
> > etc.macros, but that's not something I want to attack.  It's especially
> > messy that there are two sets of version variables for in-tree and
> > external linkers.  Probably fodder for the build maintainers.
> >
> > Anyway, here's what I've got.  Tested by configuring with
> >
> > * no --with-ld arg (i.e. /usr/ccs/bin/ld)
> >
> > * --with-ld=/path/to/gld-2.21 --with-gnu-ld
> >
> > * --with-plugin-ld=/path/to/gld-2.21
> >
> > * --with-ld=/path/to/gld-2.21 --with-gnu-ld --with-plugin-ld=/usr/ccs/bin/ld
> >
> > and checking HAVE_LTO_PLUGIN in auto-host.h (0, 2, 2, 0).
> >
> > I haven't found if there are provisions for in-tree gold, though, and
> > still cannot test that.
> [...]
> > Could the whole bunch eventually be backported to the 4.6 branch?
> >
> > 	http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00759.html
> >         http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01890.html
> >         http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01250.html
> >
> > and this one?
> 
> This question remains: is this series appropriate for the 4.6 branch or
> should it stay on mainline only?

I think it should stay on mainline for now.

Richard.