[C23] Use thread_local semantics#70107
Merged
AaronBallman merged 4 commits intollvm:mainfrom Oct 25, 2023
Merged
Conversation
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 llvm#70068
Member
|
@llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) ChangesWhen 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 #70068 Full diff: https://github.com/llvm/llvm-project/pull/70107.diff 4 Files Affected:
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;
+}
+
|
cor3ntin
reviewed
Oct 24, 2023
Contributor
cor3ntin
left a comment
There was a problem hiding this comment.
LGTM but I did not review the codegen tests.
| // 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}} |
Contributor
There was a problem hiding this comment.
Another solution would be to say 'thread local variables' which is spelling agnostic.
erichkeane
reviewed
Oct 24, 2023
erichkeane
reviewed
Oct 24, 2023
erichkeane
approved these changes
Oct 24, 2023
Fznamznon
reviewed
Oct 25, 2023
AaronBallman
added a commit
to AaronBallman/llvm-project
that referenced
this pull request
Oct 25, 2023
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 llvm#70068 Fixes llvm#69167
tru
pushed a commit
that referenced
this pull request
Oct 27, 2023
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 #70068 Fixes #69167
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 #70068