| Message ID | ydd1v1idmcf.fsf@manam.CeBiTec.Uni-Bielefeld.DE |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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
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
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.
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 <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
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.
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.])