Move acquisition/release of the JIT mutex from
  jit-recording.c:gcc::jit::recording::context::compile
into
  jit-playback.c: gcc::jit::playback::context::compile
and reduce the amount of code guarded by the mutex somewhat.

For acquisition, we didn't need the mutex when building the tempdir and
building toplev's argv, so now acquire the mutex immediately before using
toplev.

For release, we didn't need the mutex when cleaning up the tempdir and
the other cleanup of the playback context, so release the mutex on each exit
path from playback::context::compile.

Ideally we'd release it immediately after toplev.finalize (), but there
are various places after that point that use timevars (which is global
state) so we have to hold the mutex until we're done with them.

Committed to trunk as r218528.

gcc/jit/ChangeLog:
        * jit-playback.c (gcc::jit::playback::context::compile): Acquire the
        mutex here, immediately before using toplev, and release it here, on
        each exit path after acquisition.
        (jit_mutex): Move this variable here, from jit-recording.c.
        (gcc::jit::playback::context::acquire_mutex): New function, based on
        code in jit-recording.c.
        (gcc::jit::playback::context::release_mutex): Likewise.
        * jit-playback.h (gcc::jit::playback::context::acquire_mutex): New
        function.
        (gcc::jit::playback::context::release_mutex): New function.
        * jit-recording.c (jit_mutex): Move this variable to jit-playback.c.
        (gcc::jit::recording::context::compile): Move mutex-handling from
        here into jit-playback.c's gcc::jit::playback::context::compile.
        * notes.txt: Update to show the new locations of ACQUIRE_MUTEX
        and RELEASE_MUTEX.

Index: gcc/jit/jit-recording.c
===================================================================
--- gcc/jit/jit-recording.c	(revision 218527)
+++ gcc/jit/jit-recording.c	(revision 218528)
@@ -888,12 +888,6 @@
   m_requested_dumps.safe_push (d);
 }
 
-
-/* This mutex guards gcc::jit::recording::context::compile, so that only
-   one thread can be accessing the bulk of GCC's state at once.  */
-
-static pthread_mutex_t jit_mutex = PTHREAD_MUTEX_INITIALIZER;
-
 /* Validate this context, and if it passes, compile it within a
    mutex.
 
@@ -908,20 +902,12 @@
   if (errors_occurred ())
     return NULL;
 
-  /* Acquire the big GCC mutex. */
-  pthread_mutex_lock (&jit_mutex);
-  gcc_assert (NULL == ::gcc::jit::active_playback_ctxt);
-
   /* Set up a playback context.  */
   ::gcc::jit::playback::context replayer (this);
-  ::gcc::jit::active_playback_ctxt = &replayer;
 
+  /* Use it.  */
   result *result_obj = replayer.compile ();
 
-  /* Release the big GCC mutex. */
-  ::gcc::jit::active_playback_ctxt = NULL;
-  pthread_mutex_unlock (&jit_mutex);
-
   return result_obj;
 }
 
Index: gcc/jit/ChangeLog
===================================================================
--- gcc/jit/ChangeLog	(revision 218527)
+++ gcc/jit/ChangeLog	(revision 218528)
@@ -1,5 +1,23 @@
 2014-12-09  David Malcolm  <dmalc...@redhat.com>
 
+	* jit-playback.c (gcc::jit::playback::context::compile): Acquire the
+	mutex here, immediately before using toplev, and release it here, on
+	each exit path after acquisition.
+	(jit_mutex): Move this variable here, from jit-recording.c.
+	(gcc::jit::playback::context::acquire_mutex): New function, based on
+	code in jit-recording.c.
+	(gcc::jit::playback::context::release_mutex): Likewise.
+	* jit-playback.h (gcc::jit::playback::context::acquire_mutex): New
+	function.
+	(gcc::jit::playback::context::release_mutex): New function.
+	* jit-recording.c (jit_mutex): Move this variable to jit-playback.c.
+	(gcc::jit::recording::context::compile): Move mutex-handling from
+	here into jit-playback.c's gcc::jit::playback::context::compile.
+	* notes.txt: Update to show the new locations of ACQUIRE_MUTEX
+	and RELEASE_MUTEX.
+
+2014-12-09  David Malcolm  <dmalc...@redhat.com>
+
 	* jit-playback.c (gcc::jit::playback::context::compile): Move the
 	dlopen code into...
 	(gcc::jit::playback::context::dlopen_built_dso): ...this new
Index: gcc/jit/jit-playback.c
===================================================================
--- gcc/jit/jit-playback.c	(revision 218527)
+++ gcc/jit/jit-playback.c	(revision 218528)
@@ -1622,6 +1622,9 @@
   if (errors_occurred ())
     return NULL;
 
+  /* Acquire the JIT mutex and set "this" as the active playback ctxt.  */
+  acquire_mutex ();
+
   /* This runs the compiler.  */
   toplev toplev (false);
   toplev.main (fake_args.length (),
@@ -1635,10 +1638,14 @@
   /* Clean up the compiler.  */
   toplev.finalize ();
 
-  active_playback_ctxt = NULL;
+  /* Ideally we would release the jit mutex here, but we can't yet since
+     followup activities use timevars, which are global state.  */
 
   if (errors_occurred ())
-    return NULL;
+    {
+      release_mutex ();
+      return NULL;
+    }
 
   if (get_bool_option (GCC_JIT_BOOL_OPTION_DUMP_GENERATED_CODE))
     dump_generated_code ();
@@ -1645,15 +1652,47 @@
 
   convert_to_dso (ctxt_progname);
   if (errors_occurred ())
-    return NULL;
+    {
+      release_mutex ();
+      return NULL;
+    }
 
   result_obj = dlopen_built_dso ();
 
+  release_mutex ();
+
   return result_obj;
 }
 
 /* Helper functions for gcc::jit::playback::context::compile.  */
 
+/* This mutex guards gcc::jit::recording::context::compile, so that only
+   one thread can be accessing the bulk of GCC's state at once.  */
+
+static pthread_mutex_t jit_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/* Acquire jit_mutex and set "this" as the active playback ctxt.  */
+
+void
+playback::context::acquire_mutex ()
+{
+  /* Acquire the big GCC mutex. */
+  pthread_mutex_lock (&jit_mutex);
+  gcc_assert (NULL == active_playback_ctxt);
+  active_playback_ctxt = this;
+}
+
+/* Release jit_mutex and clear the active playback ctxt.  */
+
+void
+playback::context::release_mutex ()
+{
+  /* Release the big GCC mutex. */
+  gcc_assert (active_playback_ctxt == this);
+  active_playback_ctxt = NULL;
+  pthread_mutex_unlock (&jit_mutex);
+}
+
 /* Build a fake argv for toplev::main from the options set
    by the user on the context .  */
 
Index: gcc/jit/jit-playback.h
===================================================================
--- gcc/jit/jit-playback.h	(revision 218527)
+++ gcc/jit/jit-playback.h	(revision 218528)
@@ -235,6 +235,9 @@
 
   /* Functions for implementing "compile".  */
 
+  void acquire_mutex ();
+  void release_mutex ();
+
   void
   make_fake_args (vec <char *> *argvec,
 		  const char *ctxt_progname,
Index: gcc/jit/notes.txt
===================================================================
--- gcc/jit/notes.txt	(revision 218527)
+++ gcc/jit/notes.txt	(revision 218528)
@@ -20,11 +20,11 @@
     ──────────────────────────>      .               .
               .           .    │ start of recording::context::compile ()
               .           .    │     .               .
-              .           .    │ ACQUIRE MUTEX       .
-              .           .    │     .               .
               .           .    │ start of playback::context::compile ()
               .           .    │   (create tempdir)  .
               .           .    │     .               .
+              .           .    │ ACQUIRE MUTEX       .
+              .           .    │     .               .
               .           .    V───────────────────────> toplev::main (for now)
               .           .          .               .       │
               .           .          .               .   (various code)
@@ -78,6 +78,8 @@
               .           .    │     .               .
               .           .    │ Load DSO (dlopen "fake.so")
               .           .    │     .               .
+              .           .    │ RELEASE MUTEX       .
+              .           .    │     .               .
               .           .    │ end of playback::context::compile ()
               .           .    │     .               .
               .           .    │ playback::context dtor
@@ -87,8 +89,6 @@
               .           .       │    filesystem at this point)
               .           .    <──   .               .
               .           .    │     .               .
-              .           .    │ RELEASE MUTEX       .
-              .           .    │     .               .
               .           .    │ end of recording::context::compile ()
    <───────────────────────────      .               .
    │          .           .          .               .

Reply via email to