diff mbox

Avoid the need to install when running the jit testsuite

Message ID 1413562945-14129-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Oct. 17, 2014, 4:22 p.m. UTC 
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(-)

Comments

Joseph Myers Oct. 17, 2014, 5:56 p.m. UTC | #1 
On Fri, 17 Oct 2014, David Malcolm wrote:

> +# 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 $< $@

I believe $(LN_S) would be normal, though (a) I don't see it being used 
anywhere, despite the definition, and (b) while the GNU Coding Standards 
still say "If you use symbolic links, you should implement a fallback for 
systems that don't have symbolic links." I'm doubtful that's of practical 
relevance to systems people are building GCC on any more.

I don't have any comments on the other parts of this patch.
diff mbox

Patch

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
 }