| Message ID | 1518556781-10049-1-git-send-email-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | jit: fix link on OS X and Solaris (PR jit/64089 and PR jit/84288) | expand |
Hi David, > libgccjit fails to link on OS X and Solaris due to jit/Make-lang.in, > due to the assumption there that the linker is GNU ld. Specifically, > jit/Make-lang.in hardcodes the use of two options: --version-script > and -soname. > > * on Darwin, --version-script doesn't seem to exist in the linker, and > it uses -install_name rather than -soname. > > * on Solaris, ld doesn't support --version-script. However, the version > script used for libgccjit.so doesn't use any gld extensions, so one can > just use -M instead. > > This patch fixes these issues by using variables emitted by gcc's "configure" > rather than hardcoding the options in jit/Make-lang.in. > > It's based on the first part of Rainer's patch for PR jit/84288, but I > made the following changes: > * the GNU ld case in configure.ac wasn't setting ld_version_script_option. > I set it to "--version-script" for that case. That's weird: the configure.ac part starts with ld_version_script_option='--version-script' only overriding it in the Solaris case to keep things for other hosts as they were. Besides, I'm pretty sure I tested the patch on Solaris with gas/gld to make certain that combo continues to work as is... > * I moved the: > LD_VERSION_SCRIPT_OPTION = @ld_version_script_option@ > from gcc/jit/Make-lang.in to gcc/Makefile.in, as the Make-lang.in files > aren't substituted, only the gcc/Makefile.in. > Rainer: how did this work for you? It didn't: what I'd attached to the PR was an initial version of the patch in the middle between manually hacking gcc/jit/Make-lang.in and properly autoconfiguring things. > * added LD_SONAME_OPTION, done in the same way > * conditionalized the usage of the options in Make-lang.in to cope with > empty LD_VERSION_SCRIPT_OPTION (as is presumably the case on OS X). > I used ($if condition,then-part[,else-part]) for this. > I had to add a $(COMMA) since the "then-part" contains commas, which > need to be treated as part of the "then-part", rather than separators > for the "else-part". > Hopefully this is compatible with every "make" implementation that we > support. > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. > > I lightly tested the not-recognized case by hacking up the configure.ac > (on x86_64-pc-linux-gnu) and verifying that it links, and that a > smoketest of jit.dg/test-factorial works. > > Does this fix the jit linker issues on OS X and Solaris? I'll give it a whirl tomorrow, including the jit-recording.c part of my patch to allow the build to complete. Thanks. Rainer
Hi David, >> * added LD_SONAME_OPTION, done in the same way [...] >> Does this fix the jit linker issues on OS X and Solaris? > > I'll give it a whirl tomorrow, including the jit-recording.c part of my > patch to allow the build to complete. actually, I've replaced the Makefile and configure parts of my patch with yours and did a jit-only bootstrap on i386-pc-solaris2.11 with as/ld and gas/ld. Both went fine with a minor caveat: I noticed that LD_SONAME_OPTION wasn't set in gcc/Makefile. Fixed with the following (so far untested) snippet: diff --git a/gcc/configure.ac b/gcc/configure.ac --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -3715,6 +3715,12 @@ elif test x$gcc_cv_ld != x; then gcc_cv_ld_soname=yes ld_soname_option='-install_name' ;; + # Solaris 2 ld always supports -h. It also supports --soname for GNU + # ld compatiblity since some Solaris 10 update. + *-*-solaris2*) + gcc_cv_ld_soname=yes + ld_soname_option='-h' + ;; esac fi # Don't AC_DEFINE result, only used in jit/Make-lang.in so far. I've also checked that the original Solaris 10 release didn't support ld -soname, so it's safer to always use the Solaris-native -h option instead. Rainer
> Does this fix the jit linker issues on OS X and Solaris?
The patch fails to bootstrap on x86_64-apple-darwin17. gcc/config.log says:
gcc_cv_ld_version_script=no
ld_version_script_option='--version-script’
gcc/Makefile says:
LD_VERSION_SCRIPT_OPTION = --version-script
LD_SONAME_OPTION = -install_name
which makes it try to link with:
-Wl,--version-script,../../trunk/gcc/jit/libgccjit.map \
-Wl,-install_name,libgccjit.so.0
which fails with: ld: unknown option: --version-script
I think the patch to gcc/configure.ac should be:
+AC_MSG_CHECKING(linker --version-script option)
+gcc_cv_ld_version_script=no
+ld_version_script_option=''
FX
I can confirm that, with the attached revised patch, a bootstrap with --enable-languages=c,c++,jit --enable-host-shared is successful on macOS. FX
Hi David, Any news on that patch? Cheers, FX
On Wed, 2018-02-14 at 23:36 +0100, FX wrote: > I can confirm that, with the attached revised patch, a bootstrap with > --enable-languages=c,c++,jit --enable-host-shared is successful on > macOS. > > FX Looks good to me; thanks for fixing. Release managers: I'd like to apply FX's patch here: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00881/patch to trunk, to fix the build of jit on OS X, and to make it easier to fix it on Solaris. This involves touching gcc/configure.ac (and configure). I've successfully bootstrapped and regression-tested it on x86_64-pc- linux-gnu. FX reports above that it fixes the build on macOS, and Rainer has an (untested) patch on top of it that ought to fix the build on Solaris: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00835.html We're in stage 4, and the two bugs in question: PR jit/64089 ("libgccjit.so.0.0.1 linkage failure on darwin") https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64089 PR jit/84288 ("Support jit on Solaris") https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84288 aren't regressions. However, I believe this is a low-risk patch, and is mostly confined to jit (and those targets). Is it OK for trunk now, or does this need to wait until next stage 1? Thanks Dave
On Fri, Mar 09, 2018 at 09:14:13AM -0500, David Malcolm wrote: > On Wed, 2018-02-14 at 23:36 +0100, FX wrote: > > I can confirm that, with the attached revised patch, a bootstrap with > > --enable-languages=c,c++,jit --enable-host-shared is successful on > > macOS. > > > > FX > > Looks good to me; thanks for fixing. > > > Release managers: > > I'd like to apply FX's patch here: > https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00881/patch > to trunk, to fix the build of jit on OS X, and to make it easier to fix > it on Solaris. > > Is it OK for trunk now, or does this need to wait until next stage 1? Ok for trunk now. Jakub
Hi David, > Release managers: > > I'd like to apply FX's patch here: > https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00881/patch > to trunk, to fix the build of jit on OS X, and to make it easier to fix > it on Solaris. > > This involves touching gcc/configure.ac (and configure). > > I've successfully bootstrapped and regression-tested it on x86_64-pc- > linux-gnu. FX reports above that it fixes the build on macOS, and > Rainer has an (untested) patch on top of it that ought to fix the build > on Solaris: > https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00835.html I've now tested the patch (together with the one from PR jit/84288 for several remaining issues). I've needed another snippet for Solaris/SPARC which links libkstat into xgcc and needs it in libgccjit.so, too. Bootstrapped without regressions on i386-pc-solaris2.11 and sparc-sun-solaris2.11. Ok for mainline? Rainer
On Tue, 2018-03-20 at 09:38 +0100, Rainer Orth wrote: > Hi David, > > > Release managers: > > > > I'd like to apply FX's patch here: > > https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00881/patch > > to trunk, to fix the build of jit on OS X, and to make it easier to > > fix > > it on Solaris. > > > > This involves touching gcc/configure.ac (and configure). > > > > I've successfully bootstrapped and regression-tested it on x86_64- > > pc- > > linux-gnu. FX reports above that it fixes the build on macOS, and > > Rainer has an (untested) patch on top of it that ought to fix the > > build > > on Solaris: > > https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00835.html > > I've now tested the patch (together with the one from PR jit/84288 > for > several remaining issues). I've needed another snippet for > Solaris/SPARC which links libkstat into xgcc and needs it in > libgccjit.so, too. Bootstrapped without regressions on > i386-pc-solaris2.11 and sparc-sun-solaris2.11. FWIW I've successfully tested this on x86_64-pc-linux-gnu (regenerating the gcc/configure), and, as jit maintainer, it looks good to me [1], though it may still need RM approval given stage 4. Dave [1] ...though I have a slight preference for listing $(EXTRA_GCC_LIBS) on the same line as $(EXTRA_GCC_OBJS) in jit/Make- lang.in, so that these two items needed to embed the driver code into the libgccjit shared library are visually grouped together. > Ok for mainline? > > Rainer >
Hi Malcolm, >> I've now tested the patch (together with the one from PR jit/84288 >> for >> several remaining issues). I've needed another snippet for >> Solaris/SPARC which links libkstat into xgcc and needs it in >> libgccjit.so, too. Bootstrapped without regressions on >> i386-pc-solaris2.11 and sparc-sun-solaris2.11. > > FWIW I've successfully tested this on x86_64-pc-linux-gnu (regenerating > the gcc/configure), and, as jit maintainer, it looks good to me [1], > though it may still need RM approval given stage 4. thanks for trying this. > [1] ...though I have a slight preference for listing > $(EXTRA_GCC_LIBS) on the same line as $(EXTRA_GCC_OBJS) in jit/Make- > lang.in, so that these two items needed to embed the driver code into > the libgccjit shared library are visually grouped together. I've selected the location of $(EXTRA_GCC_LIBS) in the link line to match what gcc/Makefile.in does for xgcc etc. Rainer
On Wed, 2018-03-21 at 00:39 +0100, Rainer Orth wrote: > Hi Malcolm, > > > > I've now tested the patch (together with the one from PR > > > jit/84288 > > > for > > > several remaining issues). I've needed another snippet for > > > Solaris/SPARC which links libkstat into xgcc and needs it in > > > libgccjit.so, too. Bootstrapped without regressions on > > > i386-pc-solaris2.11 and sparc-sun-solaris2.11. > > > > FWIW I've successfully tested this on x86_64-pc-linux-gnu > > (regenerating > > the gcc/configure), and, as jit maintainer, it looks good to me > > [1], > > though it may still need RM approval given stage 4. > > thanks for trying this. > > > [1] ...though I have a slight preference for listing > > $(EXTRA_GCC_LIBS) on the same line as $(EXTRA_GCC_OBJS) in > > jit/Make- > > lang.in, so that these two items needed to embed the driver code > > into > > the libgccjit shared library are visually grouped together. > > I've selected the location of $(EXTRA_GCC_LIBS) in the link line to > match what gcc/Makefile.in does for xgcc etc. Indeed, I don't want to bikeshed it - I care much more about whether it works ;)
On Tue, 20 Mar 2018, David Malcolm wrote: > On Wed, 2018-03-21 at 00:39 +0100, Rainer Orth wrote: > > Hi Malcolm, > > > > > > I've now tested the patch (together with the one from PR > > > > jit/84288 > > > > for > > > > several remaining issues). I've needed another snippet for > > > > Solaris/SPARC which links libkstat into xgcc and needs it in > > > > libgccjit.so, too. Bootstrapped without regressions on > > > > i386-pc-solaris2.11 and sparc-sun-solaris2.11. > > > > > > FWIW I've successfully tested this on x86_64-pc-linux-gnu > > > (regenerating > > > the gcc/configure), and, as jit maintainer, it looks good to me > > > [1], > > > though it may still need RM approval given stage 4. > > > > thanks for trying this. > > > > > [1] ...though I have a slight preference for listing > > > $(EXTRA_GCC_LIBS) on the same line as $(EXTRA_GCC_OBJS) in > > > jit/Make- > > > lang.in, so that these two items needed to embed the driver code > > > into > > > the libgccjit shared library are visually grouped together. > > > > I've selected the location of $(EXTRA_GCC_LIBS) in the link line to > > match what gcc/Makefile.in does for xgcc etc. > > Indeed, I don't want to bikeshed it - I care much more about whether it > works ;) I'm fine with this patch. Richard.
diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 6c37e46..903da58 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1116,6 +1116,10 @@ endif LANG_MAKEFRAGS = @all_lang_makefrags@ +# Used by gcc/jit/Make-lang.in +LD_VERSION_SCRIPT_OPTION = @ld_version_script_option@ +LD_SONAME_OPTION = @ld_soname_option@ + # Flags to pass to recursive makes. # CC is set by configure. # ??? The choices here will need some experimenting with. diff --git a/gcc/configure.ac b/gcc/configure.ac index b7f9728..265920c 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -3640,6 +3640,44 @@ if test x"$gcc_cv_ld_static_dynamic" = xyes; then fi AC_MSG_RESULT($gcc_cv_ld_static_dynamic) +AC_MSG_CHECKING(linker --version-script option) +gcc_cv_ld_version_script=no +ld_version_script_option='--version-script' +if test $in_tree_ld = yes || test x"$gnu_ld" = xyes; then + gcc_cv_ld_version_script=yes + ld_version_script_option='--version-script' +elif test x$gcc_cv_ld != x; then + case "$target" in + # Solaris 2 ld always supports -M. It also supports a subset of + # --version-script since Solaris 11.4, but requires + # -z gnu-version-script-compat to activate. + *-*-solaris2*) + gcc_cv_ld_version_script=yes + ld_version_script_option='-M' + ;; + esac +fi +# Don't AC_DEFINE result, only used in jit/Make-lang.in so far. +AC_MSG_RESULT($gcc_cv_ld_version_script) +AC_SUBST(ld_version_script_option) + +AC_MSG_CHECKING(linker soname option) +gcc_cv_ld_soname=no +if test $in_tree_ld = yes || test x"$gnu_ld" = xyes; then + gcc_cv_ld_soname=yes + ld_soname_option='-soname' +elif test x$gcc_cv_ld != x; then + case "$target" in + *-*-darwin*) + gcc_cv_ld_soname=yes + ld_soname_option='-install_name' + ;; + esac +fi +# Don't AC_DEFINE result, only used in jit/Make-lang.in so far. +AC_MSG_RESULT($gcc_cv_ld_soname) +AC_SUBST(ld_soname_option) + if test x"$demangler_in_ld" = xyes; then AC_MSG_CHECKING(linker --demangle support) gcc_cv_ld_demangle=no diff --git a/gcc/jit/Make-lang.in b/gcc/jit/Make-lang.in index d4362a9..ba78f8e 100644 --- a/gcc/jit/Make-lang.in +++ b/gcc/jit/Make-lang.in @@ -51,6 +51,19 @@ LIBGCCJIT_FILENAME = \ LIBGCCJIT_LINKER_NAME_SYMLINK = $(LIBGCCJIT_LINKER_NAME) LIBGCCJIT_SONAME_SYMLINK = $(LIBGCCJIT_SONAME) +# Conditionalize the use of the LD_VERSION_SCRIPT_OPTION and +# LD_SONAME_OPTION depending if configure found them, using $(if) +# We have to define a COMMA here, otherwise the commas in the "true" +# result are treated as separators by the $(if). +COMMA := , +LIBGCCJIT_VERSION_SCRIPT_OPTION = \ + $(if $(LD_VERSION_SCRIPT_OPTION),\ + -Wl$(COMMA)$(LD_VERSION_SCRIPT_OPTION)$(COMMA)$(srcdir)/jit/libgccjit.map) + +LIBGCCJIT_SONAME_OPTION = \ + $(if $(LD_SONAME_OPTION), \ + -Wl$(COMMA)$(LD_SONAME_OPTION)$(COMMA)$(LIBGCCJIT_SONAME)) + jit: $(LIBGCCJIT_FILENAME) \ $(LIBGCCJIT_SYMLINK) \ $(LIBGCCJIT_LINKER_NAME_SYMLINK) \ @@ -85,8 +98,8 @@ $(LIBGCCJIT_FILENAME): $(jit_OBJS) \ $(jit_OBJS) libbackend.a libcommon-target.a libcommon.a \ $(CPPLIB) $(LIBDECNUMBER) $(LIBS) $(BACKENDLIBS) \ $(EXTRA_GCC_OBJS) \ - -Wl,--version-script=$(srcdir)/jit/libgccjit.map \ - -Wl,-soname,$(LIBGCCJIT_SONAME) + $(LIBGCCJIT_VERSION_SCRIPT_OPTION) \ + $(LIBGCCJIT_SONAME_OPTION) $(LIBGCCJIT_SONAME_SYMLINK): $(LIBGCCJIT_FILENAME) ln -sf $(LIBGCCJIT_FILENAME) $(LIBGCCJIT_SONAME_SYMLINK)
libgccjit fails to link on OS X and Solaris due to jit/Make-lang.in, due to the assumption there that the linker is GNU ld. Specifically, jit/Make-lang.in hardcodes the use of two options: --version-script and -soname. * on Darwin, --version-script doesn't seem to exist in the linker, and it uses -install_name rather than -soname. * on Solaris, ld doesn't support --version-script. However, the version script used for libgccjit.so doesn't use any gld extensions, so one can just use -M instead. This patch fixes these issues by using variables emitted by gcc's "configure" rather than hardcoding the options in jit/Make-lang.in. It's based on the first part of Rainer's patch for PR jit/84288, but I made the following changes: * the GNU ld case in configure.ac wasn't setting ld_version_script_option. I set it to "--version-script" for that case. * I moved the: LD_VERSION_SCRIPT_OPTION = @ld_version_script_option@ from gcc/jit/Make-lang.in to gcc/Makefile.in, as the Make-lang.in files aren't substituted, only the gcc/Makefile.in. Rainer: how did this work for you? * added LD_SONAME_OPTION, done in the same way * conditionalized the usage of the options in Make-lang.in to cope with empty LD_VERSION_SCRIPT_OPTION (as is presumably the case on OS X). I used ($if condition,then-part[,else-part]) for this. I had to add a $(COMMA) since the "then-part" contains commas, which need to be treated as part of the "then-part", rather than separators for the "else-part". Hopefully this is compatible with every "make" implementation that we support. Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. I lightly tested the not-recognized case by hacking up the configure.ac (on x86_64-pc-linux-gnu) and verifying that it links, and that a smoketest of jit.dg/test-factorial works. Does this fix the jit linker issues on OS X and Solaris? (I didn't include the autogenerate configure changes) gcc/ChangeLog: PR jit/64089 PR jit/84288 * Makefile.in (LD_VERSION_SCRIPT_OPTION, LD_SONAME_OPTION): New. * configure: Regenerate. * configure.ac ("linker --version-script option"): New. ("linker soname option"): New. gcc/jit/ChangeLog: PR jit/64089 PR jit/84288 * Make-lang.in (COMMA): New. (LIBGCCJIT_VERSION_SCRIPT_OPTION): New. (LIBGCCJIT_SONAME_OPTION): New. (jit): Move --version-script and -soname linker options to the above. --- gcc/Makefile.in | 4 ++++ gcc/configure.ac | 38 ++++++++++++++++++++++++++++++++++++++ gcc/jit/Make-lang.in | 17 +++++++++++++++-- 3 files changed, 57 insertions(+), 2 deletions(-)