[clang] [C23] Use thread_local semantics (PR #70107)
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 25 04:41:48 PDT 2023
https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/70107
>From 9db10fc83ec3683b1d5b6f8606fc37a83fe224fa Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 24 Oct 2023 15:15:15 -0400
Subject: [PATCH 1/4] [C23] Use thread_local semantics
When implementing thread_local as a keyword in C23, we accidentally
started using C++11 thread_local semantics when using that keyword
instead of using C11 _Thread_local semantics.
This oversight is fixed by pretending the user wrote _Thread_local
instead. This doesn't have the best behavior in terms of diagnostics,
but it does correct the semantic behavior.
Fixes https://github.com/llvm/llvm-project/issues/70068
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/lib/Parse/ParseDecl.cpp | 11 +++++++++--
clang/test/CodeGen/thread_local.c | 27 +++++++++++++++++++++++++++
clang/test/Sema/thread_local.c | 16 ++++++++++++++++
4 files changed, 55 insertions(+), 2 deletions(-)
create mode 100644 clang/test/CodeGen/thread_local.c
create mode 100644 clang/test/Sema/thread_local.c
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cf3f0c343b4d014..b104c5da97ab124 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -493,6 +493,9 @@ Bug Fixes in This Version
Fixes (`#65143 <https://github.com/llvm/llvm-project/issues/65143>`_)
- Fix crash in formatting the real/imaginary part of a complex lvalue.
Fixes (`#69218 <https://github.com/llvm/llvm-project/issues/69218>`_)
+- No longer use C++ ``thread_local`` semantics in C23 when using
+ ``thread_local`` instead of ``_Thread_local``.
+ Fixes (`#70068 <https://github.com/llvm/llvm-project/issues/70068>`_)
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 14a28e5a31c57db..43c18090d6ef79c 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4076,8 +4076,15 @@ void Parser::ParseDeclarationSpecifiers(
case tok::kw_thread_local:
if (getLangOpts().C23)
Diag(Tok, diag::warn_c23_compat_keyword) << Tok.getName();
- isInvalid = DS.SetStorageClassSpecThread(DeclSpec::TSCS_thread_local, Loc,
- PrevSpec, DiagID);
+ // We map thread_local to _Thread_local in C23 mode so it retains the C
+ // semantics rather than getting the C++ semantics.
+ // FIXME: diagnostics will now show _Thread_local when the user wrote
+ // thread_local in source in C23 mode; we need some general way to
+ // identify which way the user spelled the keyword in source.
+ isInvalid = DS.SetStorageClassSpecThread(
+ getLangOpts().C23 ? DeclSpec::TSCS__Thread_local
+ : DeclSpec::TSCS_thread_local,
+ Loc, PrevSpec, DiagID);
isStorageClass = true;
break;
case tok::kw__Thread_local:
diff --git a/clang/test/CodeGen/thread_local.c b/clang/test/CodeGen/thread_local.c
new file mode 100644
index 000000000000000..2daa43492cbf271
--- /dev/null
+++ b/clang/test/CodeGen/thread_local.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -std=c23 -emit-llvm -o - %s | FileCheck %s
+
+// Ensure that thread_local and _Thread_local emit the same codegen. See
+// https://github.com/llvm/llvm-project/issues/70068 for details.
+
+void func(void) {
+ static thread_local int i = 12;
+ static _Thread_local int j = 13;
+
+ extern thread_local int k;
+ extern thread_local int l;
+
+ (void)k;
+ (void)l;
+}
+
+// CHECK: @func.i = internal thread_local global i32 12, align 4
+// CHECK-NEXT: @func.j = internal thread_local global i32 13, align 4
+// CHECK-NEXT: @k = external thread_local global i32, align 4
+// CHECK-NEXT: @l = external thread_local global i32, align 4
+
+// CHECK: define dso_local void @func()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: %0 = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @k)
+// CHECK-NEXT: %1 = load i32, ptr %0, align 4
+// CHECK-NEXT: %2 = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @l)
+// CHECK-NEXT: %3 = load i32, ptr %2, align 4
diff --git a/clang/test/Sema/thread_local.c b/clang/test/Sema/thread_local.c
new file mode 100644
index 000000000000000..c4a4dd4ceb1cd0d
--- /dev/null
+++ b/clang/test/Sema/thread_local.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c23 %s -verify
+
+// Ensure that thread_local and _Thread_local are synonyms in C23 and both
+// restrict local variables to be explicitly static or extern.
+void func(void) {
+ // FIXME: it would be nice if the diagnostic said 'thread_local' in this case.
+ thread_local int i = 12; // expected-error {{'_Thread_local' variables must have global storage}}
+ _Thread_local int j = 13; // expected-error {{'_Thread_local' variables must have global storage}}
+
+ static thread_local int k = 14;
+ static _Thread_local int l = 15;
+
+ extern thread_local int m;
+ extern thread_local int n;
+}
+
>From ce80fb2f8b3a43308aad7b02d128dd0c5cdf21ec Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 24 Oct 2023 15:49:10 -0400
Subject: [PATCH 2/4] Address review feedback
---
clang/lib/Parse/ParseDecl.cpp | 2 +-
clang/test/CodeGen/thread_local.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 43c18090d6ef79c..78c3ab72979a007 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4078,7 +4078,7 @@ void Parser::ParseDeclarationSpecifiers(
Diag(Tok, diag::warn_c23_compat_keyword) << Tok.getName();
// We map thread_local to _Thread_local in C23 mode so it retains the C
// semantics rather than getting the C++ semantics.
- // FIXME: diagnostics will now show _Thread_local when the user wrote
+ // FIXME: diagnostics will show _Thread_local when the user wrote
// thread_local in source in C23 mode; we need some general way to
// identify which way the user spelled the keyword in source.
isInvalid = DS.SetStorageClassSpecThread(
diff --git a/clang/test/CodeGen/thread_local.c b/clang/test/CodeGen/thread_local.c
index 2daa43492cbf271..e0e9cd29053d8f2 100644
--- a/clang/test/CodeGen/thread_local.c
+++ b/clang/test/CodeGen/thread_local.c
@@ -21,7 +21,7 @@ void func(void) {
// CHECK: define dso_local void @func()
// CHECK-NEXT: entry:
-// CHECK-NEXT: %0 = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @k)
-// CHECK-NEXT: %1 = load i32, ptr %0, align 4
-// CHECK-NEXT: %2 = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @l)
-// CHECK-NEXT: %3 = load i32, ptr %2, align 4
+// CHECK-NEXT: %[[K:.+]] = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @k)
+// CHECK-NEXT: %{{.+}} = load i32, ptr %[[K]], align 4
+// CHECK-NEXT: %[[L:.+]] = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @l)
+// CHECK-NEXT: %{{.+}} = load i32, ptr %[[L]], align 4
>From 704f3819d57a9cf6bcc745676174264ca7b5e041 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 24 Oct 2023 15:54:21 -0400
Subject: [PATCH 3/4] Addressing more review comments
---
clang/test/CodeGen/thread_local.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/CodeGen/thread_local.c b/clang/test/CodeGen/thread_local.c
index e0e9cd29053d8f2..b97f31c2fff2f6c 100644
--- a/clang/test/CodeGen/thread_local.c
+++ b/clang/test/CodeGen/thread_local.c
@@ -22,6 +22,6 @@ void func(void) {
// CHECK: define dso_local void @func()
// CHECK-NEXT: entry:
// CHECK-NEXT: %[[K:.+]] = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @k)
-// CHECK-NEXT: %{{.+}} = load i32, ptr %[[K]], align 4
+// CHECK-NEXT: load i32, ptr %[[K]], align 4
// CHECK-NEXT: %[[L:.+]] = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @l)
-// CHECK-NEXT: %{{.+}} = load i32, ptr %[[L]], align 4
+// CHECK-NEXT: load i32, ptr %[[L]], align 4
>From 18b4d888efb43fb52d98691a7bdb6af2d5238e7d Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Wed, 25 Oct 2023 07:41:06 -0400
Subject: [PATCH 4/4] Add a test case + release note for #69167, which this
also resolves
---
clang/docs/ReleaseNotes.rst | 3 ++-
clang/test/Sema/thread_local.c | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b104c5da97ab124..a4a9dc40b473424 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -495,7 +495,8 @@ Bug Fixes in This Version
Fixes (`#69218 <https://github.com/llvm/llvm-project/issues/69218>`_)
- No longer use C++ ``thread_local`` semantics in C23 when using
``thread_local`` instead of ``_Thread_local``.
- Fixes (`#70068 <https://github.com/llvm/llvm-project/issues/70068>`_)
+ Fixes (`#70068 <https://github.com/llvm/llvm-project/issues/70068>`_) and
+ (`#69167 <https://github.com/llvm/llvm-project/issues/69167>`_)
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/test/Sema/thread_local.c b/clang/test/Sema/thread_local.c
index c4a4dd4ceb1cd0d..a0de0aa4e39a6e1 100644
--- a/clang/test/Sema/thread_local.c
+++ b/clang/test/Sema/thread_local.c
@@ -14,3 +14,6 @@ void func(void) {
extern thread_local int n;
}
+// This would previously fail because the tls models were different.
+extern thread_local unsigned a;
+_Thread_local unsigned a = 0;
More information about the cfe-commits
mailing list