* [PATCH] libgccjit: Add support for TLS variable [PR95415]
@ 2021-05-19 0:43 Antoni Boucher
2021-05-20 20:11 ` David Malcolm
0 siblings, 1 reply; 4+ messages in thread
From: Antoni Boucher @ 2021-05-19 0:43 UTC (permalink / raw)
To: jit, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 251 bytes --]
Hello.
This patch adds support for TLS variables.
One thing to fix before we merge it is the libgccjit.map file which
contains LIBGCCJIT_ABI_16 instead of LIBGCCJIT_ABI_17.
LIBGCCJIT_ABI_16 was added in one of my other patches.
Thanks for the review.
[-- Attachment #2: 0001-Add-support-for-TLS-variable-PR95415.patch --]
[-- Type: text/x-patch, Size: 11789 bytes --]
From 6092e3d347972d331ed9ac6cae153168e98ecd0d Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Tue, 11 May 2021 19:23:54 -0400
Subject: [PATCH] Add support for TLS variable [PR95415]
2021-05-18 Antoni Boucher <bouanto@zoho.com>
gcc/jit/
PR target/95415
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_17): New ABI
tag.
* docs/topics/expressions.rst: Add document for the function
gcc_jit_lvalue_set_tls_model.
* jit-playback.h: New function (set_tls_model) and make
rvalue::m_inner public.
* jit-recording.c: New function (set_tls_model), new
variables (tls_models and tls_model_enum_strings) and support
for setting the tls model.
* jit-recording.h: New function (set_tls_model) and new
field m_tls_model.
* libgccjit.c: New function (gcc_jit_lvalue_set_tls_model).
* libgccjit.h: New function (gcc_jit_lvalue_set_tls_model)
and new enum (gcc_jit_tls_model).
* libgccjit.map (LIBGCCJIT_ABI_17): New ABI tag.
gcc/testsuite/
PR target/95415
* jit.dg/all-non-failing-tests.h: Add test-tls.c.
* jit.dg/test-tls.c: New test.
---
gcc/jit/docs/topics/compatibility.rst | 9 +++++
gcc/jit/docs/topics/expressions.rst | 28 +++++++++++++
gcc/jit/jit-playback.h | 8 ++++
gcc/jit/jit-recording.c | 41 ++++++++++++++++++--
gcc/jit/jit-recording.h | 7 +++-
gcc/jit/libgccjit.c | 18 +++++++++
gcc/jit/libgccjit.h | 21 ++++++++++
gcc/jit/libgccjit.map | 5 +++
gcc/testsuite/jit.dg/all-non-failing-tests.h | 7 ++++
gcc/testsuite/jit.dg/test-tls.c | 29 ++++++++++++++
10 files changed, 169 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-tls.c
diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 239b6aa1a92..d10bc1df080 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -243,3 +243,12 @@ embedding assembler instructions:
* :func:`gcc_jit_extended_asm_add_input_operand`
* :func:`gcc_jit_extended_asm_add_clobber`
* :func:`gcc_jit_context_add_top_level_asm`
+
+.. _LIBGCCJIT_ABI_17:
+
+``LIBGCCJIT_ABI_17``
+-----------------------
+``LIBGCCJIT_ABI_17`` covers the addition of an API entrypoint to set the
+thread-local storage model of a variable:
+
+ * :func:`gcc_jit_lvalue_set_tls_model`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 396259ef07e..68defd6a311 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -539,6 +539,34 @@ where the rvalue is computed by reading from the storage area.
in C.
+.. function:: void\
+ gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,\
+ enum gcc_jit_tls_model model)
+
+ Make a variable a thread-local variable.
+
+ The "model" parameter determines the thread-local storage model of the "lvalue":
+
+ .. type:: enum gcc_jit_tls_model
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_INITIAL_EXEC
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_LOCAL_EXEC
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_DEFAULT
+
+ This is analogous to:
+
+ .. code-block:: c
+
+ _Thread_local int foo;
+
+ in C.
+
Global variables
****************
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index 825a3e172e9..654a9c472d4 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -650,6 +650,8 @@ public:
private:
context *m_ctxt;
+
+protected:
tree m_inner;
};
@@ -670,6 +672,12 @@ public:
rvalue *
get_address (location *loc);
+ void
+ set_tls_model (enum tls_model tls_model)
+ {
+ set_decl_tls_model (m_inner, tls_model);
+ }
+
private:
bool mark_addressable (location *loc);
};
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 117ff70114c..64f3ae2d8f9 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -3713,6 +3713,12 @@ recording::lvalue::get_address (recording::location *loc)
return result;
}
+void
+recording::lvalue::set_tls_model (enum gcc_jit_tls_model model)
+{
+ m_tls_model = model;
+}
+
/* The implementation of class gcc::jit::recording::param. */
/* Implementation of pure virtual hook recording::memento::replay_into
@@ -4539,6 +4545,15 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
# pragma GCC diagnostic pop
#endif
+namespace recording {
+static const enum tls_model tls_models[] = {
+ TLS_MODEL_GLOBAL_DYNAMIC, /* GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC */
+ TLS_MODEL_LOCAL_DYNAMIC, /* GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC */
+ TLS_MODEL_INITIAL_EXEC, /* GCC_JIT_TLS_MODEL_INITIAL_EXEC */
+ TLS_MODEL_LOCAL_EXEC, /* GCC_JIT_TLS_MODEL_LOCAL_EXEC */
+};
+} /* namespace recording */
+
/* The implementation of class gcc::jit::recording::global. */
/* Implementation of pure virtual hook recording::memento::replay_into
@@ -4547,8 +4562,7 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
void
recording::global::replay_into (replayer *r)
{
- set_playback_obj (
- m_initializer
+ playback::lvalue *global = m_initializer
? r->new_global_initialized (playback_location (r, m_loc),
m_kind,
m_type->playback_type (),
@@ -4560,7 +4574,12 @@ recording::global::replay_into (replayer *r)
: r->new_global (playback_location (r, m_loc),
m_kind,
m_type->playback_type (),
- playback_string (m_name)));
+ playback_string (m_name));
+ if (m_tls_model != GCC_JIT_TLS_MODEL_DEFAULT)
+ {
+ global->set_tls_model (recording::tls_models[m_tls_model]);
+ }
+ set_playback_obj (global);
}
/* Override the default implementation of
@@ -4658,6 +4677,14 @@ recording::global::write_initializer_reproducer (const char *id, reproducer &r)
/* Implementation of recording::memento::write_reproducer for globals. */
+static const char * const tls_model_enum_strings[] = {
+ "GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC",
+ "GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC",
+ "GCC_JIT_TLS_MODEL_INITIAL_EXEC",
+ "GCC_JIT_TLS_MODEL_LOCAL_EXEC",
+ "GCC_JIT_TLS_MODEL_DEFAULT",
+};
+
void
recording::global::write_reproducer (reproducer &r)
{
@@ -4675,6 +4702,14 @@ recording::global::write_reproducer (reproducer &r)
r.get_identifier_as_type (get_type ()),
m_name->get_debug_string ());
+ if (m_tls_model)
+ {
+ r.write (" gcc_jit_lvalue_set_tls_model (%s, /* gcc_jit_lvalue *lvalue */\n"
+ " %s); /* enum gcc_jit_tls_model model */\n",
+ id,
+ tls_model_enum_strings[m_tls_model]);
+ }
+
if (m_initializer)
switch (m_type->dereference ()->get_size ())
{
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 03fa1160cf0..893bf72dd31 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1105,7 +1105,8 @@ public:
lvalue (context *ctxt,
location *loc,
type *type_)
- : rvalue (ctxt, loc, type_)
+ : rvalue (ctxt, loc, type_),
+ m_tls_model (GCC_JIT_TLS_MODEL_DEFAULT)
{}
playback::lvalue *
@@ -1127,6 +1128,10 @@ public:
const char *access_as_rvalue (reproducer &r) OVERRIDE;
virtual const char *access_as_lvalue (reproducer &r);
virtual bool is_global () const { return false; }
+ void set_tls_model (enum gcc_jit_tls_model model);
+
+protected:
+ enum gcc_jit_tls_model m_tls_model;
};
class param : public lvalue
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index 0cc650f9810..768b99499cf 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -1953,6 +1953,24 @@ gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue,
return (gcc_jit_rvalue *)lvalue->get_address (loc);
}
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, the real work is done by the
+ gcc::jit::recording::lvalue::set_tls_model method in jit-recording.c. */
+
+void
+gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,
+ enum gcc_jit_tls_model model)
+{
+ RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
+ JIT_LOG_FUNC (lvalue->get_context ()->get_logger ());
+ RETURN_IF_FAIL_PRINTF1 (lvalue->is_global (), NULL, NULL,
+ "lvalue \"%s\" not a global",
+ lvalue->get_debug_string ());
+
+ lvalue->set_tls_model (model);
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, the real work is done by the
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 5c722c2c57f..2a52b351a49 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -722,6 +722,16 @@ enum gcc_jit_function_kind
GCC_JIT_FUNCTION_ALWAYS_INLINE
};
+/* Thread local storage model. */
+enum gcc_jit_tls_model
+{
+ GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC,
+ GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC,
+ GCC_JIT_TLS_MODEL_INITIAL_EXEC,
+ GCC_JIT_TLS_MODEL_LOCAL_EXEC,
+ GCC_JIT_TLS_MODEL_DEFAULT,
+};
+
/* Create a function. */
extern gcc_jit_function *
gcc_jit_context_new_function (gcc_jit_context *ctxt,
@@ -1072,6 +1082,17 @@ extern gcc_jit_rvalue *
gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue,
gcc_jit_location *loc);
+#define LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model
+
+/* Set the thread-local storage model of a global variable
+
+ This API entrypoint was added in LIBGCCJIT_ABI_17; you can test for its
+ presence using
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model */
+extern void
+gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,
+ enum gcc_jit_tls_model model);
+
extern gcc_jit_lvalue *
gcc_jit_function_new_local (gcc_jit_function *func,
gcc_jit_location *loc,
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 337ea6c7fe4..605c624ec4a 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -205,3 +205,8 @@ LIBGCCJIT_ABI_15 {
gcc_jit_extended_asm_add_clobber;
gcc_jit_context_add_top_level_asm;
} LIBGCCJIT_ABI_14;
+
+LIBGCCJIT_ABI_16 {
+ global:
+ gcc_jit_lvalue_set_tls_model;
+} LIBGCCJIT_ABI_15;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 4202eb7798b..c2d87a30cca 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -181,6 +181,13 @@
#undef create_code
#undef verify_code
+/* test-tls.c */
+#define create_code create_code_tls
+#define verify_code verify_code_tls
+#include "test-tls.c"
+#undef create_code
+#undef verify_code
+
/* test-hello-world.c */
#define create_code create_code_hello_world
#define verify_code verify_code_hello_world
diff --git a/gcc/testsuite/jit.dg/test-tls.c b/gcc/testsuite/jit.dg/test-tls.c
new file mode 100644
index 00000000000..d4508b16c1e
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-tls.c
@@ -0,0 +1,29 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.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:
+
+ _Thread_local int foo;
+ */
+ gcc_jit_type *int_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+
+ gcc_jit_lvalue *foo =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo");
+ gcc_jit_lvalue_set_tls_model (foo, GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+}
--
2.31.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libgccjit: Add support for TLS variable [PR95415]
2021-05-19 0:43 [PATCH] libgccjit: Add support for TLS variable [PR95415] Antoni Boucher
@ 2021-05-20 20:11 ` David Malcolm
2021-11-20 22:34 ` Antoni Boucher
0 siblings, 1 reply; 4+ messages in thread
From: David Malcolm @ 2021-05-20 20:11 UTC (permalink / raw)
To: Antoni Boucher, jit, gcc-patches
On Tue, 2021-05-18 at 20:43 -0400, Antoni Boucher via Gcc-patches
wrote:
> Hello.
> This patch adds support for TLS variables.
> One thing to fix before we merge it is the libgccjit.map file which
> contains LIBGCCJIT_ABI_16 instead of LIBGCCJIT_ABI_17.
> LIBGCCJIT_ABI_16 was added in one of my other patches.
> Thanks for the review.
> diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
> index 239b6aa1a92..d10bc1df080 100644
> --- a/gcc/jit/docs/topics/compatibility.rst
> +++ b/gcc/jit/docs/topics/compatibility.rst
> @@ -243,3 +243,12 @@ embedding assembler instructions:
> * :func:`gcc_jit_extended_asm_add_input_operand`
> * :func:`gcc_jit_extended_asm_add_clobber`
> * :func:`gcc_jit_context_add_top_level_asm`
> +
> +.. _LIBGCCJIT_ABI_17:
> +
> +``LIBGCCJIT_ABI_17``
> +-----------------------
> +``LIBGCCJIT_ABI_17`` covers the addition of an API entrypoint to set the
> +thread-local storage model of a variable:
> +
> + * :func:`gcc_jit_lvalue_set_tls_model`
Sorry about the delay in reviewing patches.
Is there a summary somewhere of the various outstanding patches and
their associated ABI versions? Are there dependencies between the
patches?
> diff --git a/gcc/jit/docs/topics/expressions.rst
b/gcc/jit/docs/topics/expressions.rst
> index 396259ef07e..68defd6a311 100644
> --- a/gcc/jit/docs/topics/expressions.rst
> +++ b/gcc/jit/docs/topics/expressions.rst
> @@ -539,6 +539,34 @@ where the rvalue is computed by reading from the storage area.
>
> in C.
>
> +.. function:: void\
> + gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,\
> + enum gcc_jit_tls_model model)
> +
> + Make a variable a thread-local variable.
> +
> + The "model" parameter determines the thread-local storage model of the "lvalue":
> +
> + .. type:: enum gcc_jit_tls_model
> +
> + .. c:macro:: GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC
> +
> + .. c:macro:: GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC
> +
> + .. c:macro:: GCC_JIT_TLS_MODEL_INITIAL_EXEC
> +
> + .. c:macro:: GCC_JIT_TLS_MODEL_LOCAL_EXEC
> +
> + .. c:macro:: GCC_JIT_TLS_MODEL_DEFAULT
> +
> + This is analogous to:
> +
> + .. code-block:: c
> +
> + _Thread_local int foo;
> +
> + in C.
This comment needs the usual "This entrypoint was added in" text to
state which API version it was added in.
I confess to being a bit hazy on the different TLS models, and it's
unclear to me what all the different enum values do. Is this
equivalent to the various values for __attribute__((tls_model(VALUE)))
? This attribute is documented in
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html,
though sadly that document doesn't seem to have a good anchor for that
attribute.
https://gcc.gnu.org/onlinedocs/gcc/Thread-Local.html currently links to
https://www.akkadia.org/drepper/tls.pdf "for a detailed explanation of
the four thread-local storage addressing models, and how the runtime is
expected to function."
One thing that should be clarified: does GCC_JIT_TLS_MODEL_DEFAULT mean
(a) thread-local storage, using a default model, or
(b) non-thread-local storage i.e. normal storage.
?
Reading the docs I thought it meant (a), but when I looked in more
detail at the implementation it looks like it means (b); is it meant
to? This needs clarifying.
Are you using all of these enum values in your code? Is this something
you need to expose for the rustc backend?
> Global variables
> ****************
>
> diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
> index 825a3e172e9..654a9c472d4 100644
> --- a/gcc/jit/jit-playback.h
> +++ b/gcc/jit/jit-playback.h
> @@ -650,6 +650,8 @@ public:
>
> private:
> context *m_ctxt;
> +
> +protected:
> tree m_inner;
> };
As noted in another review, I don't think you need to make this
protected...
>
> @@ -670,6 +672,12 @@ public:
> rvalue *
> get_address (location *loc);
>
> + void
> + set_tls_model (enum tls_model tls_model)
> + {
> + set_decl_tls_model (m_inner, tls_model);
> + }
...as I think you can use "as_tree ()" to get at m_inner here.
> diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
> index 117ff70114c..64f3ae2d8f9 100644
> --- a/gcc/jit/jit-recording.c
> +++ b/gcc/jit/jit-recording.c
> @@ -3713,6 +3713,12 @@ recording::lvalue::get_address (recording::location *loc)
> return result;
> }
>
> +void
> +recording::lvalue::set_tls_model (enum gcc_jit_tls_model model)
> +{
> + m_tls_model = model;
> +}
> +
> /* The implementation of class gcc::jit::recording::param. */
>
> /* Implementation of pure virtual hook recording::memento::replay_into
> @@ -4539,6 +4545,15 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
> # pragma GCC diagnostic pop
> #endif
>
> +namespace recording {
> +static const enum tls_model tls_models[] = {
> + TLS_MODEL_GLOBAL_DYNAMIC, /* GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC */
> + TLS_MODEL_LOCAL_DYNAMIC, /* GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC */
> + TLS_MODEL_INITIAL_EXEC, /* GCC_JIT_TLS_MODEL_INITIAL_EXEC */
> + TLS_MODEL_LOCAL_EXEC, /* GCC_JIT_TLS_MODEL_LOCAL_EXEC */
> +};
> +} /* namespace recording */
> +
> /* The implementation of class gcc::jit::recording::global. */
>
> /* Implementation of pure virtual hook recording::memento::replay_into
> @@ -4547,8 +4562,7 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
> void
> recording::global::replay_into (replayer *r)
> {
> - set_playback_obj (
> - m_initializer
> + playback::lvalue *global = m_initializer
> ? r->new_global_initialized (playback_location (r, m_loc),
> m_kind,
> m_type->playback_type (),
> @@ -4560,7 +4574,12 @@ recording::global::replay_into (replayer *r)
> : r->new_global (playback_location (r, m_loc),
> m_kind,
> m_type->playback_type (),
> - playback_string (m_name)));
> + playback_string (m_name));
> + if (m_tls_model != GCC_JIT_TLS_MODEL_DEFAULT)
> + {
> + global->set_tls_model (recording::tls_models[m_tls_model]);
> + }
> + set_playback_obj (global);
> }
>
> /* Override the default implementation of
[...snip...]
> @@ -4675,6 +4702,14 @@ recording::global::write_reproducer (reproducer &r)
> r.get_identifier_as_type (get_type ()),
> m_name->get_debug_string ());
>
> + if (m_tls_model)
> + {
I think this conditional should be:
if (m_tls_model != GCC_JIT_TLS_MODEL_DEFAULT)
since the default value isn't 0. (Maybe it should be?)
> + r.write (" gcc_jit_lvalue_set_tls_model (%s, /*
gcc_jit_lvalue *lvalue */\n"
> + " %s); /* enum gcc_jit_tls_model model */\n",
> + id,
> + tls_model_enum_strings[m_tls_model]);
> + }
> +
> if (m_initializer)
> switch (m_type->dereference ()->get_size ())
> {
[...snip...]
> class param : public lvalue
> diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> index 0cc650f9810..768b99499cf 100644
> --- a/gcc/jit/libgccjit.c
> +++ b/gcc/jit/libgccjit.c
> @@ -1953,6 +1953,24 @@ gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue,
> return (gcc_jit_rvalue *)lvalue->get_address (loc);
> }
>
> +/* Public entrypoint. See description in libgccjit.h.
> +
> + After error-checking, the real work is done by the
> + gcc::jit::recording::lvalue::set_tls_model method in jit-recording.c. */
> +
> +void
> +gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,
> + enum gcc_jit_tls_model model)
> +{
> + RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
> + JIT_LOG_FUNC (lvalue->get_context ()->get_logger ());
> + RETURN_IF_FAIL_PRINTF1 (lvalue->is_global (), NULL, NULL,
> + "lvalue \"%s\" not a global",
> + lvalue->get_debug_string ());
This should pass lvalue's context to the RETURN_IF_FAIL_PRINTF1, so
that if it fails, the error is associated with the context.
> + lvalue->set_tls_model (model);
> +}
> +
> /* Public entrypoint. See description in libgccjit.h.
>
> After error-checking, the real work is done by the
> diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
> index 5c722c2c57f..2a52b351a49 100644
> --- a/gcc/jit/libgccjit.h
> +++ b/gcc/jit/libgccjit.h
> @@ -722,6 +722,16 @@ enum gcc_jit_function_kind
> GCC_JIT_FUNCTION_ALWAYS_INLINE
> };
>
> +/* Thread local storage model. */
> +enum gcc_jit_tls_model
> +{
> + GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC,
> + GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC,
> + GCC_JIT_TLS_MODEL_INITIAL_EXEC,
> + GCC_JIT_TLS_MODEL_LOCAL_EXEC,
> + GCC_JIT_TLS_MODEL_DEFAULT,
As noted above, should the DEFAULT one be the first?
If DEFAULT means "not thread-local", maybe "NONE" would be clearer than
"DEFAULT"?
> +};
> +
> /* Create a function. */
> extern gcc_jit_function *
> gcc_jit_context_new_function (gcc_jit_context *ctxt,
> @@ -1072,6 +1082,17 @@ extern gcc_jit_rvalue *
> gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue,
> gcc_jit_location *loc);
>
> +#define LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model
> +
> +/* Set the thread-local storage model of a global variable
> +
> + This API entrypoint was added in LIBGCCJIT_ABI_17; you can test for its
> + presence using
> + #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model */
> +extern void
> +gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,
> + enum gcc_jit_tls_model model);
> +
> extern gcc_jit_lvalue *
> gcc_jit_function_new_local (gcc_jit_function *func,
> gcc_jit_location *loc,
> diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
> index 337ea6c7fe4..605c624ec4a 100644
> --- a/gcc/jit/libgccjit.map
> +++ b/gcc/jit/libgccjit.map
> @@ -205,3 +205,8 @@ LIBGCCJIT_ABI_15 {
> gcc_jit_extended_asm_add_clobber;
> gcc_jit_context_add_top_level_asm;
> } LIBGCCJIT_ABI_14;
> +
> +LIBGCCJIT_ABI_16 {
> + global:
> + gcc_jit_lvalue_set_tls_model;
> +} LIBGCCJIT_ABI_15;
Obviously the ABI needs to be fixed up here.
> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> index 4202eb7798b..c2d87a30cca 100644
> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> @@ -181,6 +181,13 @@
> #undef create_code
> #undef verify_code
>
> +/* test-tls.c */
> +#define create_code create_code_tls
> +#define verify_code verify_code_tls
> +#include "test-tls.c"
> +#undef create_code
> +#undef verify_code
> +
> /* test-hello-world.c */
> #define create_code create_code_hello_world
> #define verify_code verify_code_hello_world
This is missing an entry in the "testcases" array at the bottom of the
header to make use of the new {create|verify}_code_tls functions.
> diff --git a/gcc/testsuite/jit.dg/test-tls.c b/gcc/testsuite/jit.dg/test-tls.c
> new file mode 100644
> index 00000000000..d4508b16c1e
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-tls.c
> @@ -0,0 +1,29 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <time.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:
> +
> + _Thread_local int foo;
> + */
> + gcc_jit_type *int_type =
> + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
> +
> + gcc_jit_lvalue *foo =
> + gcc_jit_context_new_global (
> + ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo");
> + gcc_jit_lvalue_set_tls_model (foo, GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC);
How many of the different enum values can be supported? How target-
dependent is this?
> +}
> +
> +void
> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> +{
Should probably at least try to read and write the global(s).
Hope this is constructive
Dave
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libgccjit: Add support for TLS variable [PR95415]
2021-05-20 20:11 ` David Malcolm
@ 2021-11-20 22:34 ` Antoni Boucher
2021-11-21 20:28 ` David Malcolm
0 siblings, 1 reply; 4+ messages in thread
From: Antoni Boucher @ 2021-11-20 22:34 UTC (permalink / raw)
To: David Malcolm, jit, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 14762 bytes --]
Hi.
Here's the updated patch.
See comments below.
Thanks for your reviews!
Le jeudi 20 mai 2021 à 16:11 -0400, David Malcolm a écrit :
> On Tue, 2021-05-18 at 20:43 -0400, Antoni Boucher via Gcc-patches
> wrote:
> > Hello.
> > This patch adds support for TLS variables.
> > One thing to fix before we merge it is the libgccjit.map file which
> > contains LIBGCCJIT_ABI_16 instead of LIBGCCJIT_ABI_17.
> > LIBGCCJIT_ABI_16 was added in one of my other patches.
> > Thanks for the review.
>
> > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > b/gcc/jit/docs/topics/compatibility.rst
> > index 239b6aa1a92..d10bc1df080 100644
> > --- a/gcc/jit/docs/topics/compatibility.rst
> > +++ b/gcc/jit/docs/topics/compatibility.rst
> > @@ -243,3 +243,12 @@ embedding assembler instructions:
> > * :func:`gcc_jit_extended_asm_add_input_operand`
> > * :func:`gcc_jit_extended_asm_add_clobber`
> > * :func:`gcc_jit_context_add_top_level_asm`
> > +
> > +.. _LIBGCCJIT_ABI_17:
> > +
> > +``LIBGCCJIT_ABI_17``
> > +-----------------------
> > +``LIBGCCJIT_ABI_17`` covers the addition of an API entrypoint to set
> > the
> > +thread-local storage model of a variable:
> > +
> > + * :func:`gcc_jit_lvalue_set_tls_model`
>
> Sorry about the delay in reviewing patches.
>
> Is there a summary somewhere of the various outstanding patches and
> their associated ABI versions? Are there dependencies between the
> patches?
The list of patches is there:
https://github.com/antoyo/libgccjit-patches but I don't keep them up-
to-date.
If that would help you, I could add a README to tell what is the new
ABI version for each patch.
I believe there might be some patches that depend on a previous one.
>
> > diff --git a/gcc/jit/docs/topics/expressions.rst
> b/gcc/jit/docs/topics/expressions.rst
> > index 396259ef07e..68defd6a311 100644
> > --- a/gcc/jit/docs/topics/expressions.rst
> > +++ b/gcc/jit/docs/topics/expressions.rst
> > @@ -539,6 +539,34 @@ where the rvalue is computed by reading from
> > the storage area.
> >
> > in C.
> >
> > +.. function:: void\
> > + gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue
> > *lvalue,\
> > + enum gcc_jit_tls_model
> > model)
> > +
> > + Make a variable a thread-local variable.
> > +
> > + The "model" parameter determines the thread-local storage model
> > of the "lvalue":
> > +
> > + .. type:: enum gcc_jit_tls_model
> > +
> > + .. c:macro:: GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC
> > +
> > + .. c:macro:: GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC
> > +
> > + .. c:macro:: GCC_JIT_TLS_MODEL_INITIAL_EXEC
> > +
> > + .. c:macro:: GCC_JIT_TLS_MODEL_LOCAL_EXEC
> > +
> > + .. c:macro:: GCC_JIT_TLS_MODEL_DEFAULT
> > +
> > + This is analogous to:
> > +
> > + .. code-block:: c
> > +
> > + _Thread_local int foo;
> > +
> > + in C.
>
> This comment needs the usual "This entrypoint was added in" text to
> state which API version it was added in.
>
> I confess to being a bit hazy on the different TLS models, and it's
> unclear to me what all the different enum values do. Is this
> equivalent to the various values for
> __attribute__((tls_model(VALUE)))
> ? This attribute is documented in
> https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html,
> though sadly that document doesn't seem to have a good anchor for
> that
> attribute.
Yes, it is the equivalent of this attribute.
>
> https://gcc.gnu.org/onlinedocs/gcc/Thread-Local.html currently links
> to
> https://www.akkadia.org/drepper/tls.pdf "for a detailed explanation
> of
> the four thread-local storage addressing models, and how the runtime
> is
> expected to function."
>
> One thing that should be clarified: does GCC_JIT_TLS_MODEL_DEFAULT
> mean
> (a) thread-local storage, using a default model, or
> (b) non-thread-local storage i.e. normal storage.
>
> ?
>
> Reading the docs I thought it meant (a), but when I looked in more
> detail at the implementation it looks like it means (b); is it meant
> to? This needs clarifying.
>
> Are you using all of these enum values in your code? Is this
> something
> you need to expose for the rustc backend?
Yes, I use all of these enum values in the rustc gcc codegen.
>
>
> > Global variables
> > ****************
> >
> > diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
> > index 825a3e172e9..654a9c472d4 100644
> > --- a/gcc/jit/jit-playback.h
> > +++ b/gcc/jit/jit-playback.h
> > @@ -650,6 +650,8 @@ public:
> >
> > private:
> > context *m_ctxt;
> > +
> > +protected:
> > tree m_inner;
> > };
>
> As noted in another review, I don't think you need to make this
> protected...
>
> >
> > @@ -670,6 +672,12 @@ public:
> > rvalue *
> > get_address (location *loc);
> >
> > + void
> > + set_tls_model (enum tls_model tls_model)
> > + {
> > + set_decl_tls_model (m_inner, tls_model);
> > + }
>
> ...as I think you can use "as_tree ()" to get at m_inner here.
>
>
> > diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
> > index 117ff70114c..64f3ae2d8f9 100644
> > --- a/gcc/jit/jit-recording.c
> > +++ b/gcc/jit/jit-recording.c
> > @@ -3713,6 +3713,12 @@ recording::lvalue::get_address
> > (recording::location *loc)
> > return result;
> > }
> >
> > +void
> > +recording::lvalue::set_tls_model (enum gcc_jit_tls_model model)
> > +{
> > + m_tls_model = model;
> > +}
> > +
> > /* The implementation of class gcc::jit::recording::param. */
> >
> > /* Implementation of pure virtual hook
> > recording::memento::replay_into
> > @@ -4539,6 +4545,15 @@ recording::block::dump_edges_to_dot
> > (pretty_printer *pp)
> > # pragma GCC diagnostic pop
> > #endif
> >
> > +namespace recording {
> > +static const enum tls_model tls_models[] = {
> > + TLS_MODEL_GLOBAL_DYNAMIC, /* GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC */
> > + TLS_MODEL_LOCAL_DYNAMIC, /* GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC */
> > + TLS_MODEL_INITIAL_EXEC, /* GCC_JIT_TLS_MODEL_INITIAL_EXEC */
> > + TLS_MODEL_LOCAL_EXEC, /* GCC_JIT_TLS_MODEL_LOCAL_EXEC */
> > +};
> > +} /* namespace recording */
> > +
> > /* The implementation of class gcc::jit::recording::global. */
> >
> > /* Implementation of pure virtual hook
> > recording::memento::replay_into
> > @@ -4547,8 +4562,7 @@ recording::block::dump_edges_to_dot
> > (pretty_printer *pp)
> > void
> > recording::global::replay_into (replayer *r)
> > {
> > - set_playback_obj (
> > - m_initializer
> > + playback::lvalue *global = m_initializer
> > ? r->new_global_initialized (playback_location (r, m_loc),
> > m_kind,
> > m_type->playback_type (),
> > @@ -4560,7 +4574,12 @@ recording::global::replay_into (replayer *r)
> > : r->new_global (playback_location (r, m_loc),
> > m_kind,
> > m_type->playback_type (),
> > - playback_string (m_name)));
> > + playback_string (m_name));
> > + if (m_tls_model != GCC_JIT_TLS_MODEL_DEFAULT)
> > + {
> > + global->set_tls_model (recording::tls_models[m_tls_model]);
> > + }
> > + set_playback_obj (global);
> > }
> >
> > /* Override the default implementation of
>
> [...snip...]
>
> > @@ -4675,6 +4702,14 @@ recording::global::write_reproducer
> > (reproducer &r)
> > r.get_identifier_as_type (get_type ()),
> > m_name->get_debug_string ());
> >
> > + if (m_tls_model)
> > + {
>
> I think this conditional should be:
>
> if (m_tls_model != GCC_JIT_TLS_MODEL_DEFAULT)
>
> since the default value isn't 0. (Maybe it should be?)
>
> > + r.write (" gcc_jit_lvalue_set_tls_model (%s, /*
> gcc_jit_lvalue *lvalue */\n"
> > + " %s); /* enum
> > gcc_jit_tls_model model */\n",
> > + id,
> > + tls_model_enum_strings[m_tls_model]);
> > + }
> > +
> > if (m_initializer)
> > switch (m_type->dereference ()->get_size ())
> > {
>
> [...snip...]
>
> > class param : public lvalue
> > diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> > index 0cc650f9810..768b99499cf 100644
> > --- a/gcc/jit/libgccjit.c
> > +++ b/gcc/jit/libgccjit.c
> > @@ -1953,6 +1953,24 @@ gcc_jit_lvalue_get_address (gcc_jit_lvalue
> > *lvalue,
> > return (gcc_jit_rvalue *)lvalue->get_address (loc);
> > }
> >
> > +/* Public entrypoint. See description in libgccjit.h.
> > +
> > + After error-checking, the real work is done by the
> > + gcc::jit::recording::lvalue::set_tls_model method in jit-
> > recording.c. */
> > +
> > +void
> > +gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,
> > + enum gcc_jit_tls_model model)
> > +{
> > + RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
> > + JIT_LOG_FUNC (lvalue->get_context ()->get_logger ());
> > + RETURN_IF_FAIL_PRINTF1 (lvalue->is_global (), NULL, NULL,
> > + "lvalue \"%s\" not a global",
> > + lvalue->get_debug_string ());
>
> This should pass lvalue's context to the RETURN_IF_FAIL_PRINTF1, so
> that if it fails, the error is associated with the context.
>
>
> > + lvalue->set_tls_model (model);
> > +}
> > +
> > /* Public entrypoint. See description in libgccjit.h.
> >
> > After error-checking, the real work is done by the
> > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
> > index 5c722c2c57f..2a52b351a49 100644
> > --- a/gcc/jit/libgccjit.h
> > +++ b/gcc/jit/libgccjit.h
> > @@ -722,6 +722,16 @@ enum gcc_jit_function_kind
> > GCC_JIT_FUNCTION_ALWAYS_INLINE
> > };
> >
> > +/* Thread local storage model. */
> > +enum gcc_jit_tls_model
> > +{
> > + GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC,
> > + GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC,
> > + GCC_JIT_TLS_MODEL_INITIAL_EXEC,
> > + GCC_JIT_TLS_MODEL_LOCAL_EXEC,
> > + GCC_JIT_TLS_MODEL_DEFAULT,
>
> As noted above, should the DEFAULT one be the first?
I made it the first one.
>
> If DEFAULT means "not thread-local", maybe "NONE" would be clearer
> than
> "DEFAULT"?
>
> > +};
> > +
> > /* Create a function. */
> > extern gcc_jit_function *
> > gcc_jit_context_new_function (gcc_jit_context *ctxt,
> > @@ -1072,6 +1082,17 @@ extern gcc_jit_rvalue *
> > gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue,
> > gcc_jit_location *loc);
> >
> > +#define LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model
> > +
> > +/* Set the thread-local storage model of a global variable
> > +
> > + This API entrypoint was added in LIBGCCJIT_ABI_17; you can test
> > for its
> > + presence using
> > + #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model */
> > +extern void
> > +gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,
> > + enum gcc_jit_tls_model model);
> > +
> > extern gcc_jit_lvalue *
> > gcc_jit_function_new_local (gcc_jit_function *func,
> > gcc_jit_location *loc,
> > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
> > index 337ea6c7fe4..605c624ec4a 100644
> > --- a/gcc/jit/libgccjit.map
> > +++ b/gcc/jit/libgccjit.map
> > @@ -205,3 +205,8 @@ LIBGCCJIT_ABI_15 {
> > gcc_jit_extended_asm_add_clobber;
> > gcc_jit_context_add_top_level_asm;
> > } LIBGCCJIT_ABI_14;
> > +
> > +LIBGCCJIT_ABI_16 {
> > + global:
> > + gcc_jit_lvalue_set_tls_model;
> > +} LIBGCCJIT_ABI_15;
>
> Obviously the ABI needs to be fixed up here.
>
> > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > index 4202eb7798b..c2d87a30cca 100644
> > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
> > @@ -181,6 +181,13 @@
> > #undef create_code
> > #undef verify_code
> >
> > +/* test-tls.c */
> > +#define create_code create_code_tls
> > +#define verify_code verify_code_tls
> > +#include "test-tls.c"
> > +#undef create_code
> > +#undef verify_code
> > +
> > /* test-hello-world.c */
> > #define create_code create_code_hello_world
> > #define verify_code verify_code_hello_world
>
> This is missing an entry in the "testcases" array at the bottom of
> the
> header to make use of the new {create|verify}_code_tls functions.
>
> > diff --git a/gcc/testsuite/jit.dg/test-tls.c
> > b/gcc/testsuite/jit.dg/test-tls.c
> > new file mode 100644
> > index 00000000000..d4508b16c1e
> > --- /dev/null
> > +++ b/gcc/testsuite/jit.dg/test-tls.c
> > @@ -0,0 +1,29 @@
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <time.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:
> > +
> > + _Thread_local int foo;
> > + */
> > + gcc_jit_type *int_type =
> > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
> > +
> > + gcc_jit_lvalue *foo =
> > + gcc_jit_context_new_global (
> > + ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo");
> > + gcc_jit_lvalue_set_tls_model (foo,
> > GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC);
>
> How many of the different enum values can be supported? How target-
> dependent is this?
I'm not sure what you mean here. Are you asking that I test all the
different enum values?
The tls_model enum is defined in gcc/coretypes.h and does not seem to
change depending on the target. Maybe there are checks elsewhere for
that, though.
>
> > +}
> > +
> > +void
> > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> > +{
>
> Should probably at least try to read and write the global(s).
>
> Hope this is constructive
> Dave
>
>
[-- Attachment #2: 0001-libgccjit-Add-support-for-TLS-variable-PR95415.patch --]
[-- Type: text/x-patch, Size: 13150 bytes --]
From d64f50ff9551d1bd49c9e5304f2d056b031d046b Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Tue, 11 May 2021 19:23:54 -0400
Subject: [PATCH] libgccjit: Add support for TLS variable [PR95415]
2021-11-20 Antoni Boucher <bouanto@zoho.com>
gcc/jit/
PR target/95415
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_17): New ABI
tag.
* docs/topics/expressions.rst: Add document for the function
gcc_jit_lvalue_set_tls_model.
* jit-playback.h: New function (set_tls_model) and make
rvalue::m_inner public.
* jit-recording.c: New function (set_tls_model), new
variables (tls_models and tls_model_enum_strings) and support
for setting the tls model.
* jit-recording.h: New function (set_tls_model) and new
field m_tls_model.
* libgccjit.c: New function (gcc_jit_lvalue_set_tls_model).
* libgccjit.h: New function (gcc_jit_lvalue_set_tls_model)
and new enum (gcc_jit_tls_model).
* libgccjit.map (LIBGCCJIT_ABI_17): New ABI tag.
gcc/testsuite/
PR target/95415
* jit.dg/all-non-failing-tests.h: Add test-tls.c.
* jit.dg/test-tls.c: New test.
---
gcc/jit/docs/topics/compatibility.rst | 9 +++
gcc/jit/docs/topics/expressions.rst | 37 +++++++++++
gcc/jit/jit-playback.h | 6 ++
gcc/jit/jit-recording.c | 42 ++++++++++++-
gcc/jit/jit-recording.h | 7 ++-
gcc/jit/libgccjit.c | 18 ++++++
gcc/jit/libgccjit.h | 21 +++++++
gcc/jit/libgccjit.map | 5 ++
gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
gcc/testsuite/jit.dg/test-tls.c | 64 ++++++++++++++++++++
10 files changed, 215 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-tls.c
diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 52ee3f860a7..2ad6e4232f7 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -284,3 +284,12 @@ entrypoints:
* :func:`gcc_jit_struct_get_field`
* :func:`gcc_jit_struct_get_field_count`
+
+.. _LIBGCCJIT_ABI_17:
+
+``LIBGCCJIT_ABI_17``
+-----------------------
+``LIBGCCJIT_ABI_17`` covers the addition of an API entrypoint to set the
+thread-local storage model of a variable:
+
+ * :func:`gcc_jit_lvalue_set_tls_model`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 396259ef07e..386b80d8f8b 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -539,6 +539,43 @@ where the rvalue is computed by reading from the storage area.
in C.
+.. function:: void\
+ gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,\
+ enum gcc_jit_tls_model model)
+
+ Make a variable a thread-local variable.
+
+ The "model" parameter determines the thread-local storage model of the "lvalue":
+
+ .. type:: enum gcc_jit_tls_model
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_NONE
+
+ Don't set the TLS model.
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_INITIAL_EXEC
+
+ .. c:macro:: GCC_JIT_TLS_MODEL_LOCAL_EXEC
+
+ This is analogous to:
+
+ .. code-block:: c
+
+ _Thread_local int foo __attribute__ ((tls_model("MODEL")));
+
+ in C.
+
+ This entrypoint was added in :ref:`LIBGCCJIT_ABI_17`; you can test for
+ its presence using
+
+ .. code-block:: c
+
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model
+
Global variables
****************
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index f670c9e81df..c9839c21a66 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -675,6 +675,12 @@ public:
rvalue *
get_address (location *loc);
+ void
+ set_tls_model (enum tls_model tls_model)
+ {
+ set_decl_tls_model (as_tree (), tls_model);
+ }
+
private:
bool mark_addressable (location *loc);
};
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 117ff70114c..1296fece4cf 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -3713,6 +3713,12 @@ recording::lvalue::get_address (recording::location *loc)
return result;
}
+void
+recording::lvalue::set_tls_model (enum gcc_jit_tls_model model)
+{
+ m_tls_model = model;
+}
+
/* The implementation of class gcc::jit::recording::param. */
/* Implementation of pure virtual hook recording::memento::replay_into
@@ -4539,6 +4545,16 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
# pragma GCC diagnostic pop
#endif
+namespace recording {
+static const enum tls_model tls_models[] = {
+ TLS_MODEL_NONE, /* GCC_JIT_TLS_MODEL_NONE */
+ TLS_MODEL_GLOBAL_DYNAMIC, /* GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC */
+ TLS_MODEL_LOCAL_DYNAMIC, /* GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC */
+ TLS_MODEL_INITIAL_EXEC, /* GCC_JIT_TLS_MODEL_INITIAL_EXEC */
+ TLS_MODEL_LOCAL_EXEC, /* GCC_JIT_TLS_MODEL_LOCAL_EXEC */
+};
+} /* namespace recording */
+
/* The implementation of class gcc::jit::recording::global. */
/* Implementation of pure virtual hook recording::memento::replay_into
@@ -4547,8 +4563,7 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
void
recording::global::replay_into (replayer *r)
{
- set_playback_obj (
- m_initializer
+ playback::lvalue *global = m_initializer
? r->new_global_initialized (playback_location (r, m_loc),
m_kind,
m_type->playback_type (),
@@ -4560,7 +4575,12 @@ recording::global::replay_into (replayer *r)
: r->new_global (playback_location (r, m_loc),
m_kind,
m_type->playback_type (),
- playback_string (m_name)));
+ playback_string (m_name));
+ if (m_tls_model != GCC_JIT_TLS_MODEL_NONE)
+ {
+ global->set_tls_model (recording::tls_models[m_tls_model]);
+ }
+ set_playback_obj (global);
}
/* Override the default implementation of
@@ -4658,6 +4678,14 @@ recording::global::write_initializer_reproducer (const char *id, reproducer &r)
/* Implementation of recording::memento::write_reproducer for globals. */
+static const char * const tls_model_enum_strings[] = {
+ "GCC_JIT_TLS_MODEL_NONE",
+ "GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC",
+ "GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC",
+ "GCC_JIT_TLS_MODEL_INITIAL_EXEC",
+ "GCC_JIT_TLS_MODEL_LOCAL_EXEC",
+};
+
void
recording::global::write_reproducer (reproducer &r)
{
@@ -4675,6 +4703,14 @@ recording::global::write_reproducer (reproducer &r)
r.get_identifier_as_type (get_type ()),
m_name->get_debug_string ());
+ if (m_tls_model != GCC_JIT_TLS_MODEL_NONE)
+ {
+ r.write (" gcc_jit_lvalue_set_tls_model (%s, /* gcc_jit_lvalue *lvalue */\n"
+ " %s); /* enum gcc_jit_tls_model model */\n",
+ id,
+ tls_model_enum_strings[m_tls_model]);
+ }
+
if (m_initializer)
switch (m_type->dereference ()->get_size ())
{
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 4a994fe7094..f64e9bee9a6 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1112,7 +1112,8 @@ public:
lvalue (context *ctxt,
location *loc,
type *type_)
- : rvalue (ctxt, loc, type_)
+ : rvalue (ctxt, loc, type_),
+ m_tls_model (GCC_JIT_TLS_MODEL_NONE)
{}
playback::lvalue *
@@ -1134,6 +1135,10 @@ public:
const char *access_as_rvalue (reproducer &r) OVERRIDE;
virtual const char *access_as_lvalue (reproducer &r);
virtual bool is_global () const { return false; }
+ void set_tls_model (enum gcc_jit_tls_model model);
+
+protected:
+ enum gcc_jit_tls_model m_tls_model;
};
class param : public lvalue
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index c744b634f4b..e16eed4a6c0 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -2217,6 +2217,24 @@ gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue,
return (gcc_jit_rvalue *)lvalue->get_address (loc);
}
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, the real work is done by the
+ gcc::jit::recording::lvalue::set_tls_model method in jit-recording.c. */
+
+void
+gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,
+ enum gcc_jit_tls_model model)
+{
+ RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
+ JIT_LOG_FUNC (lvalue->get_context ()->get_logger ());
+ RETURN_IF_FAIL_PRINTF1 (lvalue->is_global (), lvalue->get_context (), NULL,
+ "lvalue \"%s\" not a global",
+ lvalue->get_debug_string ());
+
+ lvalue->set_tls_model (model);
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, the real work is done by the
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index a1c9436c545..5aa2e40c7a4 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -739,6 +739,16 @@ enum gcc_jit_function_kind
GCC_JIT_FUNCTION_ALWAYS_INLINE
};
+/* Thread local storage model. */
+enum gcc_jit_tls_model
+{
+ GCC_JIT_TLS_MODEL_NONE,
+ GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC,
+ GCC_JIT_TLS_MODEL_LOCAL_DYNAMIC,
+ GCC_JIT_TLS_MODEL_INITIAL_EXEC,
+ GCC_JIT_TLS_MODEL_LOCAL_EXEC,
+};
+
/* Create a function. */
extern gcc_jit_function *
gcc_jit_context_new_function (gcc_jit_context *ctxt,
@@ -1089,6 +1099,17 @@ extern gcc_jit_rvalue *
gcc_jit_lvalue_get_address (gcc_jit_lvalue *lvalue,
gcc_jit_location *loc);
+#define LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model
+
+/* Set the thread-local storage model of a global variable
+
+ This API entrypoint was added in LIBGCCJIT_ABI_17; you can test for its
+ presence using
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_tls_model */
+extern void
+gcc_jit_lvalue_set_tls_model (gcc_jit_lvalue *lvalue,
+ enum gcc_jit_tls_model model);
+
extern gcc_jit_lvalue *
gcc_jit_function_new_local (gcc_jit_function *func,
gcc_jit_location *loc,
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 64e790949e8..98d693fd00c 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -226,3 +226,8 @@ LIBGCCJIT_ABI_16 {
gcc_jit_type_is_struct;
gcc_jit_struct_get_field_count;
} LIBGCCJIT_ABI_15;
+
+LIBGCCJIT_ABI_17 {
+ global:
+ gcc_jit_lvalue_set_tls_model;
+} LIBGCCJIT_ABI_16;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index a7fddf96db8..f64ea91b14e 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -188,6 +188,13 @@
#undef create_code
#undef verify_code
+/* test-tls.c */
+#define create_code create_code_tls
+#define verify_code verify_code_tls
+#include "test-tls.c"
+#undef create_code
+#undef verify_code
+
/* test-hello-world.c */
#define create_code create_code_hello_world
#define verify_code verify_code_hello_world
@@ -453,6 +460,9 @@ const struct testcase testcases[] = {
{"switch",
create_code_switch,
verify_code_switch},
+ {"tls",
+ create_code_tls,
+ verify_code_tls},
{"types",
create_code_types,
verify_code_types},
diff --git a/gcc/testsuite/jit.dg/test-tls.c b/gcc/testsuite/jit.dg/test-tls.c
new file mode 100644
index 00000000000..3b20182ac10
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-tls.c
@@ -0,0 +1,64 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.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:
+
+ _Thread_local int foo;
+
+ int test_using_tls()
+ {
+ foo = 42;
+ return foo;
+ }
+ */
+ gcc_jit_type *int_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+
+ gcc_jit_lvalue *foo =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo");
+ gcc_jit_lvalue_set_tls_model (foo, GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC);
+
+ /* Build the test_fn. */
+ gcc_jit_function *test_fn =
+ gcc_jit_context_new_function (ctxt, NULL,
+ GCC_JIT_FUNCTION_EXPORTED,
+ int_type,
+ "test_using_tls",
+ 0, NULL,
+ 0);
+ gcc_jit_block *block = gcc_jit_function_new_block (test_fn, NULL);
+ gcc_jit_block_add_assignment (
+ block, NULL,
+ foo,
+ gcc_jit_context_new_rvalue_from_int (ctxt, int_type, 42));
+ gcc_jit_block_end_with_return (block,
+ NULL,
+ gcc_jit_lvalue_as_rvalue (foo));
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ typedef int (*fn_type) (void);
+ CHECK_NON_NULL (result);
+
+ fn_type test_using_tls =
+ (fn_type)gcc_jit_result_get_code (result, "test_using_tls");
+ CHECK_NON_NULL (test_using_tls);
+ int return_value = test_using_tls();
+ CHECK_VALUE (return_value, 42);
+
+ int *foo = (int *)gcc_jit_result_get_global (result, "foo");
+ CHECK_NON_NULL (foo);
+ CHECK_VALUE (*foo, 42);
+}
--
2.26.2.7.g19db9cfb68.dirty
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libgccjit: Add support for TLS variable [PR95415]
2021-11-20 22:34 ` Antoni Boucher
@ 2021-11-21 20:28 ` David Malcolm
0 siblings, 0 replies; 4+ messages in thread
From: David Malcolm @ 2021-11-21 20:28 UTC (permalink / raw)
To: Antoni Boucher, jit, gcc-patches
On Sat, 2021-11-20 at 17:34 -0500, Antoni Boucher wrote:
> Hi.
> Here's the updated patch.
> See comments below.
> Thanks for your reviews!
>
> Le jeudi 20 mai 2021 à 16:11 -0400, David Malcolm a écrit :
> > On Tue, 2021-05-18 at 20:43 -0400, Antoni Boucher via Gcc-patches
> > wrote:
> > > Hello.
> > > This patch adds support for TLS variables.
> > > One thing to fix before we merge it is the libgccjit.map file
> > > which
> > > contains LIBGCCJIT_ABI_16 instead of LIBGCCJIT_ABI_17.
> > > LIBGCCJIT_ABI_16 was added in one of my other patches.
> > > Thanks for the review.
> >
> > > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > > b/gcc/jit/docs/topics/compatibility.rst
> > > index 239b6aa1a92..d10bc1df080 100644
> > > --- a/gcc/jit/docs/topics/compatibility.rst
> > > +++ b/gcc/jit/docs/topics/compatibility.rst
> > > @@ -243,3 +243,12 @@ embedding assembler instructions:
> > > * :func:`gcc_jit_extended_asm_add_input_operand`
> > > * :func:`gcc_jit_extended_asm_add_clobber`
> > > * :func:`gcc_jit_context_add_top_level_asm`
> > > +
> > > +.. _LIBGCCJIT_ABI_17:
> > > +
> > > +``LIBGCCJIT_ABI_17``
> > > +-----------------------
> > > +``LIBGCCJIT_ABI_17`` covers the addition of an API entrypoint to
> > > set
> > > the
> > > +thread-local storage model of a variable:
> > > +
> > > + * :func:`gcc_jit_lvalue_set_tls_model`
> >
> > Sorry about the delay in reviewing patches.
> >
> > Is there a summary somewhere of the various outstanding patches and
> > their associated ABI versions? Are there dependencies between the
> > patches?
>
> The list of patches is there:
> https://github.com/antoyo/libgccjit-patches but I don't keep them up-
> to-date.
> If that would help you, I could add a README to tell what is the new
> ABI version for each patch.
> I believe there might be some patches that depend on a previous one.
That's not needed; I think all I need to know is what the next patch
you need me to look at is (FWIW I'm about to go on vacation for a week)
[...snip...]
>
> >
> > > +
> > > +void
> > > +create_code (gcc_jit_context *ctxt, void *user_data)
> > > +{
> > > + /* Let's try to inject the equivalent of:
> > > +
> > > + _Thread_local int foo;
> > > + */
> > > + gcc_jit_type *int_type =
> > > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
> > > +
> > > + gcc_jit_lvalue *foo =
> > > + gcc_jit_context_new_global (
> > > + ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo");
> > > + gcc_jit_lvalue_set_tls_model (foo,
> > > GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC);
> >
> > How many of the different enum values can be supported? How
> > target-
> > dependent is this?
>
> I'm not sure what you mean here. Are you asking that I test all the
> different enum values?
That would be ideal, but I don't think it's necessary.
> The tls_model enum is defined in gcc/coretypes.h and does not seem to
> change depending on the target. Maybe there are checks elsewhere for
> that, though.
It might be that some targets only support some modes; I don't know.
[...snip...]
Thanks for the updated patch. It looks good to push to trunk once the
earlier ones are in place, though as usual please re-test it before
pushing.
Dave
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-21 20:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 0:43 [PATCH] libgccjit: Add support for TLS variable [PR95415] Antoni Boucher
2021-05-20 20:11 ` David Malcolm
2021-11-20 22:34 ` Antoni Boucher
2021-11-21 20:28 ` 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).