[...]

2019-01-08  Marc Nieper-Wißkirchen  <m...@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  <m...@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  <m...@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.

[...]

Reply via email to