On Fri, May 23, 2025 at 10:23:51AM -0400, Jason Merrill wrote:
> On 5/22/25 8:22 AM, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
> >
> > (Also is renaming the old test OK/appropriate? Or should I keep it
> > before and just name the new tests as tls1/2, with a comment referring
> > to pr113292?)
> >
> > -- >8 --
> >
> > The PR notes that we missed setting DECL_CONTEXT on the TLS init
> > function; we missed this initially because this function is not created
> > in header units, only named modules.
> >
> > I also noticed that 'DECL_CONTEXT (fn) = DECL_CONTEXT (var)' was
> > incorrect: for class members, this ends up having the modules merging
> > machinery treat the decl as a member function, which breaks when
> > attempting to dedup against an existing completed class type. Instead
> > we can just use the global_namespace as the context, because the name of
> > the function is already mangled appropriately so that we'll match the
> > correct duplicates.
> >
> > PR c++/120363
> >
> > gcc/cp/ChangeLog:
> >
> > * decl2.cc (get_tls_init_fn): Set context as global_namespace.
> > (get_tls_wrapper_fn): Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/pr113292_a.H: Move to...
> > * g++.dg/modules/tls-1_a.H: ...here.
> > * g++.dg/modules/pr113292_b.C: Move to...
> > * g++.dg/modules/tls-1_b.C: ...here.
> > * g++.dg/modules/pr113292_c.C: Move to...
> > * g++.dg/modules/tls-1_c.C: ...here.
> > * g++.dg/modules/tls-2_a.C: New test.
> > * g++.dg/modules/tls-2_b.C: New test.
> > * g++.dg/modules/tls-2_c.C: New test.
> > * g++.dg/modules/tls-3.h: New test.
> > * g++.dg/modules/tls-3_a.H: New test.
> > * g++.dg/modules/tls-3_b.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > ---
> > gcc/cp/decl2.cc | 3 +-
> > .../modules/{pr113292_a.H => tls-1_a.H} | 0
> > .../modules/{pr113292_b.C => tls-1_b.C} | 2 +-
> > .../modules/{pr113292_c.C => tls-1_c.C} | 2 +-
> > gcc/testsuite/g++.dg/modules/tls-2_a.C | 12 ++++++
> > gcc/testsuite/g++.dg/modules/tls-2_b.C | 5 +++
> > gcc/testsuite/g++.dg/modules/tls-2_c.C | 11 +++++
> > gcc/testsuite/g++.dg/modules/tls-3.h | 42 +++++++++++++++++++
> > gcc/testsuite/g++.dg/modules/tls-3_a.H | 4 ++
> > gcc/testsuite/g++.dg/modules/tls-3_b.C | 4 ++
> > 10 files changed, 82 insertions(+), 3 deletions(-)
> > rename gcc/testsuite/g++.dg/modules/{pr113292_a.H => tls-1_a.H} (100%)
> > rename gcc/testsuite/g++.dg/modules/{pr113292_b.C => tls-1_b.C} (93%)
> > rename gcc/testsuite/g++.dg/modules/{pr113292_c.C => tls-1_c.C} (93%)
> > create mode 100644 gcc/testsuite/g++.dg/modules/tls-2_a.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/tls-2_b.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/tls-2_c.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/tls-3.h
> > create mode 100644 gcc/testsuite/g++.dg/modules/tls-3_a.H
> > create mode 100644 gcc/testsuite/g++.dg/modules/tls-3_b.C
> >
> > diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> > index a08d173c0df..be82ccd8bc1 100644
> > --- a/gcc/cp/decl2.cc
> > +++ b/gcc/cp/decl2.cc
> > @@ -4028,6 +4028,7 @@ get_tls_init_fn (tree var)
> > SET_DECL_LANGUAGE (fn, lang_c);
> > TREE_PUBLIC (fn) = TREE_PUBLIC (var);
> > DECL_ARTIFICIAL (fn) = true;
> > + DECL_CONTEXT (fn) = global_namespace;
>
> Generally we do FROB_CONTEXT (global_namespace) so we never actually see
> global_namespace in DECL_CONTEXT. I'm not sure that's necessary in this
> particular case, but best to be consistent.
I see, thanks. Updated below, and fixed thinko in commit title; testing
modules.exp passed successfully on x86_64-pc-linux-gnu.
OK for trunk/15 if full bootstrap+regtest succeeds?
-- >8 --
Subject: [PATCH] c++/modules: Fix merge of TLS init functions [PR120363]
The PR notes that we missed setting DECL_CONTEXT on the TLS init
function; we missed this initially because this function is not created
in header units, only named modules.
I also noticed that 'DECL_CONTEXT (fn) = DECL_CONTEXT (var)' was
incorrect: for class members, this ends up having the modules merging
machinery treat the decl as a member function, which breaks when
attempting to dedup against an existing completed class type. Instead
we can just use the global_namespace as the context, because the name of
the function is already mangled appropriately so that we'll match the
correct duplicates.
PR c++/120363
gcc/cp/ChangeLog:
* decl2.cc (get_tls_init_fn): Set context as global_namespace.
(get_tls_wrapper_fn): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr113292_a.H: Move to...
* g++.dg/modules/tls-1_a.H: ...here.
* g++.dg/modules/pr113292_b.C: Move to...
* g++.dg/modules/tls-1_b.C: ...here.
* g++.dg/modules/pr113292_c.C: Move to...
* g++.dg/modules/tls-1_c.C: ...here.
* g++.dg/modules/tls-2_a.C: New test.
* g++.dg/modules/tls-2_b.C: New test.
* g++.dg/modules/tls-2_c.C: New test.
* g++.dg/modules/tls-3.h: New test.
* g++.dg/modules/tls-3_a.H: New test.
* g++.dg/modules/tls-3_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/decl2.cc | 3 +-
.../modules/{pr113292_a.H => tls-1_a.H} | 0
.../modules/{pr113292_b.C => tls-1_b.C} | 2 +-
.../modules/{pr113292_c.C => tls-1_c.C} | 2 +-
gcc/testsuite/g++.dg/modules/tls-2_a.C | 12 ++++++
gcc/testsuite/g++.dg/modules/tls-2_b.C | 5 +++
gcc/testsuite/g++.dg/modules/tls-2_c.C | 11 +++++
gcc/testsuite/g++.dg/modules/tls-3.h | 42 +++++++++++++++++++
gcc/testsuite/g++.dg/modules/tls-3_a.H | 4 ++
gcc/testsuite/g++.dg/modules/tls-3_b.C | 4 ++
10 files changed, 82 insertions(+), 3 deletions(-)
rename gcc/testsuite/g++.dg/modules/{pr113292_a.H => tls-1_a.H} (100%)
rename gcc/testsuite/g++.dg/modules/{pr113292_b.C => tls-1_b.C} (93%)
rename gcc/testsuite/g++.dg/modules/{pr113292_c.C => tls-1_c.C} (93%)
create mode 100644 gcc/testsuite/g++.dg/modules/tls-2_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/tls-2_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/tls-2_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/tls-3.h
create mode 100644 gcc/testsuite/g++.dg/modules/tls-3_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/tls-3_b.C
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index a08d173c0df..666ab784dee 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -4028,6 +4028,7 @@ get_tls_init_fn (tree var)
SET_DECL_LANGUAGE (fn, lang_c);
TREE_PUBLIC (fn) = TREE_PUBLIC (var);
DECL_ARTIFICIAL (fn) = true;
+ DECL_CONTEXT (fn) = FROB_CONTEXT (global_namespace);
DECL_COMDAT (fn) = DECL_COMDAT (var);
DECL_EXTERNAL (fn) = DECL_EXTERNAL (var);
if (DECL_ONE_ONLY (var))
@@ -4087,7 +4088,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);
+ DECL_CONTEXT (fn) = FROB_CONTEXT (global_namespace);
/* The wrapper is inline and emitted everywhere var is used. */
DECL_DECLARED_INLINE_P (fn) = true;
if (TREE_PUBLIC (var))
diff --git a/gcc/testsuite/g++.dg/modules/pr113292_a.H
b/gcc/testsuite/g++.dg/modules/tls-1_a.H
similarity index 100%
rename from gcc/testsuite/g++.dg/modules/pr113292_a.H
rename to gcc/testsuite/g++.dg/modules/tls-1_a.H
diff --git a/gcc/testsuite/g++.dg/modules/pr113292_b.C
b/gcc/testsuite/g++.dg/modules/tls-1_b.C
similarity index 93%
rename from gcc/testsuite/g++.dg/modules/pr113292_b.C
rename to gcc/testsuite/g++.dg/modules/tls-1_b.C
index fc582a5a0cf..941bff2710a 100644
--- a/gcc/testsuite/g++.dg/modules/pr113292_b.C
+++ b/gcc/testsuite/g++.dg/modules/tls-1_b.C
@@ -1,7 +1,7 @@
// PR c++/113292
// { dg-additional-options "-fmodules-ts" }
-import "pr113292_a.H";
+import "tls-1_a.H";
// provide a definition of 'instance' so things link
thread_local test test::instance;
diff --git a/gcc/testsuite/g++.dg/modules/pr113292_c.C
b/gcc/testsuite/g++.dg/modules/tls-1_c.C
similarity index 93%
rename from gcc/testsuite/g++.dg/modules/pr113292_c.C
rename to gcc/testsuite/g++.dg/modules/tls-1_c.C
index b5acf79db63..4568413bb22 100644
--- a/gcc/testsuite/g++.dg/modules/pr113292_c.C
+++ b/gcc/testsuite/g++.dg/modules/tls-1_c.C
@@ -4,7 +4,7 @@
// { dg-add-options tls }
// { dg-additional-options "-fmodules-ts" }
-import "pr113292_a.H";
+import "tls-1_a.H";
int main() {
auto& instance = test::get_instance();
diff --git a/gcc/testsuite/g++.dg/modules/tls-2_a.C
b/gcc/testsuite/g++.dg/modules/tls-2_a.C
new file mode 100644
index 00000000000..2efae6c00b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tls-2_a.C
@@ -0,0 +1,12 @@
+// PR c++/120363
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi M }
+
+export module M;
+
+export struct test {
+ static inline const int& get_instance() {
+ return instance;
+ }
+ static thread_local int instance;
+};
diff --git a/gcc/testsuite/g++.dg/modules/tls-2_b.C
b/gcc/testsuite/g++.dg/modules/tls-2_b.C
new file mode 100644
index 00000000000..af3e70949c5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tls-2_b.C
@@ -0,0 +1,5 @@
+// PR c++/120363
+// { dg-additional-options "-fmodules" }
+
+module M;
+thread_local int test::instance;
diff --git a/gcc/testsuite/g++.dg/modules/tls-2_c.C
b/gcc/testsuite/g++.dg/modules/tls-2_c.C
new file mode 100644
index 00000000000..4489516fa7b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tls-2_c.C
@@ -0,0 +1,11 @@
+// PR c++/120363
+// { dg-module-do link }
+// { dg-require-effective-target tls_runtime }
+// { dg-add-options tls }
+// { dg-additional-options "-fmodules" }
+
+import M;
+
+int main() {
+ auto& instance = test::get_instance();
+}
diff --git a/gcc/testsuite/g++.dg/modules/tls-3.h
b/gcc/testsuite/g++.dg/modules/tls-3.h
new file mode 100644
index 00000000000..6f4a236b7eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tls-3.h
@@ -0,0 +1,42 @@
+inline thread_local int tla;
+inline int& get_tla() {
+ return tla;
+}
+
+static thread_local int tlb;
+static int& get_tlb() {
+ return tlb;
+}
+
+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;
+
+template struct test_template<int>;
+template const test_template<int>&
test_template<int>::get_template_instance<int>();
diff --git a/gcc/testsuite/g++.dg/modules/tls-3_a.H
b/gcc/testsuite/g++.dg/modules/tls-3_a.H
new file mode 100644
index 00000000000..beef907128b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tls-3_a.H
@@ -0,0 +1,4 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+#include "tls-3.h"
diff --git a/gcc/testsuite/g++.dg/modules/tls-3_b.C
b/gcc/testsuite/g++.dg/modules/tls-3_b.C
new file mode 100644
index 00000000000..ab77b1ec077
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tls-3_b.C
@@ -0,0 +1,4 @@
+// { dg-additional-options "-fmodules -fno-module-lazy" }
+
+#include "tls-3.h"
+import "tls-3_a.H";
--
2.47.0
