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 <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.
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 <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.
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);
+}
