[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