On Wed, 2014-10-15 at 17:29 -0400, David Malcolm wrote: > On Wed, 2014-10-15 at 14:51 -0600, Jeff Law wrote: > > On 10/15/14 12:25, David Malcolm wrote: > > > On Wed, 2014-10-15 at 11:36 -0600, Jeff Law wrote: > > >> On 10/13/14 11:45, David Malcolm wrote: > > >>> gcc/ChangeLog: > > >>> * configure.ac (gcc_version): Expose this value for use via > > >>> AC_SUBST, since the jit code needs it within the new file > > >>> libgccjit.pc.in. > > >>> (doc_build_sys): New variable, set to "sphinx" if > > >>> sphinx is installed, falling back to "texinfo" otherwise. > > >>> (gcc-driver-name.h): Generate a gcc-driver-name.h file > > >>> containing > > >>> GCC_DRIVER_NAME for the benefit of jit/jit-playback.c. > > >>> * configure: Regenerate. > > >>> * Makefile.in (doc_build_sys): New. > > >>> (bindir): New. > > >>> (pkgconfigdir): New. > > >>> (installdirs): Add creation of $(DESTDIR)$(pkgconfigdir). > > >>> (site.exp): When constructing site.exp, add a line to set > > >>> "bindir". > > >> Mostly OK. Though a couple questions. > > >> > > >> Why do we need pkgconfig and why do you need a bindir in site.exp? > [...snip pkg-config discussion...]
> > As for the "bindir" in site.exp, Joseph asked me when the library > > > invokes a driver to convert from .s to .so to: > > > > > > On Tue, 2014-09-23 at 23:27 +0000, Joseph S. Myers wrote: > > >> * use the $(target_noncanonical)-gcc-$(version) name for the > > >> driver rather than plain "gcc", to maximise the chance that it > > >> is actually the same compiler the JIT library was built for (I > > >> realise you may not actually depend on it being the same > > >> compiler, but that does seem best; in principle in future it > > >> should be possible to load multiple copies of the JIT library > > >> to JIT for different targets, so that code for an offload > > >> accelerator can go through the JIT). > > > ( https://gcc.gnu.org/ml/jit/2014-q3/msg00033.html ) > > > > > > This full name is used when *installing* the driver, but doesn't exist > > > within the build directory. > > > Hence when running the library, the installation bindir needs to be in > > > the PATH. In particular, (in > > > https://gcc.gnu.org/ml/jit/2014-q4/msg00005.html ) when running the jit > > > testsuite we rely on the driver having been installed, and in jit.exp we > > > need to temporarily prepend the installation bindir onto the front of > > > PATH when running test programs linked against libgccjit.so. Hence we > > > need to know what bindir is from expect, hence we add it to site.exp. > > So if I'm reading all this correctly, what's being implied is that > > testing is done using the installed bits rather than the in-build-tree > > bits? We really need this to run without having been installed. > > One approach here might be to do make a copy of the driver binary with > the final name within the *build* dir (e.g. > "x86_64-unknown-linux-gnu-gcc-5.0.0"). > > Another might be the "run the driver code in-process" approach I dabbled > with here: > https://gcc.gnu.org/ml/jit/2014-q3/msg00049.html > though that's some way from working, and is more invasive (no-one > replied to that email) The first approach seems to work well, and avoids the need to "make install" before running the testsuite. Jeff, Joseph: how does the following look? (not yet committed to the branch) Avoid discrepancy between the installed name of the driver as used by libgccjit (e.g. "x86_64-unknown-linux-gnu-gcc-5.0.0") and that within the builddir by adding a simple symlink to xgcc within the builddir, with the install-time name. When running the testsuite, set PATH and LIBRARY_PATH when invoking built binaries that use libgccjit so that it can find the driver, and so that when the driver invokes the linker, the linker can find libgcc, libgcc_s, crtbeginS.o, etc, finding them all in the builddir via "xgcc" via libgloss. This avoids the need for installation when running the testsuite, whilst avoiding "baking in" any paths into the built library. This patch doesn't touch the documentation (I plan to do that as a followup). gcc/ChangeLog.jit: * Makefile.in (FULL_DRIVER_NAME): New variable, adapted from the install-driver target. New target, a symlink within the builddir, linked to "xgcc", for use when running the JIT library from the builddir. (MOSTLYCLEANFILES): Add FULL_DRIVER_NAME. (install-driver): Use $(FULL_DRIVER_NAME) rather than spelling it out. (site.exp): Don't set "bindir", as this is no longer needed when running the jit testsuite. gcc/jit/ChangeLog.jit: * Make-lang.in (jit): Add $(FULL_DRIVER_NAME) as a dependency, so that the symlink is created for testing. * jit-playback.c (gcc::jit::playback::context::compile): Add "-fno-use-linker-plugin" when invoking the driver. Update error messages to talk about the "gcc driver" rather than the "gcc harness". To ease troubleshooting, add error messages giving the driver name and PATH to the error-handling code that fires when the driver can't be found. gcc/testsuite/ChangeLog.jit: * jit.dg/jit.exp (get_path_of_driver): New procedure. (jit-dg-test): Don't unsetenv GCC_EXEC_PREFIX, since jit-playback.c now adds -fno-use-linker-plugin to the driver cmdline sidestepping the builddir/installdir libtto_plugin naming issue. When setting up PATH so that the JIT library can invoke the driver by installation name, don't use the installation "bindir". Instead, simply use the location of xgcc as detected get_path_of_driver. In addition, set up LIBRARY_PATH so that the linker run from inside the JIT library can locate libgcc etc when building the .so, pointing it at the same directory. --- gcc/Makefile.in | 16 +++++++--- gcc/jit/Make-lang.in | 5 +++- gcc/jit/jit-playback.c | 24 ++++++++++++--- gcc/testsuite/jit.dg/jit.exp | 69 +++++++++++++++++++++++++++++++------------- 4 files changed, 85 insertions(+), 29 deletions(-) diff --git a/gcc/Makefile.in b/gcc/Makefile.in index f5e3d4c..523d1db 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1504,6 +1504,9 @@ BACKEND = libbackend.a main.o @TREEBROWSER@ libcommon-target.a libcommon.a \ # front-end checking. TREECHECKING = @TREECHECKING@ +# The full name of the driver on installation +FULL_DRIVER_NAME=$(target_noncanonical)-gcc-$(version)$(exeext) + MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ insn-output.c insn-recog.c insn-emit.c insn-extract.c insn-peep.c \ insn-attr.h insn-attr-common.h insn-attrtab.c insn-dfatab.c \ @@ -1511,7 +1514,7 @@ MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ tm-preds.h tm-constrs.h checksum-options \ tree-check.h min-insn-modes.c insn-modes.c insn-modes.h \ genrtl.h gt-*.h gtype-*.h gtype-desc.c gtyp-input.list \ - xgcc$(exeext) cpp$(exeext) \ + xgcc$(exeext) cpp$(exeext) $(FULL_DRIVER_NAME) \ $(EXTRA_PROGRAMS) gcc-cross$(exeext) \ $(SPECS) collect2$(exeext) gcc-ar$(exeext) gcc-nm$(exeext) \ gcc-ranlib$(exeext) \ @@ -1520,6 +1523,12 @@ MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ gengtype$(exeext) *.[0-9][0-9].* *.[si] *-checksum.c libbackend.a \ libcommon-target.a libcommon.a libgcc.mk +# This symlink makes the full installation name of the driver be available +# from within the *build* directory, for use when running the JIT library +# from there (e.g. when running its testsuite). +$(FULL_DRIVER_NAME): ./xgcc + $(LN) -s $< $@ + # # Language makefile fragments. @@ -3248,9 +3257,9 @@ install-driver: installdirs xgcc$(exeext) -rm -f $(DESTDIR)$(bindir)/$(GCC_INSTALL_NAME)$(exeext) -$(INSTALL_PROGRAM) xgcc$(exeext) $(DESTDIR)$(bindir)/$(GCC_INSTALL_NAME)$(exeext) -if [ "$(GCC_INSTALL_NAME)" != "$(target_noncanonical)-gcc-$(version)" ]; then \ - rm -f $(DESTDIR)$(bindir)/$(target_noncanonical)-gcc-$(version)$(exeext); \ + rm -f $(DESTDIR)$(bindir)/$(FULL_DRIVER_NAME); \ ( cd $(DESTDIR)$(bindir) && \ - $(LN) $(GCC_INSTALL_NAME)$(exeext) $(target_noncanonical)-gcc-$(version)$(exeext) ); \ + $(LN) $(GCC_INSTALL_NAME)$(exeext) $(FULL_DRIVER_NAME) ); \ fi -if [ ! -f gcc-cross$(exeext) ] \ && [ "$(GCC_INSTALL_NAME)" != "$(GCC_TARGET_INSTALL_NAME)" ]; then \ @@ -3504,7 +3513,6 @@ site.exp: ./config.status Makefile @echo "# add them to the last section" >> ./site.tmp @echo "set rootme \"`${PWD_COMMAND}`\"" >> ./site.tmp @echo "set srcdir \"`cd ${srcdir}; ${PWD_COMMAND}`\"" >> ./site.tmp - @echo "set bindir \"`cd ${bindir}; ${PWD_COMMAND}`\"" >> ./site.tmp @echo "set host_triplet $(host)" >> ./site.tmp @echo "set build_triplet $(build)" >> ./site.tmp @echo "set target_triplet $(target)" >> ./site.tmp diff --git a/gcc/jit/Make-lang.in b/gcc/jit/Make-lang.in index 3115b1d..ac179f4 100644 --- a/gcc/jit/Make-lang.in +++ b/gcc/jit/Make-lang.in @@ -51,7 +51,10 @@ LIBGCCJIT_FILENAME = \ LIBGCCJIT_LINKER_NAME_SYMLINK = $(LIBGCCJIT_LINKER_NAME) LIBGCCJIT_SONAME_SYMLINK = $(LIBGCCJIT_SONAME) -jit: $(LIBGCCJIT_FILENAME) $(LIBGCCJIT_SYMLINK) $(LIBGCCJIT_LINKER_NAME_SYMLINK) +jit: $(LIBGCCJIT_FILENAME) \ + $(LIBGCCJIT_SYMLINK) \ + $(LIBGCCJIT_LINKER_NAME_SYMLINK) \ + $(FULL_DRIVER_NAME) # Tell GNU make to ignore these if they exist. .PHONY: jit diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c index 6937408..74ddccc 100644 --- a/gcc/jit/jit-playback.c +++ b/gcc/jit/jit-playback.c @@ -1631,7 +1631,7 @@ compile () { auto_timevar assemble_timevar (TV_ASSEMBLE); const char *errmsg; - const char *argv[6]; + const char *argv[7]; int exit_status = 0; int err = 0; const char *gcc_driver_name = GCC_DRIVER_NAME; @@ -1643,8 +1643,18 @@ compile () /* The output: shared library. */ argv[3] = "-o"; argv[4] = m_path_so_file; + + /* Don't use the linker plugin. + If running with just a "make" and not a "make install", then we'd + run into + "fatal error: -fuse-linker-plugin, but liblto_plugin.so not found" + libto_plugin is a .la at build time, with it becoming installed with + ".so" suffix: i.e. it doesn't exist with a .so suffix until install + time. */ + argv[5] = "-fno-use-linker-plugin"; + /* pex argv arrays are NULL-terminated. */ - argv[5] = NULL; + argv[6] = NULL; errmsg = pex_one (PEX_SEARCH, /* int flags, */ gcc_driver_name, @@ -1656,7 +1666,7 @@ compile () &err); /* int *err*/ if (errmsg) { - add_error (NULL, "error invoking gcc harness: %s", errmsg); + add_error (NULL, "error invoking gcc driver: %s", errmsg); return NULL; } @@ -1665,8 +1675,14 @@ compile () if (exit_status || err) { add_error (NULL, - "error invoking gcc harness: exit_status: %i err: %i", + "error invoking gcc driver: exit_status: %i err: %i", exit_status, err); + add_error (NULL, + "whilst attempting to run a driver named: %s", + gcc_driver_name); + add_error (NULL, + "PATH was: %s", + getenv ("PATH")); return NULL; } } diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp index 76a1d9d..c85e2a5 100644 --- a/gcc/testsuite/jit.dg/jit.exp +++ b/gcc/testsuite/jit.dg/jit.exp @@ -172,6 +172,18 @@ set tests [lsort [concat $tests [find $srcdir/../jit/docs/examples *.c]]] verbose "tests: $tests" +# libgloss has found the driver (as "xgcc" or "gcc) and stored +# its full path as GCC_UNDER_TEST. +proc get_path_of_driver {} { + global GCC_UNDER_TEST + + verbose "GCC_UNDER_TEST: $GCC_UNDER_TEST" + set binary [lindex $GCC_UNDER_TEST 0] + verbose "binary: $binary" + + return [file dirname $binary] +} + proc jit-dg-test { prog do_what extra_tool_flags } { verbose "within jit-dg-test..." verbose " prog: $prog" @@ -206,31 +218,41 @@ proc jit-dg-test { prog do_what extra_tool_flags } { set ld_library_path "$base_dir/../../" set_ld_library_path_env_vars - # If running with just a "make" and not a "make install", then I was - # running into - # "fatal error: -fuse-linker-plugin, but liblto_plugin.so not found" - # emitted by the inner gcc invoked to convert the .s into .so - # This appears to be due to not installing the built compiler; - # libto_plugin is a .la at build time, with the .so becoming installed - # at install time; the "set_ld_library_path_env_vars" function from - # target-libpath.exp that I'm using to set LD_LIBRARY_PATH to find - # the library under test, libgccjit.so, was setting GCC_EXEC_PREFIX to - # the builddir, thus picking up the built-but-not-installed toolchain. - # Hacking in an "unsetenv GCC_EXEC_PREFIX" here fixes the issue, - # allowing quick running of testsuite without needing a full install. - # - unsetenv GCC_EXEC_PREFIX - # libgccjit uses the driver to convert .s files to .so libraries - # via its *installed* name, the expansion of: + # via its *installed* name, FULL_DRIVER_NAME # ${target_noncanonical}-gcc-${gcc_BASEVER}${exeext} # e.g. "x86_64-unknown-linux-gnu-gcc-5.0.0" - # looking for it on PATH. Hence we need to prepend the installation - # bindir to PATH when running the tests + # looking for it on PATH. Hence we need to prepend the location of + # that executable to PATH when running the tests + set dir_containing_driver [get_path_of_driver ] + verbose "dir_containing_driver: $dir_containing_driver" global env - global bindir set old_path $env(PATH) - setenv "PATH" $bindir:$env(PATH) + setenv "PATH" $dir_containing_driver:$old_path + verbose -log "PATH=[getenv PATH]" + + # We have: + # test-executables + # linked to -> libgccjit.so + # -> invokes driver: + # -> invokes the assembler + # -> invokes the linker + # We want to be able to run this from the builddir without installing + # but the linker needs to be able to locate various libraries, or we + # get: + # ld: cannot find crtbeginS.o: No such file or directory + # ld: cannot find -lgcc + # ld: cannot find -lgcc_s + # These can be found in the "gcc" subdir of the build. + # Hence to be able to run the testsuite without installing, we need + # to set or prepend the "gcc" subdir of the build to LIBRARY_PATH: + if { [info exists env(LIBRARY_PATH) ] } { + set old_library_path $env(LIBRARY_PATH) + setenv "LIBRARY_PATH" $dir_containing_driver:$old_library_path + } else { + setenv "LIBRARY_PATH" $dir_containing_driver + } + verbose -log "LIBRARY_PATH=[getenv LIBRARY_PATH]" # dejagnu.exp's host_execute has code to scrape out test results # from the DejaGnu C API and bring back into the tcl world, so we @@ -245,6 +267,13 @@ proc jit-dg-test { prog do_what extra_tool_flags } { # Restore PATH setenv "PATH" $old_path + # Restore LIBRARY_PATH + if { [info exists old_library_path] } { + setenv "LIBRARY_PATH" $old_library_path + } else { + unsetenv "LIBRARY_PATH" + } + restore_ld_library_path_env_vars } -- 1.8.5.3