Re: [PATCH][jit] Add thread-local globals to the libgccjit frontend

Re: [PATCH][jit] Add thread-local globals to the libgccjit frontend

[Marc Nieper-Wißkirchen]

Dear David,

thank you very much for your timely response and for talking a thorough 
look at my proposed patch.

Am 07.01.19 um 21:34 schrieb David Malcolm:

> Have you done the legal paperwork with the FSF for contributing to GCC?
>   See https://gcc.gnu.org/contribute.html#legal

Not yet; this is my first patch I would like to contribute to GCC. You 
should have received a private email to get the legal matters done.

>> ChangeLog
>>
>> 2019-01-05  Marc Nieper-Wißkirchen  <marc@nieper-wisskirchen.de>
>>
>>          * docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11.
>>          * docs/topics/expressions.rst (Global variables): Add
>>          documentation of gcc_jit_lvalue_set_bool_thread_local.
>> * docs/_build/texinfo/libgccjit.texi: Regenerate.
>> * jit-playback.c: Include "varasm.h".
>> Within namespace gcc::jit::playback...
>> (context::new_global) Add "thread_local_p" param and use it
>> to set DECL_TLS_MODEL.
>> * jit-playback.h: Within namespace gcc::jit::playback...
>> (context::new_global: Add "thread_local_p" param.
>> * jit-recording.c: Within namespace gcc::jit::recording...
>> (global::replay_into): Provide m_thread_local to call to
>> new_global.
>> (global::write_reproducer): Call write_reproducer_thread_local.
>> (global::write_reproducer_thread_local): New method.
>> * jit-recording.h: Within namespace gcc::jit::recording...
>> (lvalue::dyn_cast_global): New virtual function.
>> (global::m_thread_local): New field.
>> * libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New
>> function.
>> * libgccjit.h
>> (LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New
>> macro.
>> (gcc_jit_lvalue_set_bool_thread_local): New function.
>> * libgccjit.map (LIBGCCJIT_ABI_11): New.
>> (gcc_jit_lvalue_set_bool_thread_local): Add.
>>
>> Testing
>>
>> The result of `make check-jit' is (the failing test in
>> `test-sum-squares.c` is also failing without this patch on my
>> machine):
>>
>> Native configuration is x86_64-pc-linux-gnu
> [...]
>
>> FAIL:  test-combination.c.exe iteration 1 of 5:
>> verify_code_sum_of_squares: dump_vrp1: actual: "
>> FAIL: test-combination.c.exe killed: 20233 exp6 0 0 CHILDKILLED
>> SIGABRT SIGABRT
>> FAIL:  test-sum-of-squares.c.exe iteration 1 of 5: verify_code:
>> dump_vrp1: actual: "
>> FAIL: test-sum-of-squares.c.exe killed: 22698 exp6 0 0 CHILDKILLED
>> SIGABRT SIGABRT
>> FAIL:  test-threads.c.exe: verify_code_sum_of_squares: dump_vrp1:
>> actual: "
>> FAIL: test-threads.c.exe killed: 22840 exp6 0 0 CHILDKILLED SIGABRT
>> SIGABRT
> That one's failing for me as well.  I'm investigating (I've filed it as
> PR jit/88747).

I have applied your recent patch. With the patch, there are no more 
failures.

Including the new test case for thread-local globals (see below), the 
final output of `make check-jit' is now as follows:

Test run by mnieper on Tue Jan  8 14:18:27 2019

Native configuration is x86_64-pc-linux-gnu

         === jit tests ===

Schedule of variations:

     unix

Running target unix

Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.

Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.

Using /home/mnieper/gcc/src/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.

Running /home/mnieper/gcc/src/gcc/testsuite/jit.dg/jit.exp ...

         === jit Summary ===

# of expected passes        10394

> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
>> b/gcc/testsuite/jit.dg/all-non-failing-tests.h
>> index bf02e1258..c2654ff09 100644
>> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
>> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
>> @@ -224,6 +224,13 @@
>>   #undef create_code
>>   #undef verify_code
>>
>> +/* test-factorial-must-tail-call.c */
> Looks like a cut&paste error: presumably the above comment is meant to
> refer to the new test...

Yep.

>> +#define create_code create_code_thread_local
>> +#define verify_code verify_code_thread_local
>> +#include "test-thread-local.c"
> ...but it looks like the new test file is missing from the patch.

My fault. I supplied the wrong arguments to `git diff'.

At the end of this email, you'll find the updated ChangeLog and the 
amended patch.

Thanks again,

Marc

2019-01-08  Marc Nieper-Wißkirchen  <marc@nieper-wisskirchen.de>

         * docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11.
         * docs/topics/expressions.rst (Global variables): Add
         documentation of gcc_jit_lvalue_set_bool_thread_local.
	* docs/_build/texinfo/libgccjit.texi: Regenerate.
	* jit-playback.c: Include "varasm.h".
	Within namespace gcc::jit::playback...
	(context::new_global) Add "thread_local_p" param and use it
	to set DECL_TLS_MODEL.
	* jit-playback.h: Within namespace gcc::jit::playback...
	(context::new_global): Add "thread_local_p" param.
	* jit-recording.c: Within namespace gcc::jit::recording...
	(global::replay_into): Provide m_thread_local to call to
	new_global.
	(global::write_reproducer): Call write_reproducer_thread_local.
	(global::write_reproducer_thread_local): New method.
	* jit-recording.h: Within namespace gcc::jit::recording...
	(lvalue::dyn_cast_global): New virtual function.
	(global::m_thread_local): New field.
	* libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New
	function.
	* libgccjit.h
	(LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New
	macro.
	(gcc_jit_lvalue_set_bool_thread_local): New function.
	* libgccjit.map (LIBGCCJIT_ABI_11): New.
	(gcc_jit_lvalue_set_bool_thread_local): Add.
	* ../testsuite/jit.dg/all-non-failing-tests.h: Include new test.
	* ../testsuite/jit.dg/jit.exp: Load pthread for tests involving
	thread-local globals.
	* ../testsuite/jit.dg/test-thread-local.c: New test case for
	thread-local globals.

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 38d338b1f..10926537d 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -171,3 +171,9 @@ entrypoints:
  
  ``LIBGCCJIT_ABI_10`` covers the addition of
  :func:`gcc_jit_context_new_rvalue_from_vector`
+
+``LIBGCCJIT_ABI_11``
+--------------------
+
+``LIBGCCJIT_ABI_11`` covers the addition of
+:func:`gcc_jit_lvalue_set_bool_thread_local`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 9dee2d87a..984d02cc8 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -576,6 +576,21 @@ Global variables
        referring to it.  Analogous to using an "extern" global from a
        header file.
  
+.. function:: void\
+              gcc_jit_lvalue_set_bool_thread_local (gcc_jit_lvalue *global,\
+                                                    int thread_local_p)
+
+   Given a :c:type:`gcc_jit_lvalue *` for a global created through
+   :c:func:`gcc_jit_context_new_global`, mark/clear the global as being
+   thread-local. Analogous to a global with thread storage duration in C11.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_11`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local
+
  Working with pointers, structs and unions
  -----------------------------------------
  
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 86f588db9..6c00a98c0 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
  #include "opt-suggestions.h"
  #include "gcc.h"
  #include "diagnostic.h"
+#include "varasm.h"
  
  #include <pthread.h>
  
@@ -461,7 +462,8 @@ playback::context::
  new_global (location *loc,
  	    enum gcc_jit_global_kind kind,
  	    type *type,
-	    const char *name)
+	    const char *name,
+	    bool thread_local_p)
  {
    gcc_assert (type);
    gcc_assert (name);
@@ -487,6 +489,8 @@ new_global (location *loc,
        DECL_EXTERNAL (inner) = 1;
        break;
      }
+  if (thread_local_p)
+    set_decl_tls_model (inner, decl_default_tls_model (inner));
  
    if (loc)
      set_tree_location (inner, loc);
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index bc4de9c03..fc0843f58 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -103,7 +103,8 @@ public:
    new_global (location *loc,
  	      enum gcc_jit_global_kind kind,
  	      type *type,
-	      const char *name);
+	      const char *name,
+	      bool thread_local_p);
  
    template <typename HOST_TYPE>
    rvalue *
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 04cc6a690..75557af19 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -4280,7 +4280,8 @@ recording::global::replay_into (replayer *r)
    set_playback_obj (r->new_global (playback_location (r, m_loc),
  				   m_kind,
  				   m_type->playback_type (),
-				   playback_string (m_name)));
+				   playback_string (m_name),
+				   m_thread_local));
  }
  
  /* Override the default implementation of
@@ -4356,6 +4357,22 @@ recording::global::write_reproducer (reproducer &r)
      global_kind_reproducer_strings[m_kind],
      r.get_identifier_as_type (get_type ()),
      m_name->get_debug_string ());
+  write_reproducer_thread_local (r, id);
+}
+
+/* Subroutine for use by global's write_reproducer methods.  */
+
+void
+recording::global::write_reproducer_thread_local (reproducer &r,
+						  const char *id)
+{
+  if (m_thread_local)
+    {
+      r.write ("  gcc_jit_lvalue_set_bool_thread_local (%s,  /* gcc_jit_lvalue *global*/\n"
+	       "                                        %i); /* int thread_local_p*/\n",
+	       id,
+	       1);
+    }
  }
  
  /* The implementation of the various const-handling classes:
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index b9c6544d0..6a3d32dd6 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1054,6 +1054,8 @@ public:
      return static_cast <playback::lvalue *> (m_playback_obj);
    }
  
+  virtual global *dyn_cast_global () { return NULL; }
+
    lvalue *
    access_field (location *loc,
  		field *field);
@@ -1285,7 +1287,8 @@ public:
  	  string *name)
    : lvalue (ctxt, loc, type),
      m_kind (kind),
-    m_name (name)
+    m_name (name),
+    m_thread_local (false)
    {}
  
    void replay_into (replayer *) FINAL OVERRIDE;
@@ -1294,7 +1297,17 @@ public:
  
    void write_to_dump (dump &d) FINAL OVERRIDE;
  
-private:
+  global *dyn_cast_global () FINAL OVERRIDE { return this; }
+
+  void set_thread_local (bool thread_local_p)
+  {
+    m_thread_local = thread_local_p;
+  }
+
+ protected:
+  void write_reproducer_thread_local (reproducer &r, const char *id);
+
+ private:
    string * make_debug_string () FINAL OVERRIDE { return m_name; }
    void write_reproducer (reproducer &r) FINAL OVERRIDE;
    enum precedence get_precedence () const FINAL OVERRIDE
@@ -1305,6 +1318,7 @@ private:
  private:
    enum gcc_jit_global_kind m_kind;
    string *m_name;
+  bool m_thread_local;
  };
  
  template <typename HOST_TYPE>
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index de7fb2579..2b643c63f 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -3102,3 +3102,23 @@ gcc_jit_context_new_rvalue_from_vector (gcc_jit_context *ctxt,
       as_vec_type,
       (gcc::jit::recording::rvalue **)elements);
  }
+
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is effectively done by the
+   gcc::jit::global::set_thread_local setter in jit-recording.h.  */
+
+void
+gcc_jit_lvalue_set_bool_thread_local (gcc_jit_lvalue *lvalue,
+				      int thread_local_p)
+{
+  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL global");
+  JIT_LOG_FUNC (lvalue->get_context ()->get_logger ());
+
+  /* Verify that it's a global.  */
+  gcc::jit::recording::global *global = lvalue->dyn_cast_global ();
+  RETURN_IF_FAIL_PRINTF1 (global, NULL, NULL, "not a global: %s",
+			  lvalue->get_debug_string ());
+
+  global->set_thread_local (thread_local_p);
+}
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index e872ae789..d64d05a8d 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1450,6 +1450,18 @@ gcc_jit_context_new_rvalue_from_vector (gcc_jit_context *ctxt,
  					size_t num_elements,
  					gcc_jit_rvalue **elements);
  
+#define LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local
+
+/* Mark/clear a global as thread-local.
+
+   This API entrypoint was added in LIBGCCJIT_ABI_11; you can test for its
+   presence using
+     #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local
+*/
+extern void
+gcc_jit_lvalue_set_bool_thread_local (gcc_jit_lvalue *global,
+				      int thread_local_p);
+
  #ifdef __cplusplus
  }
  #endif /* __cplusplus */
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 2826f1ca6..741320bbe 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -170,3 +170,8 @@ LIBGCCJIT_ABI_10 {
    global:
      gcc_jit_context_new_rvalue_from_vector;
  } LIBGCCJIT_ABI_9;
+
+LIBGCCJIT_ABI_11 {
+  global:
+    gcc_jit_lvalue_set_bool_thread_local;
+} LIBGCCJIT_ABI_10;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index bf02e1258..c91687182 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -224,6 +224,13 @@
  #undef create_code
  #undef verify_code
  
+/* test-thread-local.c */
+#define create_code create_code_thread_local
+#define verify_code verify_code_thread_local
+#include "test-thread-local.c"
+#undef create_code
+#undef verify_code
+
  /* test-types.c */
  #define create_code create_code_types
  #define verify_code verify_code_types
@@ -353,6 +360,9 @@ const struct testcase testcases[] = {
    {"switch",
     create_code_switch,
     verify_code_switch},
+  {"thread_local",
+   create_code_thread_local,
+   verify_code_thread_local},
    {"types",
     create_code_types,
     verify_code_types},
diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
index 869d9f693..3be4f2c18 100644
--- a/gcc/testsuite/jit.dg/jit.exp
+++ b/gcc/testsuite/jit.dg/jit.exp
@@ -379,6 +379,16 @@ proc jit-dg-test { prog do_what extra_tool_flags } {
  	append extra_tool_flags " -lpthread"
      }
  
+    # test-thread-local.c needs to be linked against pthreads
+    if {[string match "*test-thread-local.c" $prog]} {
+	append extra_tool_flags " -lpthread"
+    }
+
+    # test-combination.c needs to be linked against pthreads
+    if {[string match "*test-combination.c" $prog]} {
+	append extra_tool_flags " -lpthread"
+    }
+
      # Any test case that uses jit-verify-output-file-was-created
      # needs to call jit-setup-compile-to-file here.
      # (is there a better way to handle setup/finish pairs in dg?)
diff --git a/gcc/testsuite/jit.dg/test-thread-local.c b/gcc/testsuite/jit.dg/test-thread-local.c
new file mode 100644
index 000000000..287ba85e4
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-thread-local.c
@@ -0,0 +1,99 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <pthread.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Let's try to inject the equivalent of:
+
+      static thread_local int tl;
+      set_tl (int v)
+      {
+        tl = v;
+      }
+
+      int
+      get_tl (void)
+      {
+        return tl;
+      }
+
+   */
+  gcc_jit_type *the_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+  gcc_jit_type *void_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+
+  gcc_jit_lvalue *tl =
+    gcc_jit_context_new_global (ctxt, NULL, GCC_JIT_GLOBAL_INTERNAL,
+				the_type, "tl");
+  gcc_jit_lvalue_set_bool_thread_local (tl, 1);
+
+  gcc_jit_param *v =
+    gcc_jit_context_new_param (ctxt, NULL, the_type, "v");
+  gcc_jit_param *params[1] = {v};
+  gcc_jit_function *func =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  void_type,
+				  "set_tl",
+				  1, params, 0);
+  gcc_jit_block *block =
+    gcc_jit_function_new_block (func, "initial");
+  gcc_jit_block_add_assignment (block, NULL, tl,
+				gcc_jit_param_as_rvalue (v));
+  gcc_jit_block_end_with_void_return (block, NULL);
+
+  func =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  the_type,
+				  "get_tl",
+				  0, NULL, 0);
+  block =
+    gcc_jit_function_new_block (func, "initial");
+  gcc_jit_block_end_with_return (block, NULL,
+				 gcc_jit_lvalue_as_rvalue (tl));
+}
+
+static void *
+test_thread_local_run (void *arg)
+{
+  void (*set_tl) (int) = arg;
+  set_tl (43);
+  return NULL;
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  typedef void (*set_tl_fn_type) (int);
+  typedef int (*get_tl_fn_type) (void);
+
+  CHECK_NON_NULL (result);
+
+  set_tl_fn_type set_tl =
+    (set_tl_fn_type) gcc_jit_result_get_code (result, "set_tl");
+  get_tl_fn_type get_tl =
+    (get_tl_fn_type) gcc_jit_result_get_code (result, "get_tl");
+
+  CHECK_NON_NULL (set_tl);
+  CHECK_NON_NULL (get_tl);
+
+  set_tl (42);
+
+  pthread_t thread;
+  CHECK_VALUE (pthread_create(&thread, NULL, test_thread_local_run, set_tl), 0);
+  CHECK_VALUE (pthread_join(thread, NULL), 0);
+
+  int val = get_tl ();
+
+  note ("get_tl returned: %d", val);
+  CHECK_VALUE (val, 42);
+}

[committed] Fix jit test case (PR jit/88747)

[David Malcolm]

Amongst other changes, r266077 updated value_range_base::dump so
that it additionally prints the type.  This broke an assertion within
the jit testsuite, in jit.dg/test-sum-of-squares.c, which was checking
for:
  ": [-INF, n_"
but was now getting:
  ": signed int [-INF, n_"

The test is merely intended as a simple verification that we can read
dump files via gcc_jit_context_enable_dump.

This patch loosens the requirements on the dump so that it should work
with either version of value_range_base::dump.

Verified on x86_64-pc-linux-gnu, removing 6 FAIL results from jit.sum
and taking it from 6479 to 10288 PASS results.

Committed to trunk as r267671.

gcc/testsuite/ChangeLog:
	PR jit/88747
	* jit.dg/test-sum-of-squares.c (verify_code): Update expected vrp
	dump to reflect r266077.
---
 gcc/testsuite/jit.dg/test-sum-of-squares.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/jit.dg/test-sum-of-squares.c b/gcc/testsuite/jit.dg/test-sum-of-squares.c
index 46fd5c2..f095f41 100644
--- a/gcc/testsuite/jit.dg/test-sum-of-squares.c
+++ b/gcc/testsuite/jit.dg/test-sum-of-squares.c
@@ -137,6 +137,6 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
      bounds of the iteration variable. Specifically, verify that some
      variable is known to be in the range negative infinity to some
      expression based on param "n" (actually n-1).  */
-  CHECK_STRING_CONTAINS (dump_vrp1, ": [-INF, n_");
+  CHECK_STRING_CONTAINS (dump_vrp1, "[-INF, n_");
   free (dump_vrp1);
 }
-- 
1.8.5.3

[PATCH][jit] Add thread-local globals to the libgccjit frontend

[Marc Nieper-Wißkirchen]

This patch adds thread-local globals to the libgccjit frontend. The
library user can mark a global as being thread-local by calling
`gcc_jit_lvalue_set_bool_thread_local'. It is implemented by calling
`set_decl_tls_model (inner, decl_default_tls_model (inner))', where
`inner' is the GENERIC tree corresponding to the global.

ChangeLog

2019-01-05  Marc Nieper-Wißkirchen  <marc@nieper-wisskirchen.de>

        * docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11.
        * docs/topics/expressions.rst (Global variables): Add
        documentation of gcc_jit_lvalue_set_bool_thread_local.
* docs/_build/texinfo/libgccjit.texi: Regenerate.
* jit-playback.c: Include "varasm.h".
Within namespace gcc::jit::playback...
(context::new_global) Add "thread_local_p" param and use it
to set DECL_TLS_MODEL.
* jit-playback.h: Within namespace gcc::jit::playback...
(context::new_global: Add "thread_local_p" param.
* jit-recording.c: Within namespace gcc::jit::recording...
(global::replay_into): Provide m_thread_local to call to
new_global.
(global::write_reproducer): Call write_reproducer_thread_local.
(global::write_reproducer_thread_local): New method.
* jit-recording.h: Within namespace gcc::jit::recording...
(lvalue::dyn_cast_global): New virtual function.
(global::m_thread_local): New field.
* libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New
function.
* libgccjit.h
(LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New
macro.
(gcc_jit_lvalue_set_bool_thread_local): New function.
* libgccjit.map (LIBGCCJIT_ABI_11): New.
(gcc_jit_lvalue_set_bool_thread_local): Add.

Testing

The result of `make check-jit' is (the failing test in
`test-sum-squares.c` is also failing without this patch on my
machine):

Native configuration is x86_64-pc-linux-gnu

=== jit tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file
for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/mnieper/gcc/src/gcc/testsuite/config/default.exp as
tool-and-target-specific interface file.
Running /home/mnieper/gcc/src/gcc/testsuite/jit.dg/jit.exp ...
FAIL:  test-combination.c.exe iteration 1 of 5:
verify_code_sum_of_squares: dump_vrp1: actual: "
FAIL: test-combination.c.exe killed: 20233 exp6 0 0 CHILDKILLED SIGABRT SIGABRT
FAIL:  test-sum-of-squares.c.exe iteration 1 of 5: verify_code:
dump_vrp1: actual: "
FAIL: test-sum-of-squares.c.exe killed: 22698 exp6 0 0 CHILDKILLED
SIGABRT SIGABRT
FAIL:  test-threads.c.exe: verify_code_sum_of_squares: dump_vrp1: actual: "
FAIL: test-threads.c.exe killed: 22840 exp6 0 0 CHILDKILLED SIGABRT SIGABRT

=== jit Summary ===

# of expected passes 6506
# of unexpected failures 6

Patch

diff --git a/gcc/jit/docs/topics/compatibility.rst
b/gcc/jit/docs/topics/compatibility.rst
index 38d338b1f..10926537d 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -171,3 +171,9 @@ entrypoints:

 ``LIBGCCJIT_ABI_10`` covers the addition of
 :func:`gcc_jit_context_new_rvalue_from_vector`
+
+``LIBGCCJIT_ABI_11``
+--------------------
+
+``LIBGCCJIT_ABI_11`` covers the addition of
+:func:`gcc_jit_lvalue_set_bool_thread_local`
diff --git a/gcc/jit/docs/topics/expressions.rst
b/gcc/jit/docs/topics/expressions.rst
index 9dee2d87a..984d02cc8 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -576,6 +576,21 @@ Global variables
       referring to it.  Analogous to using an "extern" global from a
       header file.

+.. function:: void\
+              gcc_jit_lvalue_set_bool_thread_local (gcc_jit_lvalue *global,\
+                                                    int thread_local_p)
+
+   Given a :c:type:`gcc_jit_lvalue *` for a global created through
+   :c:func:`gcc_jit_context_new_global`, mark/clear the global as being
+   thread-local. Analogous to a global with thread storage duration in C11.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_11`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local
+
 Working with pointers, structs and unions
 -----------------------------------------

diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 86f588db9..6c00a98c0 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "opt-suggestions.h"
 #include "gcc.h"
 #include "diagnostic.h"
+#include "varasm.h"

 #include <pthread.h>

@@ -461,7 +462,8 @@ playback::context::
 new_global (location *loc,
      enum gcc_jit_global_kind kind,
      type *type,
-     const char *name)
+     const char *name,
+     bool thread_local_p)
 {
   gcc_assert (type);
   gcc_assert (name);
@@ -487,6 +489,8 @@ new_global (location *loc,
       DECL_EXTERNAL (inner) = 1;
       break;
     }
+  if (thread_local_p)
+    set_decl_tls_model (inner, decl_default_tls_model (inner));

   if (loc)
     set_tree_location (inner, loc);
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index bc4de9c03..fc0843f58 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -103,7 +103,8 @@ public:
   new_global (location *loc,
        enum gcc_jit_global_kind kind,
        type *type,
-       const char *name);
+       const char *name,
+       bool thread_local_p);

   template <typename HOST_TYPE>
   rvalue *
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 04cc6a690..75557af19 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -4280,7 +4280,8 @@ recording::global::replay_into (replayer *r)
   set_playback_obj (r->new_global (playback_location (r, m_loc),
     m_kind,
     m_type->playback_type (),
-    playback_string (m_name)));
+    playback_string (m_name),
+    m_thread_local));
 }

 /* Override the default implementation of
@@ -4356,6 +4357,22 @@ recording::global::write_reproducer (reproducer &r)
     global_kind_reproducer_strings[m_kind],
     r.get_identifier_as_type (get_type ()),
     m_name->get_debug_string ());
+  write_reproducer_thread_local (r, id);
+}
+
+/* Subroutine for use by global's write_reproducer methods.  */
+
+void
+recording::global::write_reproducer_thread_local (reproducer &r,
+   const char *id)
+{
+  if (m_thread_local)
+    {
+      r.write ("  gcc_jit_lvalue_set_bool_thread_local (%s,  /*
gcc_jit_lvalue *global*/\n"
+        "                                        %i); /* int
thread_local_p*/\n",
+        id,
+        1);
+    }
 }

 /* The implementation of the various const-handling classes:
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index b9c6544d0..6a3d32dd6 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1054,6 +1054,8 @@ public:
     return static_cast <playback::lvalue *> (m_playback_obj);
   }

+  virtual global *dyn_cast_global () { return NULL; }
+
   lvalue *
   access_field (location *loc,
  field *field);
@@ -1285,7 +1287,8 @@ public:
    string *name)
   : lvalue (ctxt, loc, type),
     m_kind (kind),
-    m_name (name)
+    m_name (name),
+    m_thread_local (false)
   {}

   void replay_into (replayer *) FINAL OVERRIDE;
@@ -1294,7 +1297,17 @@ public:

   void write_to_dump (dump &d) FINAL OVERRIDE;

-private:
+  global *dyn_cast_global () FINAL OVERRIDE { return this; }
+
+  void set_thread_local (bool thread_local_p)
+  {
+    m_thread_local = thread_local_p;
+  }
+
+ protected:
+  void write_reproducer_thread_local (reproducer &r, const char *id);
+
+ private:
   string * make_debug_string () FINAL OVERRIDE { return m_name; }
   void write_reproducer (reproducer &r) FINAL OVERRIDE;
   enum precedence get_precedence () const FINAL OVERRIDE
@@ -1305,6 +1318,7 @@ private:
 private:
   enum gcc_jit_global_kind m_kind;
   string *m_name;
+  bool m_thread_local;
 };

 template <typename HOST_TYPE>
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index de7fb2579..2b643c63f 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -3102,3 +3102,23 @@ gcc_jit_context_new_rvalue_from_vector
(gcc_jit_context *ctxt,
      as_vec_type,
      (gcc::jit::recording::rvalue **)elements);
 }
+
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is effectively done by the
+   gcc::jit::global::set_thread_local setter in jit-recording.h.  */
+
+void
+gcc_jit_lvalue_set_bool_thread_local (gcc_jit_lvalue *lvalue,
+       int thread_local_p)
+{
+  RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL global");
+  JIT_LOG_FUNC (lvalue->get_context ()->get_logger ());
+
+  /* Verify that it's a global.  */
+  gcc::jit::recording::global *global = lvalue->dyn_cast_global ();
+  RETURN_IF_FAIL_PRINTF1 (global, NULL, NULL, "not a global: %s",
+   lvalue->get_debug_string ());
+
+  global->set_thread_local (thread_local_p);
+}
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index e872ae789..d64d05a8d 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1450,6 +1450,18 @@ gcc_jit_context_new_rvalue_from_vector
(gcc_jit_context *ctxt,
  size_t num_elements,
  gcc_jit_rvalue **elements);

+#define LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local
+
+/* Mark/clear a global as thread-local.
+
+   This API entrypoint was added in LIBGCCJIT_ABI_11; you can test for its
+   presence using
+     #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local
+*/
+extern void
+gcc_jit_lvalue_set_bool_thread_local (gcc_jit_lvalue *global,
+       int thread_local_p);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 2826f1ca6..741320bbe 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -170,3 +170,8 @@ LIBGCCJIT_ABI_10 {
   global:
     gcc_jit_context_new_rvalue_from_vector;
 } LIBGCCJIT_ABI_9;
+
+LIBGCCJIT_ABI_11 {
+  global:
+    gcc_jit_lvalue_set_bool_thread_local;
+} LIBGCCJIT_ABI_10;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index bf02e1258..c2654ff09 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -224,6 +224,13 @@
 #undef create_code
 #undef verify_code

+/* test-factorial-must-tail-call.c */
+#define create_code create_code_thread_local
+#define verify_code verify_code_thread_local
+#include "test-thread-local.c"
+#undef create_code
+#undef verify_code
+
 /* test-types.c */
 #define create_code create_code_types
 #define verify_code verify_code_types
@@ -353,6 +360,9 @@ const struct testcase testcases[] = {
   {"switch",
    create_code_switch,
    verify_code_switch},
+  {"thread_local",
+   create_code_thread_local,
+   verify_code_thread_local},
   {"types",
    create_code_types,
    verify_code_types},
diff --git a/gcc/testsuite/jit.dg/jit.exp b/gcc/testsuite/jit.dg/jit.exp
index 869d9f693..3be4f2c18 100644
--- a/gcc/testsuite/jit.dg/jit.exp
+++ b/gcc/testsuite/jit.dg/jit.exp
@@ -379,6 +379,16 @@ proc jit-dg-test { prog do_what extra_tool_flags } {
  append extra_tool_flags " -lpthread"
     }

+    # test-thread-local.c needs to be linked against pthreads
+    if {[string match "*test-thread-local.c" $prog]} {
+ append extra_tool_flags " -lpthread"
+    }
+
+    # test-combination.c needs to be linked against pthreads
+    if {[string match "*test-combination.c" $prog]} {
+ append extra_tool_flags " -lpthread"
+    }
+
     # Any test case that uses jit-verify-output-file-was-created
     # needs to call jit-setup-compile-to-file here.
     # (is there a better way to handle setup/finish pairs in dg?)

Re: [PATCH][jit] Add thread-local globals to the libgccjit frontend

[David Malcolm]

On Sat, 2019-01-05 at 23:10 +0100, Marc Nieper-Wißkirchen wrote:
> This patch adds thread-local globals to the libgccjit frontend. The
> library user can mark a global as being thread-local by calling
> `gcc_jit_lvalue_set_bool_thread_local'. It is implemented by calling
> `set_decl_tls_model (inner, decl_default_tls_model (inner))', where
> `inner' is the GENERIC tree corresponding to the global.

Thanks for creating this patch.

Have you done the legal paperwork with the FSF for contributing to GCC?
 See https://gcc.gnu.org/contribute.html#legal

Overall, this looks pretty good (though we'd want to get release
manager approval for late-breaking changes given where we are in the
release cycle).

Various comments inline below.

> ChangeLog
> 
> 2019-01-05  Marc Nieper-Wißkirchen  <marc@nieper-wisskirchen.de>
> 
>         * docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11.
>         * docs/topics/expressions.rst (Global variables): Add
>         documentation of gcc_jit_lvalue_set_bool_thread_local.
> * docs/_build/texinfo/libgccjit.texi: Regenerate.
> * jit-playback.c: Include "varasm.h".
> Within namespace gcc::jit::playback...
> (context::new_global) Add "thread_local_p" param and use it
> to set DECL_TLS_MODEL.
> * jit-playback.h: Within namespace gcc::jit::playback...
> (context::new_global: Add "thread_local_p" param.
> * jit-recording.c: Within namespace gcc::jit::recording...
> (global::replay_into): Provide m_thread_local to call to
> new_global.
> (global::write_reproducer): Call write_reproducer_thread_local.
> (global::write_reproducer_thread_local): New method.
> * jit-recording.h: Within namespace gcc::jit::recording...
> (lvalue::dyn_cast_global): New virtual function.
> (global::m_thread_local): New field.
> * libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New
> function.
> * libgccjit.h
> (LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New
> macro.
> (gcc_jit_lvalue_set_bool_thread_local): New function.
> * libgccjit.map (LIBGCCJIT_ABI_11): New.
> (gcc_jit_lvalue_set_bool_thread_local): Add.
> 
> Testing
> 
> The result of `make check-jit' is (the failing test in
> `test-sum-squares.c` is also failing without this patch on my
> machine):
> 
> Native configuration is x86_64-pc-linux-gnu

[...]

> FAIL:  test-combination.c.exe iteration 1 of 5:
> verify_code_sum_of_squares: dump_vrp1: actual: "
> FAIL: test-combination.c.exe killed: 20233 exp6 0 0 CHILDKILLED
> SIGABRT SIGABRT
> FAIL:  test-sum-of-squares.c.exe iteration 1 of 5: verify_code:
> dump_vrp1: actual: "
> FAIL: test-sum-of-squares.c.exe killed: 22698 exp6 0 0 CHILDKILLED
> SIGABRT SIGABRT
> FAIL:  test-threads.c.exe: verify_code_sum_of_squares: dump_vrp1:
> actual: "
> FAIL: test-threads.c.exe killed: 22840 exp6 0 0 CHILDKILLED SIGABRT
> SIGABRT

That one's failing for me as well.  I'm investigating (I've filed it as
PR jit/88747).

[...]

diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> index bf02e1258..c2654ff09 100644
> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> @@ -224,6 +224,13 @@
>  #undef create_code
>  #undef verify_code
> 
> +/* test-factorial-must-tail-call.c */

Looks like a cut&paste error: presumably the above comment is meant to
refer to the new test...

> +#define create_code create_code_thread_local
> +#define verify_code verify_code_thread_local
> +#include "test-thread-local.c"

...but it looks like the new test file is missing from the patch.

> +#undef create_code
> +#undef verify_code
> +
>  /* test-types.c */
>  #define create_code create_code_types
>  #define verify_code verify_code_types
> @@ -353,6 +360,9 @@ const struct testcase testcases[] = {
>    {"switch",
>     create_code_switch,
>     verify_code_switch},
> +  {"thread_local",
> +   create_code_thread_local,
> +   verify_code_thread_local},
>    {"types",
>     create_code_types,
>     verify_code_types},

[...]

Thanks
Dave

Re: [PATCH][jit] Add thread-local globals to the libgccjit frontend

[Marc Nieper-Wißkirchen]

[...]

>> 2019-01-08  Marc Nieper-Wißkirchen  <marc@nieper-wisskirchen.de>
>>
>>           * docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11.
>>           * docs/topics/expressions.rst (Global variables): Add
>>           documentation of gcc_jit_lvalue_set_bool_thread_local.
>> 	* docs/_build/texinfo/libgccjit.texi: Regenerate.
>> 	* jit-playback.c: Include "varasm.h".
>> 	Within namespace gcc::jit::playback...
>> 	(context::new_global) Add "thread_local_p" param and use it
>> 	to set DECL_TLS_MODEL.
>> 	* jit-playback.h: Within namespace gcc::jit::playback...
>> 	(context::new_global): Add "thread_local_p" param.
>> 	* jit-recording.c: Within namespace gcc::jit::recording...
>> 	(global::replay_into): Provide m_thread_local to call to
>> 	new_global.
>> 	(global::write_reproducer): Call write_reproducer_thread_local.
>> 	(global::write_reproducer_thread_local): New method.
>> 	* jit-recording.h: Within namespace gcc::jit::recording...
>> 	(lvalue::dyn_cast_global): New virtual function.
>> 	(global::m_thread_local): New field.
>> 	* libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New
>> 	function.
>> 	* libgccjit.h
>> 	(LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New
>> 	macro.
>> 	(gcc_jit_lvalue_set_bool_thread_local): New function.
>> 	* libgccjit.map (LIBGCCJIT_ABI_11): New.
>> 	(gcc_jit_lvalue_set_bool_thread_local): Add.
>> 	* ../testsuite/jit.dg/all-non-failing-tests.h: Include new
>> test.
>> 	* ../testsuite/jit.dg/jit.exp: Load pthread for tests involving
>> 	thread-local globals.
>> 	* ../testsuite/jit.dg/test-thread-local.c: New test case for
>> 	thread-local globals.
> 
> BTW, the convention here is to split out the ChangeLog entries by
> directory based on the presence of ChangeLog files.
> 
> There's a gcc/jit/ChangeLog and a gcc/testsuite/ChangeLog, so for this
> patch there should be two sets of ChangeLog entries, one for each of
> these, with the paths expressed relative to the directory holding the
> ChangeLog.
> 
> So the testsuite entries would go into gcc/testsuite/ChangeLog, and
> look like:
> 
> 	* jit.dg/all-non-failing-tests.h: Include new test.
> 
> ...etc.
> 

Thanks for explaining the policy.  Corrected ChangeLogs:

gcc/jit/ChangeLog
=================

2019-01-08  Marc Nieper-Wißkirchen  <marc@nieper-wisskirchen.de>

         * docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11.
         * docs/topics/expressions.rst (Global variables): Add
         documentation of gcc_jit_lvalue_set_bool_thread_local.
	* docs/_build/texinfo/libgccjit.texi: Regenerate.
	* jit-playback.c: Include "varasm.h".
	Within namespace gcc::jit::playback...
	(context::new_global) Add "thread_local_p" param and use it
	to set DECL_TLS_MODEL.
	* jit-playback.h: Within namespace gcc::jit::playback...
	(context::new_global): Add "thread_local_p" param.
	* jit-recording.c: Within namespace gcc::jit::recording...
	(global::replay_into): Provide m_thread_local to call to
	new_global.
	(global::write_reproducer): Call write_reproducer_thread_local.
	(global::write_reproducer_thread_local): New method.
	* jit-recording.h: Within namespace gcc::jit::recording...
	(lvalue::dyn_cast_global): New virtual function.
	(global::m_thread_local): New field.
	* libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New
	function.
	* libgccjit.h
	(LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New
	macro.
	(gcc_jit_lvalue_set_bool_thread_local): New function.
	* libgccjit.map (LIBGCCJIT_ABI_11): New.
	(gcc_jit_lvalue_set_bool_thread_local): Add.

gcc/testsuite/ChangeLog
=======================

2019-01-08  Marc Nieper-Wißkirchen  <marc@nieper-wisskirchen.de>

	* jit.dg/all-non-failing-tests.h: Include new test.
	* jit.dg/jit.exp: Load pthread for tests involving
	thread-local globals.
	* jit.dg/test-thread-local.c: New test case for
	thread-local globals.

[...]

>> diff --git a/gcc/testsuite/jit.dg/test-thread-local.c
>> b/gcc/testsuite/jit.dg/test-thread-local.c
>> new file mode 100644
>> index 000000000..287ba85e4
>> --- /dev/null
>> +++ b/gcc/testsuite/jit.dg/test-thread-local.c
>> @@ -0,0 +1,99 @@
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <pthread.h>
>> +
>> +#include "libgccjit.h"
>> +
>> +#include "harness.h"
>> +
>> +void
>> +create_code (gcc_jit_context *ctxt, void *user_data)
>> +{
>> +  /* Let's try to inject the equivalent of:
>> +
>> +      static thread_local int tl;
>> +      set_tl (int v)
>> +      {
>> +        tl = v;
>> +      }
>> +
>> +      int
>> +      get_tl (void)
>> +      {
>> +        return tl;
>> +      }
> 
> Thanks for posting this.
> 
> This test is OK as far as it goes, but there are some gaps in test
> coverage: the test verifies that jit-generated code can create a new
> thread-local variable, and writes to it, but it doesn't seem to test
> reading from it; e.g. there doesn't seem to be a CHECK_VALUE that the
> thread-local var is 43 after the set_tl (43);

The test does check reading the thread-local variable in the main 
thread.  If the variable were non thread-local, the value 42 set in the 
main thread would have been overwritten by the subordinate thread.

> 
> It would probably also be good to verify that jit-generated code can
> work with a thread-local variable declared and defined in C-generated
> code.
> 
> Thinking aloud, how about the following changes:
> - split out the thread-local vars there's e.g. a
>      extern thread_local int tl_c;
>    in the C code and the "tl" becomes "tl_jit", and access it via a
>    GCC_JIT_GLOBAL_EXTERNAL, or somesuch.
> - in the test code, run two new threads, passing them two different
> "int" values; verify that the set_tl(value); can be matched with a
> CHECK_VALUE get_tl() == the value for that thread on the two threads.

Is the following extended test-case along the lines you have been thinking?

diff --git a/gcc/testsuite/jit.dg/test-thread-local.c 
b/gcc/testsuite/jit.dg/test-thread-local.c
new file mode 100644
index 000000000..e21f144c4
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-thread-local.c
@@ -0,0 +1,161 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <pthread.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+int _Thread_local tl_c;
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Let's try to inject the equivalent of:
+
+      extern thread_local int tl_c;
+      static thread_ int tl_jit;
+      set_tl_jit (int v)
+      {
+        tl_jit = v;
+      }
+
+      int
+      get_tl_jit (void)
+      {
+        return tl_jit;
+      }
+
+      set_tl_c (int v)
+      {
+        tl_c = v;
+      }
+
+      int
+      get_tl_c (void)
+      {
+        return tl_c;
+      }
+   */
+  gcc_jit_type *the_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+  gcc_jit_type *void_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+
+  gcc_jit_lvalue *tl =
+    gcc_jit_context_new_global (ctxt, NULL, GCC_JIT_GLOBAL_INTERNAL,
+				the_type, "tl_jit");
+  gcc_jit_lvalue_set_bool_thread_local (tl, 1);
+
+  gcc_jit_param *v =
+    gcc_jit_context_new_param (ctxt, NULL, the_type, "v");
+  gcc_jit_param *params[1] = {v};
+  gcc_jit_function *func =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  void_type,
+				  "set_tl_jit",
+				  1, params, 0);
+  gcc_jit_block *block =
+    gcc_jit_function_new_block (func, "initial");
+  gcc_jit_block_add_assignment (block, NULL, tl,
+				gcc_jit_param_as_rvalue (v));
+  gcc_jit_block_end_with_void_return (block, NULL);
+
+  func =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  the_type,
+				  "get_tl_jit",
+				  0, NULL, 0);
+  block =
+    gcc_jit_function_new_block (func, "initial");
+  gcc_jit_block_end_with_return (block, NULL,
+				 gcc_jit_lvalue_as_rvalue (tl));
+
+  tl =
+    gcc_jit_context_new_global (ctxt, NULL, GCC_JIT_GLOBAL_IMPORTED,
+				the_type, "tl_c");
+  gcc_jit_lvalue_set_bool_thread_local (tl, 1);
+
+  v =
+    gcc_jit_context_new_param (ctxt, NULL, the_type, "v");
+  params[0] = v;
+  func =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  void_type,
+				  "set_tl_c",
+				  1, params, 0);
+  block =
+    gcc_jit_function_new_block (func, "initial");
+  gcc_jit_block_add_assignment (block, NULL, tl,
+				gcc_jit_param_as_rvalue (v));
+  gcc_jit_block_end_with_void_return (block, NULL);
+
+  func =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  the_type,
+				  "get_tl_c",
+				  0, NULL, 0);
+  block =
+    gcc_jit_function_new_block (func, "initial");
+  gcc_jit_block_end_with_return (block, NULL,
+				 gcc_jit_lvalue_as_rvalue (tl));
+}
+
+typedef void (*set_tl_fn_type) (int);
+typedef int (*get_tl_fn_type) (void);
+
+static set_tl_fn_type set_tl_jit, set_tl_c;
+static get_tl_fn_type get_tl_jit, get_tl_c;
+
+static void *
+test_thread_local_run (void *arg)
+{
+  set_tl_jit (43);
+  set_tl_c (101);
+  int val = get_tl_jit ();
+  CHECK_VALUE (val, 43);
+  val = get_tl_c ();
+  CHECK_VALUE (val, 101);
+  return NULL;
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+
+  CHECK_NON_NULL (result);
+
+  set_tl_jit =
+    (set_tl_fn_type) gcc_jit_result_get_code (result, "set_tl_jit");
+  get_tl_jit =
+    (get_tl_fn_type) gcc_jit_result_get_code (result, "get_tl_jit");
+  set_tl_c =
+    (set_tl_fn_type) gcc_jit_result_get_code (result, "set_tl_c");
+  get_tl_c =
+    (get_tl_fn_type) gcc_jit_result_get_code (result, "get_tl_c");
+
+  CHECK_NON_NULL (set_tl_jit);
+  CHECK_NON_NULL (get_tl_jit);
+  CHECK_NON_NULL (set_tl_c);
+  CHECK_NON_NULL (get_tl_c);
+
+  set_tl_jit (42);
+  set_tl_c (100);
+
+  pthread_t thread;
+  CHECK_VALUE (pthread_create(&thread, NULL, test_thread_local_run, 
NULL), 0);
+  CHECK_VALUE (pthread_join(thread, NULL), 0);
+
+  int val = get_tl_jit ();
+  note ("get_tl_jit returned: %d", val);
+  CHECK_VALUE (val, 42);
+
+  val = get_tl_c ();
+  note ("get_tl_c returned: %d", val);
+  CHECK_VALUE (val, 100);
+}

> ...or somesuch, though the patch is in pretty good shape already.

Thanks! Your information at 
https://gcc.gnu.org/onlinedocs/jit/internals/index.html has been 
extremely helpful.

[...]

Re: [PATCH][jit] Add thread-local globals to the libgccjit frontend

[David Malcolm]

On Tue, 2019-01-08 at 14:31 +0100, Marc Nieper-Wißkirchen wrote:
> Dear David,
> 
> thank you very much for your timely response and for talking a
> thorough 
> look at my proposed patch.
> 
> Am 07.01.19 um 21:34 schrieb David Malcolm:
> 
> > Have you done the legal paperwork with the FSF for contributing to
> > GCC?
> >   See https://gcc.gnu.org/contribute.html#legal
> 
> Not yet; this is my first patch I would like to contribute to GCC.
> You 
> should have received a private email to get the legal matters done.

Thanks; I've replied to that.

[...]


> I have applied your recent patch. With the patch, there are no more 
> failures.

Excellent.

[...]

> # of expected passes        10394


[...]


> 2019-01-08  Marc Nieper-Wißkirchen  <marc@nieper-wisskirchen.de>
> 
>          * docs/topics/compatibility.rst: Add LIBGCCJIT_ABI_11.
>          * docs/topics/expressions.rst (Global variables): Add
>          documentation of gcc_jit_lvalue_set_bool_thread_local.
> 	* docs/_build/texinfo/libgccjit.texi: Regenerate.
> 	* jit-playback.c: Include "varasm.h".
> 	Within namespace gcc::jit::playback...
> 	(context::new_global) Add "thread_local_p" param and use it
> 	to set DECL_TLS_MODEL.
> 	* jit-playback.h: Within namespace gcc::jit::playback...
> 	(context::new_global): Add "thread_local_p" param.
> 	* jit-recording.c: Within namespace gcc::jit::recording...
> 	(global::replay_into): Provide m_thread_local to call to
> 	new_global.
> 	(global::write_reproducer): Call write_reproducer_thread_local.
> 	(global::write_reproducer_thread_local): New method.
> 	* jit-recording.h: Within namespace gcc::jit::recording...
> 	(lvalue::dyn_cast_global): New virtual function.
> 	(global::m_thread_local): New field.
> 	* libgccjit.c (gcc_jit_lvalue_set_bool_thread_local): New
> 	function.
> 	* libgccjit.h
> 	(LIBGCCJIT_HAVE_gcc_jit_lvalue_set_bool_thread_local): New
> 	macro.
> 	(gcc_jit_lvalue_set_bool_thread_local): New function.
> 	* libgccjit.map (LIBGCCJIT_ABI_11): New.
> 	(gcc_jit_lvalue_set_bool_thread_local): Add.
> 	* ../testsuite/jit.dg/all-non-failing-tests.h: Include new
> test.
> 	* ../testsuite/jit.dg/jit.exp: Load pthread for tests involving
> 	thread-local globals.
> 	* ../testsuite/jit.dg/test-thread-local.c: New test case for
> 	thread-local globals.

BTW, the convention here is to split out the ChangeLog entries by
directory based on the presence of ChangeLog files.

There's a gcc/jit/ChangeLog and a gcc/testsuite/ChangeLog, so for this
patch there should be two sets of ChangeLog entries, one for each of
these, with the paths expressed relative to the directory holding the
ChangeLog.

So the testsuite entries would go into gcc/testsuite/ChangeLog, and
look like:

	* jit.dg/all-non-failing-tests.h: Include new test.

...etc.

[...]

> diff --git a/gcc/testsuite/jit.dg/test-thread-local.c
> b/gcc/testsuite/jit.dg/test-thread-local.c
> new file mode 100644
> index 000000000..287ba85e4
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-thread-local.c
> @@ -0,0 +1,99 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <pthread.h>
> +
> +#include "libgccjit.h"
> +
> +#include "harness.h"
> +
> +void
> +create_code (gcc_jit_context *ctxt, void *user_data)
> +{
> +  /* Let's try to inject the equivalent of:
> +
> +      static thread_local int tl;
> +      set_tl (int v)
> +      {
> +        tl = v;
> +      }
> +
> +      int
> +      get_tl (void)
> +      {
> +        return tl;
> +      }

Thanks for posting this.  

This test is OK as far as it goes, but there are some gaps in test
coverage: the test verifies that jit-generated code can create a new
thread-local variable, and writes to it, but it doesn't seem to test
reading from it; e.g. there doesn't seem to be a CHECK_VALUE that the
thread-local var is 43 after the set_tl (43);

It would probably also be good to verify that jit-generated code can
work with a thread-local variable declared and defined in C-generated
code.

Thinking aloud, how about the following changes:
- split out the thread-local vars there's e.g. a
    extern thread_local int tl_c;
  in the C code and the "tl" becomes "tl_jit", and access it via a 
  GCC_JIT_GLOBAL_EXTERNAL, or somesuch.
- in the test code, run two new threads, passing them two different
"int" values; verify that the set_tl(value); can be matched with a
CHECK_VALUE get_tl() == the value for that thread on the two threads.

...or somesuch, though the patch is in pretty good shape already.

Dave

> +   */
> +  gcc_jit_type *the_type =
> +    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
> +  gcc_jit_type *void_type =
> +    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
> +
> +  gcc_jit_lvalue *tl =
> +    gcc_jit_context_new_global (ctxt, NULL, GCC_JIT_GLOBAL_INTERNAL,
> +				the_type, "tl");
> +  gcc_jit_lvalue_set_bool_thread_local (tl, 1);
> +
> +  gcc_jit_param *v =
> +    gcc_jit_context_new_param (ctxt, NULL, the_type, "v");
> +  gcc_jit_param *params[1] = {v};
> +  gcc_jit_function *func =
> +    gcc_jit_context_new_function (ctxt, NULL,
> +				  GCC_JIT_FUNCTION_EXPORTED,
> +				  void_type,
> +				  "set_tl",
> +				  1, params, 0);
> +  gcc_jit_block *block =
> +    gcc_jit_function_new_block (func, "initial");
> +  gcc_jit_block_add_assignment (block, NULL, tl,
> +				gcc_jit_param_as_rvalue (v));
> +  gcc_jit_block_end_with_void_return (block, NULL);
> +
> +  func =
> +    gcc_jit_context_new_function (ctxt, NULL,
> +				  GCC_JIT_FUNCTION_EXPORTED,
> +				  the_type,
> +				  "get_tl",
> +				  0, NULL, 0);
> +  block =
> +    gcc_jit_function_new_block (func, "initial");
> +  gcc_jit_block_end_with_return (block, NULL,
> +				 gcc_jit_lvalue_as_rvalue (tl));
> +}
> +
> +static void *
> +test_thread_local_run (void *arg)
> +{
> +  void (*set_tl) (int) = arg;
> +  set_tl (43);
> +  return NULL;
> +}
> +
> +void
> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> +{
> +  typedef void (*set_tl_fn_type) (int);
> +  typedef int (*get_tl_fn_type) (void);
> +
> +  CHECK_NON_NULL (result);
> +
> +  set_tl_fn_type set_tl =
> +    (set_tl_fn_type) gcc_jit_result_get_code (result, "set_tl");
> +  get_tl_fn_type get_tl =
> +    (get_tl_fn_type) gcc_jit_result_get_code (result, "get_tl");
> +
> +  CHECK_NON_NULL (set_tl);
> +  CHECK_NON_NULL (get_tl);
> +
> +  set_tl (42);
> +
> +  pthread_t thread;
> +  CHECK_VALUE (pthread_create(&thread, NULL, test_thread_local_run,
> set_tl), 0);
> +  CHECK_VALUE (pthread_join(thread, NULL), 0);
> +
> +  int val = get_tl ();
> +
> +  note ("get_tl returned: %d", val);
> +  CHECK_VALUE (val, 42);
> +}
>