c++/modules: Support thread_local statics in header modules [PR113292]
Checks
Commit Message
Bootstrapped and regtested on x86_64-pc-linux-gnu. OK for trunk?
-- >8 --
Currently, thread_locals in header modules cause ICEs. This patch makes
the required changes for them to work successfully.
Functions exported by a module need DECL_CONTEXT to be set, so we
inherit it from the variable we're wrapping.
We additionally require writing the DECL_TLS_MODEL for thread-local
variables to the module interface, and the TLS wrapper function needs to
have its DECL_BEFRIENDING_CLASSES written too as this is used to
retrieve what VAR_DECL it's a wrapper for when emitting a definition at
end of TU processing.
PR c++/113292
gcc/cp/ChangeLog:
* decl2.cc (get_tls_wrapper_fn): Set DECL_CONTEXT.
(c_parse_final_cleanups): Suppress warning for no definition of
TLS wrapper functions in header modules.
* module.cc (trees_out::lang_decl_vals): Write wrapped variable
for TLS wrapper functions.
(trees_in::lang_decl_vals): Read it.
(trees_out::decl_value): Write TLS model for thread-local vars.
(trees_in::decl_value): Read it for new decls. Remember to emit
definitions of TLS wrapper functions later.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr113292_a.H: New test.
* g++.dg/modules/pr113292_b.C: New test.
* g++.dg/modules/pr113292_c.C: New test.
Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
gcc/cp/decl2.cc | 10 ++++---
gcc/cp/module.cc | 22 +++++++++++++++
gcc/testsuite/g++.dg/modules/pr113292_a.H | 34 +++++++++++++++++++++++
gcc/testsuite/g++.dg/modules/pr113292_b.C | 13 +++++++++
gcc/testsuite/g++.dg/modules/pr113292_c.C | 11 ++++++++
5 files changed, 86 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_c.C
Comments
On 1/11/24 01:12, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu. OK for trunk?
>
> -- >8 --
>
> Currently, thread_locals in header modules cause ICEs. This patch makes
> the required changes for them to work successfully.
>
> Functions exported by a module need DECL_CONTEXT to be set, so we
> inherit it from the variable we're wrapping.
>
> We additionally require writing the DECL_TLS_MODEL for thread-local
> variables to the module interface, and the TLS wrapper function needs to
> have its DECL_BEFRIENDING_CLASSES written too as this is used to
> retrieve what VAR_DECL it's a wrapper for when emitting a definition at
> end of TU processing.
>
> PR c++/113292
>
> gcc/cp/ChangeLog:
> * decl2.cc (get_tls_wrapper_fn): Set DECL_CONTEXT.
> (c_parse_final_cleanups): Suppress warning for no definition of
> TLS wrapper functions in header modules.
> * module.cc (trees_out::lang_decl_vals): Write wrapped variable
> for TLS wrapper functions.
> (trees_in::lang_decl_vals): Read it.
> (trees_out::decl_value): Write TLS model for thread-local vars.
> (trees_in::decl_value): Read it for new decls. Remember to emit
> definitions of TLS wrapper functions later.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/pr113292_a.H: New test.
> * g++.dg/modules/pr113292_b.C: New test.
> * g++.dg/modules/pr113292_c.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
> gcc/cp/decl2.cc | 10 ++++---
> gcc/cp/module.cc | 22 +++++++++++++++
> gcc/testsuite/g++.dg/modules/pr113292_a.H | 34 +++++++++++++++++++++++
> gcc/testsuite/g++.dg/modules/pr113292_b.C | 13 +++++++++
> gcc/testsuite/g++.dg/modules/pr113292_c.C | 11 ++++++++
> 5 files changed, 86 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_b.C
> create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_c.C
>
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index fb996561f1b..ab348f8ecb7 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -3860,6 +3860,7 @@ get_tls_wrapper_fn (tree var)
> TREE_PUBLIC (fn) = TREE_PUBLIC (var);
> DECL_ARTIFICIAL (fn) = true;
> DECL_IGNORED_P (fn) = 1;
> + DECL_CONTEXT (fn) = DECL_CONTEXT (var);
> /* The wrapper is inline and emitted everywhere var is used. */
> DECL_DECLARED_INLINE_P (fn) = true;
> if (TREE_PUBLIC (var))
> @@ -5289,10 +5290,11 @@ c_parse_final_cleanups (void)
> #pragma interface, etc.) we decided not to emit the
> definition here. */
> && !DECL_INITIAL (decl)
> - /* A defaulted fn in a header module can be synthesized on
> - demand later. (In non-header modules we should have
> - synthesized it above.) */
> - && !(DECL_DEFAULTED_FN (decl) && header_module_p ())
> + /* A defaulted fn or TLS wrapper in a header module can be
> + synthesized on demand later. (In non-header modules we
> + should have synthesized it above.) */
> + && !(header_module_p ()
Hmm, should this be !module_attach_p instead of header_module_p?
The patch is OK, that can change separately if appropriate.
Jason
On Mon, Jan 15, 2024 at 05:38:25PM -0500, Jason Merrill wrote:
> On 1/11/24 01:12, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu. OK for trunk?
> >
> > -- >8 --
> >
> > Currently, thread_locals in header modules cause ICEs. This patch makes
> > the required changes for them to work successfully.
> >
> > Functions exported by a module need DECL_CONTEXT to be set, so we
> > inherit it from the variable we're wrapping.
> >
> > We additionally require writing the DECL_TLS_MODEL for thread-local
> > variables to the module interface, and the TLS wrapper function needs to
> > have its DECL_BEFRIENDING_CLASSES written too as this is used to
> > retrieve what VAR_DECL it's a wrapper for when emitting a definition at
> > end of TU processing.
> >
> > PR c++/113292
> >
> > gcc/cp/ChangeLog:
> > * decl2.cc (get_tls_wrapper_fn): Set DECL_CONTEXT.
> > (c_parse_final_cleanups): Suppress warning for no definition of
> > TLS wrapper functions in header modules.
> > * module.cc (trees_out::lang_decl_vals): Write wrapped variable
> > for TLS wrapper functions.
> > (trees_in::lang_decl_vals): Read it.
> > (trees_out::decl_value): Write TLS model for thread-local vars.
> > (trees_in::decl_value): Read it for new decls. Remember to emit
> > definitions of TLS wrapper functions later.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/pr113292_a.H: New test.
> > * g++.dg/modules/pr113292_b.C: New test.
> > * g++.dg/modules/pr113292_c.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> > gcc/cp/decl2.cc | 10 ++++---
> > gcc/cp/module.cc | 22 +++++++++++++++
> > gcc/testsuite/g++.dg/modules/pr113292_a.H | 34 +++++++++++++++++++++++
> > gcc/testsuite/g++.dg/modules/pr113292_b.C | 13 +++++++++
> > gcc/testsuite/g++.dg/modules/pr113292_c.C | 11 ++++++++
> > 5 files changed, 86 insertions(+), 4 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_a.H
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_b.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr113292_c.C
> >
> > diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> > index fb996561f1b..ab348f8ecb7 100644
> > --- a/gcc/cp/decl2.cc
> > +++ b/gcc/cp/decl2.cc
> > @@ -3860,6 +3860,7 @@ get_tls_wrapper_fn (tree var)
> > TREE_PUBLIC (fn) = TREE_PUBLIC (var);
> > DECL_ARTIFICIAL (fn) = true;
> > DECL_IGNORED_P (fn) = 1;
> > + DECL_CONTEXT (fn) = DECL_CONTEXT (var);
> > /* The wrapper is inline and emitted everywhere var is used. */
> > DECL_DECLARED_INLINE_P (fn) = true;
> > if (TREE_PUBLIC (var))
> > @@ -5289,10 +5290,11 @@ c_parse_final_cleanups (void)
> > #pragma interface, etc.) we decided not to emit the
> > definition here. */
> > && !DECL_INITIAL (decl)
> > - /* A defaulted fn in a header module can be synthesized on
> > - demand later. (In non-header modules we should have
> > - synthesized it above.) */
> > - && !(DECL_DEFAULTED_FN (decl) && header_module_p ())
> > + /* A defaulted fn or TLS wrapper in a header module can be
> > + synthesized on demand later. (In non-header modules we
> > + should have synthesized it above.) */
> > + && !(header_module_p ()
>
> Hmm, should this be !module_attach_p instead of header_module_p?
>
> The patch is OK, that can change separately if appropriate.
>
> Jason
>
Thanks. I used 'header_module_p' because that matches this condition in
decl2.cc:5117:
if (header_module_p ())
/* A header modules initializations are handled in its
importer. */
continue;
Which makes sense to me, I think: a header unit doesn't have an object
file to emit initialisations into, and it must instead be handled in its
importer, whereas other modules should be able to emit such
initialisations into their own TU.
@@ -3860,6 +3860,7 @@ get_tls_wrapper_fn (tree var)
TREE_PUBLIC (fn) = TREE_PUBLIC (var);
DECL_ARTIFICIAL (fn) = true;
DECL_IGNORED_P (fn) = 1;
+ DECL_CONTEXT (fn) = DECL_CONTEXT (var);
/* The wrapper is inline and emitted everywhere var is used. */
DECL_DECLARED_INLINE_P (fn) = true;
if (TREE_PUBLIC (var))
@@ -5289,10 +5290,11 @@ c_parse_final_cleanups (void)
#pragma interface, etc.) we decided not to emit the
definition here. */
&& !DECL_INITIAL (decl)
- /* A defaulted fn in a header module can be synthesized on
- demand later. (In non-header modules we should have
- synthesized it above.) */
- && !(DECL_DEFAULTED_FN (decl) && header_module_p ())
+ /* A defaulted fn or TLS wrapper in a header module can be
+ synthesized on demand later. (In non-header modules we
+ should have synthesized it above.) */
+ && !(header_module_p ()
+ && (DECL_DEFAULTED_FN (decl) || decl_tls_wrapper_p (decl)))
/* Don't complain if the template was defined. */
&& !(DECL_TEMPLATE_INSTANTIATION (decl)
&& DECL_INITIAL (DECL_TEMPLATE_RESULT
@@ -6948,6 +6948,9 @@ trees_out::lang_decl_vals (tree t)
if (streaming_p ())
wi (lang->u.fn.u5.fixed_offset);
}
+ else if (decl_tls_wrapper_p (t))
+ /* The wrapped variable. */
+ WT (lang->u.fn.befriending_classes);
else
WT (lang->u.fn.u5.cloned_function);
@@ -7027,6 +7030,8 @@ trees_in::lang_decl_vals (tree t)
RT (lang->u.fn.befriending_classes);
lang->u.fn.u5.fixed_offset = wi ();
}
+ else if (decl_tls_wrapper_p (t))
+ RT (lang->u.fn.befriending_classes);
else
RT (lang->u.fn.u5.cloned_function);
@@ -7926,6 +7931,9 @@ trees_out::decl_value (tree decl, depset *dep)
decl, cloned_p ? "" : "not ");
}
+ if (streaming_p () && VAR_P (decl) && CP_DECL_THREAD_LOCAL_P (decl))
+ u (decl_tls_model (decl));
+
if (streaming_p ())
dump (dumper::TREE) && dump ("Written decl:%d %C:%N", tag,
TREE_CODE (decl), decl);
@@ -8273,6 +8281,13 @@ trees_in::decl_value ()
look like templates. */
if (!install_implicit_member (inner))
set_overrun ();
+
+ /* When importing a TLS wrapper from a header unit, we haven't
+ actually emitted its definition yet. Remember it so we can
+ do this later. */
+ if (state->is_header ()
+ && decl_tls_wrapper_p (decl))
+ note_vague_linkage_fn (decl);
}
else
{
@@ -8356,6 +8371,13 @@ trees_in::decl_value ()
}
}
+ if (VAR_P (decl) && CP_DECL_THREAD_LOCAL_P (decl))
+ {
+ enum tls_model model = tls_model (u ());
+ if (is_new)
+ set_decl_tls_model (decl, model);
+ }
+
if (!NAMESPACE_SCOPE_P (inner)
&& ((TREE_CODE (inner) == TYPE_DECL
&& !is_typedef
new file mode 100644
@@ -0,0 +1,34 @@
+// PR c++/113292
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+struct test {
+ static const test& get_instance() {
+ return instance;
+ }
+ static thread_local test instance;
+};
+
+
+template <typename T>
+struct test_template {
+ static const test_template& get_instance() {
+ return instance;
+ }
+ static thread_local test_template instance;
+
+ template <typename U>
+ static const test_template& get_template_instance() {
+ return template_instance<U>;
+ }
+
+ template <typename U>
+ static thread_local test_template template_instance;
+};
+
+template <typename T>
+thread_local test_template<T> test_template<T>::instance;
+
+template <typename T>
+template <typename U>
+thread_local test_template<T> test_template<T>::template_instance;
new file mode 100644
@@ -0,0 +1,13 @@
+// PR c++/113292
+// { dg-additional-options "-fmodules-ts" }
+
+import "pr113292_a.H";
+
+// provide a definition of 'instance' so things link
+thread_local test test::instance;
+
+void instantiate() {
+ auto& instance = test::get_instance();
+ auto& t_instance = test_template<int>::get_instance();
+ auto& tt_instance = test_template<int>::get_template_instance<double>();
+};
new file mode 100644
@@ -0,0 +1,11 @@
+// PR c++/113292
+// { dg-module-do link }
+// { dg-additional-options "-fmodules-ts" }
+
+import "pr113292_a.H";
+
+int main() {
+ auto& instance = test::get_instance();
+ auto& t_instance = test_template<int>::get_instance();
+ auto& tt_instance = test_template<int>::get_template_instance<double>();
+}