public inbox for jit@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, committed] Guard less code with the JIT mutex
@ 2014-01-01  0:00 David Malcolm
  0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2014-01-01  0:00 UTC (permalink / raw)
  To: gcc-patches, jit

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

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.


[-- Attachment #2: r218528.patch --]
[-- Type: text/x-patch, Size: 7351 bytes --]

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  <dmalcolm@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  <dmalcolm@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 ()
    <───────────────────────────      .               .
    │          .           .          .               .

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2014-12-09 18:54 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-01  0:00 [PATCH, committed] Guard less code with the JIT mutex David Malcolm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).