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 <aa...@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 <aa...@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 <aa...@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 <aa...@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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
