[jit] Use the full name of the installed driver binary

[David Malcolm <dmalcolm at redhat dot com>]

On Fri, 2014-09-26 at 21:55 +0000, Joseph S. Myers wrote:
On Thu, 25 Sep 2014, David Malcolm wrote:
> 
> > Should this have the $(exeext) suffix seen in Makefile.in?
> >   $(target_noncanonical)-gcc-$(version)$(exeext)
> 
> Depends on whether that's needed for the pex code to find it.
> > As for (B), would it make sense to "bake in" the path to the binary into
> > the pex invocation, and hence to turn off PEX_SEARCH?  If so, presumably
> > I need to somehow expand the Makefile's value of $(bindir) into
> > internal-api.c, right?  (I tried this in configure.ac, but merely got
> > "$(exec_prefix)/bin" iirc).
> 
> An installation must be relocatable.  Thus, you can't just hardcode 
> looking in the configured prefix; you'd need to locate it relative to 
> libgccjit.so in some way (i.e. using make_relative_prefix, but I don't 
> know offhand how libgccjit.so would locate itself).
> 
> > A better long-term approach to this would be to extract the spec
> > machinery from gcc.c (perhaps into a "libdriver.a"?) and run it directly
> > from the jit library - but that's a rather involved patch, I suspect.
> 
> And you'd still need libgccjit.so to locate itself for proper 
> relocatability in finding other pieces such as assembler and linker.
> 
> > I wonder if the appropriate approach here is to have a single library
> > with multiple plugin backends e.g. one for the CPU, one for each GPU
> > family, with the ability to load multiple "backends" at once.
> 
> If you can get that working, sure.
> 
> > Unfortunately, "backend" is horribly overloaded here - I mean basically
> > all of gcc here, everything other than the libgccjit.h API seen by
> > client code.
> 
> (Though preferably as much as possible could be shared, i.e. properly 
> define the parts of GCC that need building separately for each target and 
> limit them as much as possible.  Joern's multi-target patches from 2010 
> that selectively built parts of GCC using namespaces while sharing others 
> without an obvious clear separation seemed very fragile.  For something 
> robust you either build everything separately for each target, or have a 
> well-defined separation between bits needing building separately and bits 
> that can be built once and ways to avoid non-obvious target dependencies 
> in bits built once.)

I've been experimenting with directly embedding the gcc.c driver code
in-process, but that patch was getting unwieldy, so for now, I'm going
with the simpler approach: just call the driver out-of-process,
specifying the full installed name:
  $(target_noncanonical)-gcc-$(version)$(exeext)
as expanded at configuration time, requiring it to be on the PATH.

Hopefully this addresses the last of the concerns raised in your initial
review; I'll do some more testing and then try to resubmit to the list
(I'm also thinking about breaking up internal-api.c/h, as they've become
rather large, into jit-recording/jit-playback.c/h)

Committed to branch dmalcolm/jit:

gcc/ChangeLog.jit:
	* Makefile.in (site.exp): When constructing site.exp, add a line
	to set "bindir".
	* configure.ac: Generate a gcc-driver-name.h file containing
	GCC_DRIVER_NAME for the benefit of jit/internal-api.c.
	* configure: Regenerate.

gcc/jit/ChangeLog.jit:
	* docs/internals/index.rst
	(Using a working copy without installing): Rename to...
	(Using a working copy without installing every time): ...this, and
	update to reflect the need to have installed the driver binary
	when running directly from a build directory.
	(Running the test suite): Add PATH setting to the example.
	* docs/intro/install.rst ("Hello world"): Likewise.
	* internal-api.c: Include new autogenerated header
	"gcc-driver-name.h".
	(gcc::jit::playback::context::compile): Rather than looking for a
	"gcc" on the path, look for GCC_DRIVER_NAME from gcc-driver-name.h,
	as created by the configure script, so that we are using one for
	the correct target.

gcc/testsuite/ChangeLog.jit:
	* jit.dg/jit.exp (jit-dg-test): Prepend the installed bindir to
	the PATH before invoking built binaries using the library, so that
	the library can find the driver.  Restore the PATH immediately
	afterwards.
---
 gcc/ChangeLog.jit                |  8 +++++
 gcc/Makefile.in                  |  1 +
 gcc/configure                    |  6 ++++
 gcc/configure.ac                 |  6 ++++
 gcc/jit/ChangeLog.jit            | 16 ++++++++++
 gcc/jit/docs/internals/index.rst | 64 ++++++++++++++++++++++++++++++++++++----
 gcc/jit/docs/intro/install.rst   | 36 ++++++++++++++--------
 gcc/jit/internal-api.c           | 10 +++++--
 gcc/testsuite/ChangeLog.jit      |  7 +++++
 gcc/testsuite/jit.dg/jit.exp     | 14 +++++++++
 10 files changed, 147 insertions(+), 21 deletions(-)

diff --git a/gcc/ChangeLog.jit b/gcc/ChangeLog.jit
index e71f7c4..ca73c04 100644
--- a/gcc/ChangeLog.jit
+++ b/gcc/ChangeLog.jit
@@ -1,3 +1,11 @@
+2014-10-07  David Malcolm  <dmalcolm@redhat.com>
+
+	* Makefile.in (site.exp): When constructing site.exp, add a line
+	to set "bindir".
+	* configure.ac: Generate a gcc-driver-name.h file containing
+	GCC_DRIVER_NAME for the benefit of jit/internal-api.c.
+	* configure: Regenerate.
+
 2014-10-03  David Malcolm  <dmalcolm@redhat.com>
 
 	* diagnostic.c (diagnostic_finish): Free the memory for
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 069a6e8..f5e3d4c 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3504,6 +3504,7 @@ 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/configure b/gcc/configure
index b943c20..8ec141f 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -28190,6 +28190,12 @@ _ACEOF
 
 fi
 
+# Generate gcc-driver-name.h containing GCC_DRIVER_NAME for the benefit
+# of jit/internal-api.c.
+cat > gcc-driver-name.h <<EOF
+#define GCC_DRIVER_NAME "${target_noncanonical}-gcc-${gcc_BASEVER}${exeext}"
+EOF
+
 # Configure the subdirectories
 # AC_CONFIG_SUBDIRS($subdirs)
 
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 7319f1c..62e5ad7 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5679,6 +5679,12 @@ if test x"${LINKER_HASH_STYLE}" != x; then
                                          [The linker hash style])
 fi
 
+# Generate gcc-driver-name.h containing GCC_DRIVER_NAME for the benefit
+# of jit/internal-api.c.
+cat > gcc-driver-name.h <<EOF
+#define GCC_DRIVER_NAME "${target_noncanonical}-gcc-${gcc_BASEVER}${exeext}"
+EOF
+
 # Configure the subdirectories
 # AC_CONFIG_SUBDIRS($subdirs)
 
diff --git a/gcc/jit/ChangeLog.jit b/gcc/jit/ChangeLog.jit
index ac8f28d..07438b9 100644
--- a/gcc/jit/ChangeLog.jit
+++ b/gcc/jit/ChangeLog.jit
@@ -1,3 +1,19 @@
+2014-10-07  David Malcolm  <dmalcolm@redhat.com>
+
+	* docs/internals/index.rst
+	(Using a working copy without installing): Rename to...
+	(Using a working copy without installing every time): ...this, and
+	update to reflect the need to have installed the driver binary
+	when running directly from a build directory.
+	(Running the test suite): Add PATH setting to the example.
+	* docs/intro/install.rst ("Hello world"): Likewise.
+	* internal-api.c: Include new autogenerated header
+	"gcc-driver-name.h".
+	(gcc::jit::playback::context::compile): Rather than looking for a
+	"gcc" on the path, look for GCC_DRIVER_NAME from gcc-driver-name.h,
+	as created by the configure script, so that we are using one for
+	the correct target.
+
 2014-09-26  David Malcolm  <dmalcolm@redhat.com>
 
 	* internal-api.h (gcc::jit::recording::context): Convert field
diff --git a/gcc/jit/docs/internals/index.rst b/gcc/jit/docs/internals/index.rst
index 9765862..7823b12 100644
--- a/gcc/jit/docs/internals/index.rst
+++ b/gcc/jit/docs/internals/index.rst
@@ -18,17 +18,66 @@
 Internals
 =========
 
-Using a working copy without installing
----------------------------------------
+Using a working copy without installing every time
+--------------------------------------------------
 When directly working on the library you can avoid needing to install to
-test changes.
+test every change.
+
+You need to do a ``make install`` of the ``gcc`` subdirectory to install
+the driver binary (the top-level ``gcc`` binary).  This is used internally
+by the library for converting from .s assembler files to .so shared
+libraries.  Specifically, it looks for an executable on the ``$PATH`` with
+a name expanded by the ``configure`` script from
+``${target_noncanonical}-gcc-${gcc_BASEVER}${exeext}``,
+such as ``x86_64-unknown-linux-gnu-gcc-5.0.0``.
+
+For example, if you configured with a prefix of ``$PREFIX`` like this:
+
+.. code-block:: bash
+
+  mkdir build
+  mkdir install
+  PREFIX=$(pwd)/install
+  cd build
+  ../src/configure \
+     --enable-host-shared \
+     --enable-languages=jit \
+     --disable-bootstrap \
+     --enable-checking=release \
+     --prefix=$PREFIX
+
+then you can install (once) to ensure that ``$PREFIX/bin/`` is populated:
+
+.. code-block:: console
+
+  [build]$ ll ../install/bin/*gcc*
+  -rwxr-xr-x. 3 david david 2733458 Oct  6 14:25 ../install/bin/gcc
+  -rwxr-xr-x. 2 david david  136921 Oct  6 14:25 ../install/bin/gcc-ar
+  -rwxr-xr-x. 2 david david  136857 Oct  6 14:25 ../install/bin/gcc-nm
+  -rwxr-xr-x. 2 david david  136869 Oct  6 14:25 ../install/bin/gcc-ranlib
+  -rwxr-xr-x. 3 david david 2733458 Oct  6 14:25 ../install/bin/x86_64-unknown-linux-gnu-gcc
+  -rwxr-xr-x. 3 david david 2733458 Oct  6 14:25 ../install/bin/x86_64-unknown-linux-gnu-gcc-5.0.0
+  -rwxr-xr-x. 2 david david  136921 Oct  6 14:25 ../install/bin/x86_64-unknown-linux-gnu-gcc-ar
+  -rwxr-xr-x. 2 david david  136857 Oct  6 14:25 ../install/bin/x86_64-unknown-linux-gnu-gcc-nm
+  -rwxr-xr-x. 2 david david  136869 Oct  6 14:25 ../install/bin/x86_64-unknown-linux-gnu-gcc-ranlib
+
+Note the presence above of ``../install/bin/x86_64-unknown-linux-gnu-gcc``.
 
 When building code using the API you need to ensure that ``-I`` points to
 the directory containing ``libgccjit.h`` and ``-L`` points to the
 directory containing the built library.
 
-You'll need to manually set ``LD_LIBRARY_PATH`` to the directory containing
-``libgccjit.so`` when running binaries (or debugging them).
+When running binaries (or debugging them), you'll need to manually set
+``LD_LIBRARY_PATH`` to the directory containing ``libgccjit.so``, and
+``PATH`` needs to contain the path to the installed binaries.
+
+and then you can run from a built (but not installed) copy:
+
+.. code-block:: console
+
+  [gcc]$ PATH=../../install/bin:$PATH LD_LIBRARY_PATH=. ./testsuite/jit/test-factorial.exe
+
+without needing to reinstall everything for every tweak to the library.
 
 Running the test suite
 ----------------------
@@ -68,7 +117,10 @@ and once a test has been compiled, you can debug it directly:
 
 .. code-block:: console
 
-   [gcc] $ LD_LIBRARY_PATH=. gdb testsuite/jit/test-factorial.exe
+   [gcc] $ PATH=../../install/bin:$PATH \
+           LD_LIBRARY_PATH=. \
+             gdb --args \
+               testsuite/jit/test-factorial.exe
 
 
 Overview of code structure
diff --git a/gcc/jit/docs/intro/install.rst b/gcc/jit/docs/intro/install.rst
index 0e14db0..a0641b7 100644
--- a/gcc/jit/docs/intro/install.rst
+++ b/gcc/jit/docs/intro/install.rst
@@ -189,17 +189,6 @@ specify the compilation and linkage flags:
       -o jit-hello-world \
       $(pkg-config libgccjit --cflags --libs)
 
-and ensure that `LD_LIBRARY_PATH` is set appropriate when running the
-built program, so that it can locate and dynamically link against
-`libgccjit.so`:
-
-.. code-block:: console
-
-  # Run the built program:
-  $ export LD_LIBRARY_PATH=$PREFIX/lib
-  $ ./jit-hello-world
-  hello world
-
 This is equivalent to handcoding the include and library paths with `-I`
 and `-L` and specifying `-lgccjit` (i.e. linkage against libgccjit):
 
@@ -211,5 +200,28 @@ and `-L` and specifying `-lgccjit` (i.e. linkage against libgccjit):
       -lgccjit \
       -I$PREFIX/include -L$PREFIX/lib
 
-  $ LD_LIBRARY_PATH=$PREFIX/lib ./jit-hello-world
+When running the built test program against a locally-built tree, two
+environment variables need to be set up:
+
+* `LD_LIBRARY_PATH` needs to be set up appropriately so that the dynamic
+  linker can locate the `libgccjit.so`
+
+* `PATH` needs to include the `bin` subdirectory below the installation
+  prefix, so that the library can locate a driver binary.  This is used
+  internally by the library for converting from .s assembler files to
+  .so shared libraries.
+
+  ..
+     Specifically, it looks for a name expanded from
+     ``${target_noncanonical}-gcc-${gcc_BASEVER}${exeext}`` on the
+     ``$PATH``, such as ``x86_64-unknown-linux-gnu-gcc-5.0.0``).
+
+For example, if you configured with a prefix of ``$PREFIX`` like above,
+you need an invocation like this:
+
+.. code-block:: console
+
+  $ LD_LIBRARY_PATH=$PREFIX/lib:$LD_LIBRARY_PATH \
+    PATH=$PREFIX/bin:$PATH \
+    ./jit-hello-world
   hello world
diff --git a/gcc/jit/internal-api.c b/gcc/jit/internal-api.c
index 8ef9af9..6849403 100644
--- a/gcc/jit/internal-api.c
+++ b/gcc/jit/internal-api.c
@@ -43,6 +43,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "stor-layout.h"
 #include "print-tree.h"
 #include "gimplify.h"
+#include "gcc-driver-name.h"
 
 #include <pthread.h>
 
@@ -4998,7 +4999,9 @@ compile ()
   /* Gross hacks follow:
      We have a .s file; we want a .so file.
      We could reuse parts of gcc/gcc.c to do this.
-     For now, just use the /usr/bin/gcc on the system...
+     For now, just use the driver binary from the install, as
+     named in gcc-driver-name.h
+     e.g. "x86_64-unknown-linux-gnu-gcc-5.0.0".
    */
   {
     auto_timevar assemble_timevar (TV_ASSEMBLE);
@@ -5006,8 +5009,9 @@ compile ()
     const char *argv[6];
     int exit_status = 0;
     int err = 0;
+    const char *gcc_driver_name = GCC_DRIVER_NAME;
 
-    argv[0] = "gcc";
+    argv[0] = gcc_driver_name;
     argv[1] = "-shared";
     /* The input: assembler.  */
     argv[2] = m_path_s_file;
@@ -5018,7 +5022,7 @@ compile ()
     argv[5] = NULL;
 
     errmsg = pex_one (PEX_SEARCH, /* int flags, */
-		      "gcc", /* const char *executable */
+		      gcc_driver_name,
 		      const_cast<char * const *> (argv),
 		      ctxt_progname, /* const char *pname */
 		      NULL, /* const char *outname */
diff --git a/gcc/testsuite/ChangeLog.jit b/gcc/testsuite/ChangeLog.jit
index 4572c3d..798f7c9 100644
--- a/gcc/testsuite/ChangeLog.jit
+++ b/gcc/testsuite/ChangeLog.jit
@@ -1,3 +1,10 @@
+2014-10-07  David Malcolm  <dmalcolm@redhat.com>
+
+	* jit.dg/jit.exp (jit-dg-test): Prepend the installed bindir to
+	the PATH before invoking built binaries using the library, so that
+	the library can find the driver.  Restore the PATH immediately
+	afterwards.
+
 2014-09-24  David Malcolm  <dmalcolm@redhat.com>
 
 	* ChangeLog.jit: Add copyright footer.
diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
index 7986185..4672063 100644
--- a/gcc/testsuite/jit.dg/jit.exp
+++ b/gcc/testsuite/jit.dg/jit.exp
@@ -92,12 +92,26 @@ proc jit-dg-test { prog do_what extra_tool_flags } {
     #
     unsetenv GCC_EXEC_PREFIX
 
+    # libgccjit uses the driver to convert .s files to .so libraries
+    # via its *installed* name, the expansion of:
+    #   ${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
+    global env
+    global bindir
+    set old_path $env(PATH)
+    setenv "PATH" $bindir:$env(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
     # use that to invoke the built code:
     set result [host_execute $output_file]
     verbose "result: $result"
 
+    # Restore PATH
+    setenv "PATH" $old_path
+
     restore_ld_library_path_env_vars
 }
 
-- 
1.7.11.7