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 <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). [...] 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
