This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 1/3] PR jit/64810: driver, arm, jit: configure-time default options


Currently the jit completely fails to work on arm.

The root cause is that the jit is ignoring configure-time
default options.

For example, on configuring arm with e.g.:
  --with-arch=armv7-a --with-float=hard --with-fpu=vfpv3-d16
these options are written into configargs.h as
"configure_default_options".  The target can provide the
macro OPTION_DEFAULT_SPECS, and the driver uses this to
expand configure_default_options; in the example above
it injects these arguments when invoking, say, cc1:
  -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16 -mtls-dialect=gnu

The issue here is that libgccjit isn't invoked by the driver:
when client code calls gcc_jit_context_compile, libgccjit
generates its own arguments to toplev::main .  libgccjit is currently
not providing the configure-time default options to itself
(leading e.g. to failures on ARM due to generating soft-float
assembler on a supposedly hard-float configuration: PR jit/64810).

My first attempt to solve this was to use configure_default_options
from libgccjit, but this doesn't work, given that the argument names
vary slightly between configure-time and cc1
(e.g. "--with-float" vs "-mfloat-abi").

The information on target-specific configure-time options the jit
needs is encoded per-target in the specs language (in the
OPTION_DEFAULT_SPECS macro), and only the driver understands the
specs language.

My second attempt to solve this was by giving xgcc a new
  --dump-configure-time-options
option, and using this during the build of the jit to build
an array of options for toplev::main.

This works, but would add a requirement that build==host when building
with the jit.  For the curious, the patch can be seen (sans ChangeLog)
at https://gcc.gnu.org/bugzilla/attachment.cgi?id=34612

Attached is the third approach: to embed enough of the driver in
libgccjit to be able to process OPTION_DEFAULT_SPECS and turn it
into options.  The jit does it once, caching the results.

This patch moves the driver's "main" function out of gcc.c into a new
gcc-main.c, so that I can link the bulk of the driver's code gcc.o into
libgccjit.so.

The driver code is almost completely untouched (apart from splitting it
out); I added a:
  free (decoded_options);
to do_self_spec since this showed up in valgrind as being
leaked: "decoded_options" is set up by decode_cmdline_options_to_array,
and is leaked when we exit do_self_spec.  It's a one-time leak, so I
could drop this "free" if it's regarded as risky.

I added a new function "driver_get_configure_time_options" to gcc.c,
for use by the jit, to do the minimal amount of spec-expansion work
needed to process OPTION_DEFAULT_SPECS; this isn't used by the normal
driver use-cases.

This patch is larger than I'd like for stage4, but unbreaks the jit on
arm (and avoids introducing a build==host requirement for the jit).

Successfully bootstrapped&regrtested on
x86_64-unknown-linux-gnu (Fedora 20).

Successful "make check-jit" for the following targets:

  aarch64-unknown-linux-gnu:
    All of "make check-jit" passes, apart from the known
    failure (PR jit/64752).
    No injected options.

  armv7l-unknown-linux-gnueabihf:
    Configured with:
      --with-arch=armv7-a --with-float=hard --with-fpu=vfpv3-d16
    Injected options were:
      configure_time_options[0]: -march=armv7-a
      configure_time_options[1]: -mfloat-abi=hard
      configure_time_options[2]: -mfpu=vfpv3-d16
      configure_time_options[3]: -mtls-dialect=gnu
    "make check-jit" goes from almost total failure to mostly
    passing, with:
      # of expected passes            5900
      # of unexpected failures        1
        (this is PR jit/64752).
      # of unresolved testcases       1
        UNRESOLVED: jit.dg/test-threads.c (test for excess errors)
      Saw various:
        (WARNING: Timed out executing test case)
      presumably the unresolved cases and missing passes are due to
      that.

  i686-pc-linux-gnu:
    All of "make check-jit" passes, apart from the known
    failure (PR jit/64752).
    Injected options were:
      configure_time_options[0]: -mtune=generic
      configure_time_options[1]: -march=pentiumpro

  powerpc64-unknown-linux-gnu (gcc110):
    All of "make check-jit" passes, apart from the known
    failure (PR jit/64752).
    No injected options.

  s390x-ibm-linux-gnu:
    All of "make check-jit" passes, apart from the known
    failure (PR jit/64752).
    No injected options.

  x86_64-unknown-linux-gnu:
    All of "make check-jit" passes.
    Injected options were:
       configure_time_options[0]: -mtune=generic
       configure_time_options[1]: -march=x86-64

OK for stage 4?

gcc/ChangeLog:
	PR jit/64810
	* Makefile.in (GCC_OBJS): Add gcc-main.o.
	* gcc-main.c: New file, containing "main" taken from gcc.c.
	* gcc.c (do_self_spec): Free decoded_options.
	(class driver): Move declaration to gcc.h.
	(main): Move declaration and implementation to new file
	gcc-main.c.
	(driver_get_configure_time_options): New function.
	* gcc.h (class driver): Move this declaration here, from
	gcc.c.
	(driver_get_configure_time_options): New declaration.

gcc/jit/ChangeLog:
	PR jit/64810
	* Make-lang.in (jit_OBJS): Add jit/jit-spec.o and gcc.o.
	(LIBGCCJIT_FILENAME): Add EXTRA_GCC_OBJS.
	* jit-playback.c: Include gcc.h.
	(gcc::jit::playback::context::compile): Move mutex acquisition
	to before the call to make_fake_args.
	(append_arg_from_driver): New function.
	(gcc::jit::playback::context::make_fake_args): On the first call,
	call into driver_get_configure_time_options to get configure-time
	default options and cache them.  Add them to the args for
	toplev::main.
	* jit-spec.c: New source file.
	* docs/internals/test-hello-world.exe.log.txt: Update to reflect
	above changes.
---
 gcc/Makefile.in                                    |  2 +-
 gcc/gcc-main.c                                     | 46 ++++++++++++
 gcc/gcc.c                                          | 84 ++++++++++------------
 gcc/gcc.h                                          | 38 ++++++++++
 gcc/jit/Make-lang.in                               |  8 ++-
 .../docs/internals/test-hello-world.exe.log.txt    |  9 ++-
 gcc/jit/jit-playback.c                             | 76 ++++++++++++++++++--
 gcc/jit/jit-spec.c                                 | 44 ++++++++++++
 8 files changed, 252 insertions(+), 55 deletions(-)
 create mode 100644 gcc/gcc-main.c
 create mode 100644 gcc/jit/jit-spec.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index add5416..de1f3b6 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1132,7 +1132,7 @@ CXX_TARGET_OBJS=@cxx_target_objs@
 FORTRAN_TARGET_OBJS=@fortran_target_objs@
 
 # Object files for gcc many-languages driver.
-GCC_OBJS = gcc.o ggc-none.o
+GCC_OBJS = gcc.o gcc-main.o ggc-none.o
 
 c-family-warn = $(STRICT_WARN)
 
diff --git a/gcc/gcc-main.c b/gcc/gcc-main.c
new file mode 100644
index 0000000..230ba48
--- /dev/null
+++ b/gcc/gcc-main.c
@@ -0,0 +1,46 @@
+/* "main" for the compiler driver.
+   Copyright (C) 1987-2015 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/* This source file contains "main" for the compiler driver.
+   All of the real work is done within gcc.c; we implement "main"
+   in here for the "gcc" binary so that gcc.o can be used in
+   libgccjit.so.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tm.h"
+#include "xregex.h"
+#include "obstack.h"
+#include "intl.h"
+#include "prefix.h"
+#include "gcc.h"
+
+/* Implement the top-level "main" within the driver in terms of
+   driver::main (implemented in gcc.c).  */
+
+extern int main (int, char **);
+
+int
+main (int argc, char **argv)
+{
+  driver d;
+
+  return d.main (argc, argv);
+}
diff --git a/gcc/gcc.c b/gcc/gcc.c
index f682c3b..00eea947 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -4681,6 +4681,8 @@ do_self_spec (const char *spec)
 	    }
 	}
 
+      free (decoded_options);
+
       alloc_switch ();
       switches[n_switches].part1 = 0;
     }
@@ -6861,52 +6863,6 @@ compare_files (char *cmpfile[])
   return ret;
 }
 
-/* The top-level "main" within the driver would be ~1000 lines long.
-   This class breaks it up into smaller functions and contains some
-   state shared by them.  */
-
-class driver
-{
- public:
-  int main (int argc, char **argv);
-
- private:
-  void set_progname (const char *argv0) const;
-  void expand_at_files (int *argc, char ***argv) const;
-  void decode_argv (int argc, const char **argv);
-  void global_initializations ();
-  void build_multilib_strings () const;
-  void set_up_specs () const;
-  void putenv_COLLECT_GCC (const char *argv0) const;
-  void maybe_putenv_COLLECT_LTO_WRAPPER () const;
-  void maybe_putenv_OFFLOAD_TARGETS () const;
-  void handle_unrecognized_options () const;
-  int maybe_print_and_exit () const;
-  bool prepare_infiles ();
-  void do_spec_on_infiles () const;
-  void maybe_run_linker (const char *argv0) const;
-  void final_actions () const;
-  int get_exit_code () const;
-
- private:
-  char *explicit_link_files;
-  struct cl_decoded_option *decoded_options;
-  unsigned int decoded_options_count;
-};
-
-/* Implement the top-level "main" within the driver in terms of
-   driver::main.  */
-
-extern int main (int, char **);
-
-int
-main (int argc, char **argv)
-{
-  driver d;
-
-  return d.main (argc, argv);
-}
-
 /* driver::main is implemented as a series of driver:: method calls.  */
 
 int
@@ -9433,3 +9389,39 @@ convert_white_space (char *orig)
   else
     return orig;
 }
+
+/* PR jit/64810.
+   Targets can provide configure-time default options in
+   OPTION_DEFAULT_SPECS.  The jit needs to access these, but
+   they are expressed in the spec language.
+
+   Run just enough of the driver to be able to expand these
+   specs, and then call the callback CB on each
+   such option.  The options strings are *without* a leading
+   '-' character e.g. ("march=x86-64").  Finally, clean up.  */
+
+void
+driver_get_configure_time_options (void (*cb) (const char *option,
+					       void *user_data),
+				   void *user_data)
+{
+  size_t i;
+
+  obstack_init (&obstack);
+  gcc_obstack_init (&opts_obstack);
+  n_switches = 0;
+
+  for (i = 0; i < ARRAY_SIZE (option_default_specs); i++)
+    do_option_spec (option_default_specs[i].name,
+		    option_default_specs[i].spec);
+
+  for (i = 0; (int) i < n_switches; i++)
+    {
+      gcc_assert (switches[i].part1);
+      (*cb) (switches[i].part1, user_data);
+    }
+
+  obstack_free (&opts_obstack, NULL);
+  obstack_free (&obstack, NULL);
+  n_switches = 0;
+}
diff --git a/gcc/gcc.h b/gcc/gcc.h
index 473cf6a..f10a103 100644
--- a/gcc/gcc.h
+++ b/gcc/gcc.h
@@ -23,6 +23,39 @@ along with GCC; see the file COPYING3.  If not see
 #include "version.h"
 #include "diagnostic-core.h"
 
+/* The top-level "main" within the driver would be ~1000 lines long.
+   This class breaks it up into smaller functions and contains some
+   state shared by them.  */
+
+class driver
+{
+ public:
+  int main (int argc, char **argv);
+
+ private:
+  void set_progname (const char *argv0) const;
+  void expand_at_files (int *argc, char ***argv) const;
+  void decode_argv (int argc, const char **argv);
+  void global_initializations ();
+  void build_multilib_strings () const;
+  void set_up_specs () const;
+  void putenv_COLLECT_GCC (const char *argv0) const;
+  void maybe_putenv_COLLECT_LTO_WRAPPER () const;
+  void maybe_putenv_OFFLOAD_TARGETS () const;
+  void handle_unrecognized_options () const;
+  int maybe_print_and_exit () const;
+  bool prepare_infiles ();
+  void do_spec_on_infiles () const;
+  void maybe_run_linker (const char *argv0) const;
+  void final_actions () const;
+  int get_exit_code () const;
+
+ private:
+  char *explicit_link_files;
+  struct cl_decoded_option *decoded_options;
+  unsigned int decoded_options_count;
+};
+
 /* The mapping of a spec function name to the C function that
    implements it.  */
 struct spec_function
@@ -55,4 +88,9 @@ extern int lang_specific_extra_outfiles;
 
 extern const char **outfiles;
 
+extern void
+driver_get_configure_time_options (void (*cb)(const char *option,
+					      void *user_data),
+				   void *user_data);
+
 #endif /* ! GCC_GCC_H */
diff --git a/gcc/jit/Make-lang.in b/gcc/jit/Make-lang.in
index e622690..44d0750 100644
--- a/gcc/jit/Make-lang.in
+++ b/gcc/jit/Make-lang.in
@@ -67,7 +67,9 @@ jit_OBJS = attribs.o \
 	jit/jit-playback.o \
 	jit/jit-result.o \
 	jit/jit-tempdir.o \
-	jit/jit-builtins.o
+	jit/jit-builtins.o \
+	jit/jit-spec.o \
+	gcc.o
 
 # Use strict warnings for this front end.
 jit-warn = $(STRICT_WARN)
@@ -77,10 +79,12 @@ jit-warn = $(STRICT_WARN)
 $(LIBGCCJIT_FILENAME): $(jit_OBJS) \
 	libbackend.a libcommon-target.a libcommon.a \
 	$(CPPLIB) $(LIBDECNUMBER) \
-	$(LIBDEPS) $(srcdir)/jit/libgccjit.map
+	$(LIBDEPS) $(srcdir)/jit/libgccjit.map \
+	$(EXTRA_GCC_OBJS)
 	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ -shared \
 	     $(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)
 
diff --git a/gcc/jit/docs/internals/test-hello-world.exe.log.txt b/gcc/jit/docs/internals/test-hello-world.exe.log.txt
index 876d830..a9abc10 100644
--- a/gcc/jit/docs/internals/test-hello-world.exe.log.txt
+++ b/gcc/jit/docs/internals/test-hello-world.exe.log.txt
@@ -60,10 +60,13 @@ JIT:    entering: bool gcc::jit::tempdir::create()
 JIT:     m_path_template: /tmp/libgccjit-XXXXXX
 JIT:     m_path_tempdir: /tmp/libgccjit-CKq1M9
 JIT:    exiting: bool gcc::jit::tempdir::create()
-JIT:    entering: void gcc::jit::playback::context::make_fake_args(vec<char*>*, const char*, vec<gcc::jit::recording::requested_dump>*)
-JIT:    exiting: void gcc::jit::playback::context::make_fake_args(vec<char*>*, const char*, vec<gcc::jit::recording::requested_dump>*)
 JIT:    entering: void gcc::jit::playback::context::acquire_mutex()
 JIT:    exiting: void gcc::jit::playback::context::acquire_mutex()
+JIT:    entering: void gcc::jit::playback::context::make_fake_args(vec<char*>*, const char*, vec<gcc::jit::recording::requested_dump>*)
+JIT:     reusing cached configure-time options
+JIT:     configure_time_options[0]: -mtune=generic
+JIT:     configure_time_options[1]: -march=x86-64
+JIT:    exiting: void gcc::jit::playback::context::make_fake_args(vec<char*>*, const char*, vec<gcc::jit::recording::requested_dump>*)
 JIT:    entering: toplev::main
 JIT:     argv[0]: ./test-hello-world.c.exe
 JIT:     argv[1]: /tmp/libgccjit-CKq1M9/fake.c
@@ -75,6 +78,8 @@ JIT:     argv[6]: --param
 JIT:     argv[7]: ggc-min-expand=0
 JIT:     argv[8]: --param
 JIT:     argv[9]: ggc-min-heapsize=0
+JIT:     argv[10]: -mtune=generic
+JIT:     argv[11]: -march=x86-64
 JIT:     entering: bool jit_langhook_init()
 JIT:     exiting: bool jit_langhook_init()
 JIT:     entering: void gcc::jit::playback::context::replay()
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index d2549a0..c75c076 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -63,6 +63,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "context.h"
 #include "fold-const.h"
 #include "debug.h"
+#include "gcc.h"
 
 #include "jit-common.h"
 #include "jit-logging.h"
@@ -1714,13 +1715,16 @@ compile ()
   auto_vec <recording::requested_dump> requested_dumps;
   m_recording_ctxt->get_all_requested_dumps (&requested_dumps);
 
+  /* Acquire the JIT mutex and set "this" as the active playback ctxt.  */
+  acquire_mutex ();
+
   auto_argvec fake_args;
   make_fake_args (&fake_args, ctxt_progname, &requested_dumps);
   if (errors_occurred ())
-    return;
-
-  /* Acquire the JIT mutex and set "this" as the active playback ctxt.  */
-  acquire_mutex ();
+    {
+      release_mutex ();
+      return;
+    }
 
   /* This runs the compiler.  */
   toplev toplev (false, /* use_TV_TOTAL */
@@ -2034,6 +2038,19 @@ playback::context::release_mutex ()
   pthread_mutex_unlock (&jit_mutex);
 }
 
+/* Callback used by gcc::jit::playback::context::make_fake_args when
+   invoking driver_get_configure_time_options.
+   Populate a vec <char * > with the configure-time options.  */
+
+static void
+append_arg_from_driver (const char *option, void *user_data)
+{
+  gcc_assert (option);
+  gcc_assert (user_data);
+  vec <char *> *argvec = static_cast <vec <char *> *> (user_data);
+  argvec->safe_push (concat ("-", option, NULL));
+}
+
 /* Build a fake argv for toplev::main from the options set
    by the user on the context .  */
 
@@ -2118,6 +2135,57 @@ make_fake_args (vec <char *> *argvec,
       }
   }
 
+  /* PR jit/64810: Add any target-specific default options
+     from OPTION_DEFAULT_SPECS, normally provided by the driver
+     in the non-jit case.
+
+     The target-specific code can define OPTION_DEFAULT_SPECS:
+     default command options in the form of spec macros for the
+     driver to expand ().
+
+     For cc1 etc, the driver processes OPTION_DEFAULT_SPECS and,
+     if not overriden, injects the defaults as extra arguments to
+     cc1 etc.
+     For the jit case, we need to add these arguments here.  The
+     input format (using the specs language) means that we have to run
+     part of the driver code here (driver_get_configure_time_options).
+
+     To avoid running the spec-expansion code every time, we just do
+     it the first time (via a function-static flag), saving the result
+     into a function-static vec.
+     This flag and vec are global state (i.e. per-process).
+     They are guarded by the jit mutex.  */
+  {
+    static bool have_configure_time_options = false;
+    static vec <char *> configure_time_options;
+
+    if (have_configure_time_options)
+      log ("reusing cached configure-time options");
+    else
+      {
+	have_configure_time_options = true;
+	log ("getting configure-time options from driver");
+	driver_get_configure_time_options (append_arg_from_driver,
+					   &configure_time_options);
+      }
+
+    int i;
+    char *opt;
+
+    if (get_logger ())
+      FOR_EACH_VEC_ELT (configure_time_options, i, opt)
+	log ("configure_time_options[%i]: %s", i, opt);
+
+    /* configure_time_options should now contain the expanded options
+       from OPTION_DEFAULT_SPECS (if any).  */
+    FOR_EACH_VEC_ELT (configure_time_options, i, opt)
+      {
+	gcc_assert (opt);
+	gcc_assert (opt[0] == '-');
+	ADD_ARG (opt);
+      }
+  }
+
 #undef ADD_ARG
 #undef ADD_ARG_TAKE_OWNERSHIP
 }
diff --git a/gcc/jit/jit-spec.c b/gcc/jit/jit-spec.c
new file mode 100644
index 0000000..fa7e9d4
--- /dev/null
+++ b/gcc/jit/jit-spec.c
@@ -0,0 +1,44 @@
+/* Dummy flag and argument handling of the jit "front-end".
+   Copyright (C) 2015 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tm.h"
+#include "gcc.h"
+#include "opts.h"
+
+/* Filter command line before processing by the gcc driver proper.  */
+void
+lang_specific_driver (struct cl_decoded_option **in_decoded_options ATTRIBUTE_UNUSED,
+		      unsigned int *in_decoded_options_count ATTRIBUTE_UNUSED,
+		      int *in_added_libraries ATTRIBUTE_UNUSED)
+{
+  /* Not used for jit.  */
+}
+
+/* Called before linking.  Returns 0 on success and -1 on failure.  */
+int
+lang_specific_pre_link (void)
+{
+  return 0;  /* Not used for jit.  */
+}
+
+/* Number of extra output files that lang_specific_pre_link may generate.  */
+int lang_specific_extra_outfiles = 0;  /* Not used for jit.  */
-- 
1.8.5.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]