[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/nikola-tesic-ns created
https://github.com/llvm/llvm-project/pull/96633
This patch fixes compiler generated code in `tls_init` function to access TLS
variable (`tls_guard`) via llvm.threadlocal.address intrinsic.
Related discussion:
https://discourse.llvm.org/t/address-thread-identification-problems-with-coroutine/62015/57?u=ntesic
>From 41427a3de345517025477257bfd4f614f06cbcfe Mon Sep 17 00:00:00 2001
From: Nikola Tesic
Date: Tue, 25 Jun 2024 15:58:18 +0300
Subject: [PATCH] [Clang] Access tls_guard via llvm.threadlocal.address
This patch fixes compiler generated code in `tls_init` function to access
TLS variable (`tls_guard`) via llvm.threadlocal.address intrinsic.
---
clang/lib/CodeGen/CGDeclCXX.cpp | 29 +++---
clang/test/CodeGenCXX/cxx11-thread-local.cpp | 9 --
.../static-initializer-branch-weights.cpp | 3 +-
clang/test/OpenMP/parallel_copyin_codegen.cpp | 6 ++--
.../OpenMP/target_has_device_addr_codegen.cpp | 6 ++--
clang/test/OpenMP/threadprivate_codegen.cpp | 30 ---
6 files changed, 55 insertions(+), 28 deletions(-)
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index e18b339b31d24..0663a083bf3e8 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -1059,9 +1059,10 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ llvm::Value *GuardVal = EmitLoadOfScalar(
+ MakeAddrLValue(Guard, getContext().IntTy), SourceLocation());
+ llvm::Value *Uninit =
+ Builder.CreateIsNull(GuardVal, "guard.uninitialized");
llvm::BasicBlock *InitBlock = createBasicBlock("init");
ExitBlock = createBasicBlock("exit");
EmitCXXGuardedInitBranch(Uninit, InitBlock, ExitBlock,
@@ -1070,13 +1071,21 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
- EmitInvariantStart(
- Guard.getPointer(),
- CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ EmitStoreOfScalar(llvm::ConstantInt::get(GuardVal->getType(), 1),
+MakeAddrLValue(Guard, getContext().IntTy));
+
+ // Emit invariant start for TLS guard address.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0) {
+uint64_t Width =
+CGM.getDataLayout().getTypeAllocSize(GuardVal->getType());
+llvm::Value *TLSAddr = Guard.getPointer();
+// Get the thread-local address via intrinsic.
+if (auto *GV = dyn_cast(Guard.getPointer()))
+ if (GV->isThreadLocal())
+TLSAddr = Builder.CreateThreadLocalAddress(Guard.getPointer());
+Builder.CreateInvariantStart(
+TLSAddr, llvm::ConstantInt::getSigned(Int64Ty, Width));
+ }
}
RunCleanupsScope Scope(*this);
diff --git a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
index bcc490bc32e6e..e9a0799cf8d9a 100644
--- a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -358,12 +358,15 @@ void set_anon_i() {
// CHECK: define {{.*}}@__tls_init()
-// CHECK: load i8, ptr @__tls_guard
+// CHECK: [[TLS_GUARD_ADDR_1:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK: load i8, ptr [[TLS_GUARD_ADDR_1]]
// CHECK: %[[NEED_TLS_INIT:.*]] = icmp eq i8 %{{.*}}, 0
// CHECK: br i1 %[[NEED_TLS_INIT]],
// init:
-// CHECK: store i8 1, ptr @__tls_guard
-// CHECK-OPT: call ptr @llvm.invariant.start.p0(i64 1, ptr @__tls_guard)
+// CHECK: [[TLS_GUARD_ADDR_2:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK: store i8 1, ptr [[TLS_GUARD_ADDR_2]]
+// CHECK-OPT: [[TLS_GUARD_ADDR_3:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK-OPT: call ptr @llvm.invariant.start.p0(i64 1, ptr
[[TLS_GUARD_ADDR_3]])
// CHECK-NOT: call void @[[V_M_INIT]]()
// CHECK: call void @[[A_INIT]]()
// CHECK-NOT: call void @[[V_M_INIT]]()
diff --git a/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
b/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
index 121b9b2029959..e855f54643eae 100644
--- a/clang/test/Code
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
llvmbot wrote:
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen
Author: None (nikola-tesic-ns)
Changes
This patch fixes compiler generated code in `tls_init` function to access TLS
variable (`tls_guard`) via llvm.threadlocal.address intrinsic.
Related discussion:
https://discourse.llvm.org/t/address-thread-identification-problems-with-coroutine/62015/57?u=ntesic
---
Full diff: https://github.com/llvm/llvm-project/pull/96633.diff
6 Files Affected:
- (modified) clang/lib/CodeGen/CGDeclCXX.cpp (+19-10)
- (modified) clang/test/CodeGenCXX/cxx11-thread-local.cpp (+6-3)
- (modified) clang/test/CodeGenCXX/static-initializer-branch-weights.cpp (+2-1)
- (modified) clang/test/OpenMP/parallel_copyin_codegen.cpp (+4-2)
- (modified) clang/test/OpenMP/target_has_device_addr_codegen.cpp (+4-2)
- (modified) clang/test/OpenMP/threadprivate_codegen.cpp (+20-10)
``diff
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index e18b339b31d24..0663a083bf3e8 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -1059,9 +1059,10 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ llvm::Value *GuardVal = EmitLoadOfScalar(
+ MakeAddrLValue(Guard, getContext().IntTy), SourceLocation());
+ llvm::Value *Uninit =
+ Builder.CreateIsNull(GuardVal, "guard.uninitialized");
llvm::BasicBlock *InitBlock = createBasicBlock("init");
ExitBlock = createBasicBlock("exit");
EmitCXXGuardedInitBranch(Uninit, InitBlock, ExitBlock,
@@ -1070,13 +1071,21 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
- EmitInvariantStart(
- Guard.getPointer(),
- CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ EmitStoreOfScalar(llvm::ConstantInt::get(GuardVal->getType(), 1),
+MakeAddrLValue(Guard, getContext().IntTy));
+
+ // Emit invariant start for TLS guard address.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0) {
+uint64_t Width =
+CGM.getDataLayout().getTypeAllocSize(GuardVal->getType());
+llvm::Value *TLSAddr = Guard.getPointer();
+// Get the thread-local address via intrinsic.
+if (auto *GV = dyn_cast(Guard.getPointer()))
+ if (GV->isThreadLocal())
+TLSAddr = Builder.CreateThreadLocalAddress(Guard.getPointer());
+Builder.CreateInvariantStart(
+TLSAddr, llvm::ConstantInt::getSigned(Int64Ty, Width));
+ }
}
RunCleanupsScope Scope(*this);
diff --git a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
index bcc490bc32e6e..e9a0799cf8d9a 100644
--- a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -358,12 +358,15 @@ void set_anon_i() {
// CHECK: define {{.*}}@__tls_init()
-// CHECK: load i8, ptr @__tls_guard
+// CHECK: [[TLS_GUARD_ADDR_1:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK: load i8, ptr [[TLS_GUARD_ADDR_1]]
// CHECK: %[[NEED_TLS_INIT:.*]] = icmp eq i8 %{{.*}}, 0
// CHECK: br i1 %[[NEED_TLS_INIT]],
// init:
-// CHECK: store i8 1, ptr @__tls_guard
-// CHECK-OPT: call ptr @llvm.invariant.start.p0(i64 1, ptr @__tls_guard)
+// CHECK: [[TLS_GUARD_ADDR_2:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK: store i8 1, ptr [[TLS_GUARD_ADDR_2]]
+// CHECK-OPT: [[TLS_GUARD_ADDR_3:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK-OPT: call ptr @llvm.invariant.start.p0(i64 1, ptr
[[TLS_GUARD_ADDR_3]])
// CHECK-NOT: call void @[[V_M_INIT]]()
// CHECK: call void @[[A_INIT]]()
// CHECK-NOT: call void @[[V_M_INIT]]()
diff --git a/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
b/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
index 121b9b2029959..e855f54643eae 100644
--- a/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
+++ b/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
@@ -118,7 +118,8 @@ void use_b() {
}
// CHECK-LABEL: define {{.*}}tls_init()
-// CHECK: load i8, ptr @__tls_g
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
nikola-tesic-ns wrote: @nikic @ChuanqiXu9 I couldn't add you to the reviewers list. Please, take a look at this fix. Thanks! https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,10 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ llvm::Value *GuardVal = EmitLoadOfScalar(
+ MakeAddrLValue(Guard, getContext().IntTy), SourceLocation());
ChuanqiXu9 wrote:
I am slightly not happy with the use of `MakeAddrLValue` since the term
`LValue` is a language concept in my mind and here are some codes for
implementing details for the language to me. I may feel better to implement
this with the `CreateThreadLocalAddress` API directly.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/nikola-tesic-ns updated
https://github.com/llvm/llvm-project/pull/96633
>From 41427a3de345517025477257bfd4f614f06cbcfe Mon Sep 17 00:00:00 2001
From: Nikola Tesic
Date: Tue, 25 Jun 2024 15:58:18 +0300
Subject: [PATCH 1/2] [Clang] Access tls_guard via llvm.threadlocal.address
This patch fixes compiler generated code in `tls_init` function to access
TLS variable (`tls_guard`) via llvm.threadlocal.address intrinsic.
---
clang/lib/CodeGen/CGDeclCXX.cpp | 29 +++---
clang/test/CodeGenCXX/cxx11-thread-local.cpp | 9 --
.../static-initializer-branch-weights.cpp | 3 +-
clang/test/OpenMP/parallel_copyin_codegen.cpp | 6 ++--
.../OpenMP/target_has_device_addr_codegen.cpp | 6 ++--
clang/test/OpenMP/threadprivate_codegen.cpp | 30 ---
6 files changed, 55 insertions(+), 28 deletions(-)
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index e18b339b31d24..0663a083bf3e8 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -1059,9 +1059,10 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ llvm::Value *GuardVal = EmitLoadOfScalar(
+ MakeAddrLValue(Guard, getContext().IntTy), SourceLocation());
+ llvm::Value *Uninit =
+ Builder.CreateIsNull(GuardVal, "guard.uninitialized");
llvm::BasicBlock *InitBlock = createBasicBlock("init");
ExitBlock = createBasicBlock("exit");
EmitCXXGuardedInitBranch(Uninit, InitBlock, ExitBlock,
@@ -1070,13 +1071,21 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
- EmitInvariantStart(
- Guard.getPointer(),
- CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ EmitStoreOfScalar(llvm::ConstantInt::get(GuardVal->getType(), 1),
+MakeAddrLValue(Guard, getContext().IntTy));
+
+ // Emit invariant start for TLS guard address.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0) {
+uint64_t Width =
+CGM.getDataLayout().getTypeAllocSize(GuardVal->getType());
+llvm::Value *TLSAddr = Guard.getPointer();
+// Get the thread-local address via intrinsic.
+if (auto *GV = dyn_cast(Guard.getPointer()))
+ if (GV->isThreadLocal())
+TLSAddr = Builder.CreateThreadLocalAddress(Guard.getPointer());
+Builder.CreateInvariantStart(
+TLSAddr, llvm::ConstantInt::getSigned(Int64Ty, Width));
+ }
}
RunCleanupsScope Scope(*this);
diff --git a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
index bcc490bc32e6e..e9a0799cf8d9a 100644
--- a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -358,12 +358,15 @@ void set_anon_i() {
// CHECK: define {{.*}}@__tls_init()
-// CHECK: load i8, ptr @__tls_guard
+// CHECK: [[TLS_GUARD_ADDR_1:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK: load i8, ptr [[TLS_GUARD_ADDR_1]]
// CHECK: %[[NEED_TLS_INIT:.*]] = icmp eq i8 %{{.*}}, 0
// CHECK: br i1 %[[NEED_TLS_INIT]],
// init:
-// CHECK: store i8 1, ptr @__tls_guard
-// CHECK-OPT: call ptr @llvm.invariant.start.p0(i64 1, ptr @__tls_guard)
+// CHECK: [[TLS_GUARD_ADDR_2:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK: store i8 1, ptr [[TLS_GUARD_ADDR_2]]
+// CHECK-OPT: [[TLS_GUARD_ADDR_3:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK-OPT: call ptr @llvm.invariant.start.p0(i64 1, ptr
[[TLS_GUARD_ADDR_3]])
// CHECK-NOT: call void @[[V_M_INIT]]()
// CHECK: call void @[[A_INIT]]()
// CHECK-NOT: call void @[[V_M_INIT]]()
diff --git a/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
b/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
index 121b9b2029959..e855f54643eae 100644
--- a/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
+++ b/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
@@ -118,7 +118,8 @@ void use_b() {
}
// CHECK-LABEL: define {{.*}}tls_init()
-// CHECK: load i8, ptr @__tls_guard, align 1
+// CHECK: [[TLS_GUARD_
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,10 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ llvm::Value *GuardVal = EmitLoadOfScalar(
+ MakeAddrLValue(Guard, getContext().IntTy), SourceLocation());
nikola-tesic-ns wrote:
Makes sense, thanks.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
ChuanqiXu9 wrote:
It looks like the change can be simpler if we don't introduce new the variable
`GuardAddr`, is it?
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1076,26 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn, // Mark as initialized before initializing anything else. If the // initializers use previously-initialized thread_local vars, that's // probably supposed to be OK, but the standard doesn't say. - Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); - - // The guard variable can't ever change again. - EmitInvariantStart( - Guard.getPointer(), - CharUnits::fromQuantity( - CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(; + if (auto *GV = dyn_cast(Guard.getPointer())) +// Get the thread-local address via intrinsic. +if (GV->isThreadLocal()) ChuanqiXu9 wrote: ditto https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
+// Get the thread-local address via intrinsic.
+if (GV->isThreadLocal())
+ GuardAddr = GuardAddr.withPointer(
+ Builder.CreateThreadLocalAddress(GV), NotKnownNonNull);
+ llvm::Value *GuardVal = Builder.CreateLoad(GuardAddr);
ChuanqiXu9 wrote:
These lines are not changed. So we'd better to not touch them.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1076,26 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
- EmitInvariantStart(
- Guard.getPointer(),
- CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
+// Get the thread-local address via intrinsic.
+if (GV->isThreadLocal())
+ GuardAddr = GuardAddr.withPointer(
+ Builder.CreateThreadLocalAddress(GV), NotKnownNonNull);
+ Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1),
+ GuardAddr);
+
+ // Emit invariant start for TLS guard address.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0) {
+uint64_t Width =
+CGM.getDataLayout().getTypeAllocSize(GuardVal->getType());
+llvm::Value *TLSAddr = Guard.getPointer();
+if (auto *GV = dyn_cast(Guard.getPointer()))
+ // Get the thread-local address via intrinsic.
+ if (GV->isThreadLocal())
+TLSAddr = Builder.CreateThreadLocalAddress(GV);
+Builder.CreateInvariantStart(
ChuanqiXu9 wrote:
Why do the ABI used change? And is it necessary? If not, can we remove it?
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
+// Get the thread-local address via intrinsic.
+if (GV->isThreadLocal())
ChuanqiXu9 wrote:
```suggestion
// Get the thread-local address via intrinsic.
if (auto *GV = dyn_cast(Guard.getPointer());
GV && GV->isThreadLocal())
```
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
rjmccall wrote:
We need to do this unconditionally, and we should be able to do because `Guard`
is a constant address. But if there's some code pattern where that's not true,
we need to fix that code pattern rather than silently just not using this
intrinsic.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
nikola-tesic-ns wrote:
The `Guard` is a `ConstantAddress`, so I cannot change it, that's why I
introduced new variable. If you have some suggestion, I would be happy to adapt
the code.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1076,26 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
- EmitInvariantStart(
- Guard.getPointer(),
- CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
+// Get the thread-local address via intrinsic.
+if (GV->isThreadLocal())
+ GuardAddr = GuardAddr.withPointer(
+ Builder.CreateThreadLocalAddress(GV), NotKnownNonNull);
+ Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1),
+ GuardAddr);
+
+ // Emit invariant start for TLS guard address.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0) {
+uint64_t Width =
+CGM.getDataLayout().getTypeAllocSize(GuardVal->getType());
+llvm::Value *TLSAddr = Guard.getPointer();
+if (auto *GV = dyn_cast(Guard.getPointer()))
+ // Get the thread-local address via intrinsic.
+ if (GV->isThreadLocal())
+TLSAddr = Builder.CreateThreadLocalAddress(GV);
+Builder.CreateInvariantStart(
nikola-tesic-ns wrote:
I am not sure I understood this, sorry.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
nikola-tesic-ns wrote:
There is a code pattern where this "guarded initialization" is done for non-TLS
var ([partitions.cpp
test](https://github.com/nextsilicon/next-llvm-project/blob/b36811cc9baf1c72de2fa1c8b5d8fc30bae9a15c/clang/test/CodeGenCXX/partitions.cpp)).
That's the reason I've introduced these checks.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
ChuanqiXu9 wrote:
OK, I didn't look into the context.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1076,26 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
- EmitInvariantStart(
- Guard.getPointer(),
- CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
+// Get the thread-local address via intrinsic.
+if (GV->isThreadLocal())
+ GuardAddr = GuardAddr.withPointer(
+ Builder.CreateThreadLocalAddress(GV), NotKnownNonNull);
+ Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1),
+ GuardAddr);
+
+ // Emit invariant start for TLS guard address.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0) {
+uint64_t Width =
+CGM.getDataLayout().getTypeAllocSize(GuardVal->getType());
+llvm::Value *TLSAddr = Guard.getPointer();
+if (auto *GV = dyn_cast(Guard.getPointer()))
+ // Get the thread-local address via intrinsic.
+ if (GV->isThreadLocal())
+TLSAddr = Builder.CreateThreadLocalAddress(GV);
+Builder.CreateInvariantStart(
ChuanqiXu9 wrote:
I mean, it used `EmitInvariantStart` but now it uses `CreateInvariantStart`.
(Sorry, I meant to use API)
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1076,26 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
- EmitInvariantStart(
- Guard.getPointer(),
- CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
+// Get the thread-local address via intrinsic.
+if (GV->isThreadLocal())
+ GuardAddr = GuardAddr.withPointer(
+ Builder.CreateThreadLocalAddress(GV), NotKnownNonNull);
+ Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1),
+ GuardAddr);
+
+ // Emit invariant start for TLS guard address.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0) {
+uint64_t Width =
+CGM.getDataLayout().getTypeAllocSize(GuardVal->getType());
+llvm::Value *TLSAddr = Guard.getPointer();
+if (auto *GV = dyn_cast(Guard.getPointer()))
+ // Get the thread-local address via intrinsic.
+ if (GV->isThreadLocal())
+TLSAddr = Builder.CreateThreadLocalAddress(GV);
+Builder.CreateInvariantStart(
nikola-tesic-ns wrote:
Ok, well `EmitInvariantStart` expects Constant value, which `TLSAddr` cannot be
if we are going to set it conditionally. (But maybe it should be
unconditionally)
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
rjmccall wrote:
Then you should pass down whether this is for TLS.
What I'm trying to avoid is some situation where we silently don't emit the
intrinsic for a thread local because the guard address we pass in is not
directly a `GlobalValue`.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/nikola-tesic-ns updated
https://github.com/llvm/llvm-project/pull/96633
>From 41427a3de345517025477257bfd4f614f06cbcfe Mon Sep 17 00:00:00 2001
From: Nikola Tesic
Date: Tue, 25 Jun 2024 15:58:18 +0300
Subject: [PATCH 1/3] [Clang] Access tls_guard via llvm.threadlocal.address
This patch fixes compiler generated code in `tls_init` function to access
TLS variable (`tls_guard`) via llvm.threadlocal.address intrinsic.
---
clang/lib/CodeGen/CGDeclCXX.cpp | 29 +++---
clang/test/CodeGenCXX/cxx11-thread-local.cpp | 9 --
.../static-initializer-branch-weights.cpp | 3 +-
clang/test/OpenMP/parallel_copyin_codegen.cpp | 6 ++--
.../OpenMP/target_has_device_addr_codegen.cpp | 6 ++--
clang/test/OpenMP/threadprivate_codegen.cpp | 30 ---
6 files changed, 55 insertions(+), 28 deletions(-)
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index e18b339b31d24..0663a083bf3e8 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -1059,9 +1059,10 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ llvm::Value *GuardVal = EmitLoadOfScalar(
+ MakeAddrLValue(Guard, getContext().IntTy), SourceLocation());
+ llvm::Value *Uninit =
+ Builder.CreateIsNull(GuardVal, "guard.uninitialized");
llvm::BasicBlock *InitBlock = createBasicBlock("init");
ExitBlock = createBasicBlock("exit");
EmitCXXGuardedInitBranch(Uninit, InitBlock, ExitBlock,
@@ -1070,13 +1071,21 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
- EmitInvariantStart(
- Guard.getPointer(),
- CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ EmitStoreOfScalar(llvm::ConstantInt::get(GuardVal->getType(), 1),
+MakeAddrLValue(Guard, getContext().IntTy));
+
+ // Emit invariant start for TLS guard address.
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0) {
+uint64_t Width =
+CGM.getDataLayout().getTypeAllocSize(GuardVal->getType());
+llvm::Value *TLSAddr = Guard.getPointer();
+// Get the thread-local address via intrinsic.
+if (auto *GV = dyn_cast(Guard.getPointer()))
+ if (GV->isThreadLocal())
+TLSAddr = Builder.CreateThreadLocalAddress(Guard.getPointer());
+Builder.CreateInvariantStart(
+TLSAddr, llvm::ConstantInt::getSigned(Int64Ty, Width));
+ }
}
RunCleanupsScope Scope(*this);
diff --git a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
index bcc490bc32e6e..e9a0799cf8d9a 100644
--- a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -358,12 +358,15 @@ void set_anon_i() {
// CHECK: define {{.*}}@__tls_init()
-// CHECK: load i8, ptr @__tls_guard
+// CHECK: [[TLS_GUARD_ADDR_1:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK: load i8, ptr [[TLS_GUARD_ADDR_1]]
// CHECK: %[[NEED_TLS_INIT:.*]] = icmp eq i8 %{{.*}}, 0
// CHECK: br i1 %[[NEED_TLS_INIT]],
// init:
-// CHECK: store i8 1, ptr @__tls_guard
-// CHECK-OPT: call ptr @llvm.invariant.start.p0(i64 1, ptr @__tls_guard)
+// CHECK: [[TLS_GUARD_ADDR_2:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK: store i8 1, ptr [[TLS_GUARD_ADDR_2]]
+// CHECK-OPT: [[TLS_GUARD_ADDR_3:%.+]] = call align 1 ptr
@llvm.threadlocal.address.p0(ptr align 1 @__tls_guard)
+// CHECK-OPT: call ptr @llvm.invariant.start.p0(i64 1, ptr
[[TLS_GUARD_ADDR_3]])
// CHECK-NOT: call void @[[V_M_INIT]]()
// CHECK: call void @[[A_INIT]]()
// CHECK-NOT: call void @[[V_M_INIT]]()
diff --git a/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
b/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
index 121b9b2029959..e855f54643eae 100644
--- a/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
+++ b/clang/test/CodeGenCXX/static-initializer-branch-weights.cpp
@@ -118,7 +118,8 @@ void use_b() {
}
// CHECK-LABEL: define {{.*}}tls_init()
-// CHECK: load i8, ptr @__tls_guard, align 1
+// CHECK: [[TLS_GUARD_
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
nikola-tesic-ns wrote:
I've added changes to pass down `IsTLS` flag from the point where variable is
created. That should ensure we always emit TLS intrinsic.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1059,9 +1059,15 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
if (Guard.isValid()) {
// If we have a guard variable, check whether we've already performed
// these initializations. This happens for TLS initialization functions.
- llvm::Value *GuardVal = Builder.CreateLoad(Guard);
- llvm::Value *Uninit = Builder.CreateIsNull(GuardVal,
- "guard.uninitialized");
+ Address GuardAddr = Guard;
+ if (auto *GV = dyn_cast(Guard.getPointer()))
+// Get the thread-local address via intrinsic.
+if (GV->isThreadLocal())
+ GuardAddr = GuardAddr.withPointer(
+ Builder.CreateThreadLocalAddress(GV), NotKnownNonNull);
+ llvm::Value *GuardVal = Builder.CreateLoad(GuardAddr);
nikola-tesic-ns wrote:
I think this is related to the conversation in the other comment
> The Guard is a ConstantAddress, so I cannot change it, that's why I
> introduced new variable. If you have some suggestion, I would be happy to
> adapt the code.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/nikola-tesic-ns edited https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -769,9 +777,10 @@ void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary)
{
CharUnits GuardAlign = CharUnits::One();
Guard->setAlignment(GuardAlign.getAsAlign());
GuardAddr = ConstantAddress(Guard, Int8Ty, GuardAlign);
+ IsTLS = Guard->isThreadLocal();
}
-CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, ModuleInits,
- GuardAddr);
+CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, ModuleInits,
GuardAddr,
+ IsTLS);
rjmccall wrote:
```suggestion
/*IsTLS*/ false);
```
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -769,9 +777,10 @@ void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary)
{
CharUnits GuardAlign = CharUnits::One();
Guard->setAlignment(GuardAlign.getAsAlign());
GuardAddr = ConstantAddress(Guard, Int8Ty, GuardAlign);
+ IsTLS = Guard->isThreadLocal();
rjmccall wrote:
This is always just `false`. And I believe that's correct, module
initialization is global, not thread-local.
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1084,20 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn, // Mark as initialized before initializing anything else. If the // initializers use previously-initialized thread_local vars, that's // probably supposed to be OK, but the standard doesn't say. - Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); - - // The guard variable can't ever change again. + // Get the thread-local address via intrinsic. + if (IsTLS) +GuardAddr = GuardAddr.withPointer( +Builder.CreateThreadLocalAddress(Guard.getPointer()), +NotKnownNonNull); + Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1), + GuardAddr); + + // Emit invariant start for TLS guard address. EmitInvariantStart( Guard.getPointer(), CharUnits::fromQuantity( - CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(; + CGM.getDataLayout().getTypeAllocSize(GuardVal->getType())), + IsTLS); rjmccall wrote: Just use `GuardAddr` here, please, instead of passing a flag down. https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -2933,7 +2933,8 @@ void ItaniumCXXABI::EmitThreadLocalInitFuncs( Guard->setAlignment(GuardAlign.getAsAlign()); CodeGenFunction(CGM).GenerateCXXGlobalInitFunc( -InitFunc, OrderedInits, ConstantAddress(Guard, CGM.Int8Ty, GuardAlign)); +InitFunc, OrderedInits, ConstantAddress(Guard, CGM.Int8Ty, GuardAlign), +Guard->isThreadLocal()); rjmccall wrote: ```suggestion /*IsTLS*/ true); ``` https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1084,20 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn, // Mark as initialized before initializing anything else. If the // initializers use previously-initialized thread_local vars, that's // probably supposed to be OK, but the standard doesn't say. - Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); - - // The guard variable can't ever change again. + // Get the thread-local address via intrinsic. + if (IsTLS) +GuardAddr = GuardAddr.withPointer( +Builder.CreateThreadLocalAddress(Guard.getPointer()), +NotKnownNonNull); + Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1), + GuardAddr); + + // Emit invariant start for TLS guard address. EmitInvariantStart( Guard.getPointer(), CharUnits::fromQuantity( - CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(; + CGM.getDataLayout().getTypeAllocSize(GuardVal->getType())), + IsTLS); nikola-tesic-ns wrote: Am I allowed to reuse TLS address for invariant start intrinsic (example 1), or I need to access via intrinsic each time (example 2)? If the latter is true, I would need to recalculate the `GuardAddr` again. example 1: ``` %tls_addr1 = call align 1 ptr @llvm.threadlocal.address.p0(ptr align 1 @__tls_guard) store i8 1, ptr %tls_addr1, align 1 %3 = call ptr @llvm.invariant.start.p0(i64 1, ptr %tls_addr1) ``` example 2: ``` %tls_addr1 = call align 1 ptr @llvm.threadlocal.address.p0(ptr align 1 @__tls_guard) store i8 1, ptr %tls_addr1, align 1 %tls_addr2 = call align 1 ptr @llvm.threadlocal.address.p0(ptr align 1 @__tls_guard) %4 = call ptr @llvm.invariant.start.p0(i64 1, ptr %tls_addr2) ``` https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1084,20 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn, // Mark as initialized before initializing anything else. If the // initializers use previously-initialized thread_local vars, that's // probably supposed to be OK, but the standard doesn't say. - Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); - - // The guard variable can't ever change again. + // Get the thread-local address via intrinsic. + if (IsTLS) +GuardAddr = GuardAddr.withPointer( +Builder.CreateThreadLocalAddress(Guard.getPointer()), +NotKnownNonNull); + Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1), + GuardAddr); + + // Emit invariant start for TLS guard address. EmitInvariantStart( Guard.getPointer(), CharUnits::fromQuantity( - CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(; + CGM.getDataLayout().getTypeAllocSize(GuardVal->getType())), + IsTLS); rjmccall wrote: Yes, and in fact, sometimes that's required. The way I think about it is that `threadlocal.address` is a real dynamic operation that resolves the address of the thread-local variable for a specific thread: namely, the current thread at the execution point where `threadlocal.address` appears. When we're doing a complicated operation on a specific thread-local variable, it's important to call `threadlocal.address` at the right point and then use that result throughout the operation. Otherwise, a suspension in the middle of the operation will leave us working on a different thread-local variable at different points in the operation, which is not semantically allowed. In this case, we initialize a thread-local variable and then enter an invariant region for it. We need this to: 1. resolve the address of the TLV for the current thread, prior to performing any initialization; 2. initialize the TLV, which may include suspensions that change the current thread; and 3. enter an invariant region for the TLV we initialized, *not* the TLV for the new thread. The new thread's TLV may not yet be in an invariant region. Now, this may actually be moot, because I'm not sure it's actually allowed (or at least I'm not sure it *should* be allowed) to have a coroutine suspension in the middle of the initialization of a thread-local variable. The interaction of thread-locals with coroutines that actually switch threads is deeply problematic; notably, the TLV for the new thread can actually be uninitialized when you go to use it. I haven't checked what the standard says here, but honestly this might have to have undefined behavior, in which case we just need to make sure we generate reasonable code as long as the thread *doesn't* change. https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1084,20 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn, // Mark as initialized before initializing anything else. If the // initializers use previously-initialized thread_local vars, that's // probably supposed to be OK, but the standard doesn't say. - Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); - - // The guard variable can't ever change again. + // Get the thread-local address via intrinsic. + if (IsTLS) +GuardAddr = GuardAddr.withPointer( +Builder.CreateThreadLocalAddress(Guard.getPointer()), +NotKnownNonNull); + Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1), + GuardAddr); + + // Emit invariant start for TLS guard address. EmitInvariantStart( Guard.getPointer(), CharUnits::fromQuantity( - CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(; + CGM.getDataLayout().getTypeAllocSize(GuardVal->getType())), + IsTLS); nikola-tesic-ns wrote: I had similar concerns. IIUC, the safe(st possible) bet is to generate `threadlocal.address` at each access. In the view of this PR: 1. for the load of `tls_guard` 2. for the store into `tls_guard` 3. for the `invariant.start` intrinsic for `tls_guard` I am not sure if there is a case where coroutine context switch could happen between `threadlocal.address` and its user instruction. If that is the case, then there is no guarantee for other use cases of `threadlocal.address` as well. Do you agree or I am missing something? https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1084,20 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn, // Mark as initialized before initializing anything else. If the // initializers use previously-initialized thread_local vars, that's // probably supposed to be OK, but the standard doesn't say. - Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); - - // The guard variable can't ever change again. + // Get the thread-local address via intrinsic. + if (IsTLS) +GuardAddr = GuardAddr.withPointer( +Builder.CreateThreadLocalAddress(Guard.getPointer()), +NotKnownNonNull); + Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1), + GuardAddr); + + // Emit invariant start for TLS guard address. EmitInvariantStart( Guard.getPointer(), CharUnits::fromQuantity( - CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(; + CGM.getDataLayout().getTypeAllocSize(GuardVal->getType())), + IsTLS); rjmccall wrote: That's exactly backwards — it is semantically incorrect to access different guard variables at different points during the initialization; that does not correctly implement the guard pattern. Fortunately, I've been able to verify that it doesn't matter, because [expr.await]p3 says: > An *await-expression* shall not appear in the initializer of a block variable > with static or thread storage duration. Therefore, it is fine to just do a single use of `threadlocal.address`, and it's preferable because it's less code and likely easier to optimize. (Unfortunately, this restriction doesn't completely eliminate the coherence problems with coroutines and `thread_local`. In particular, a coroutine that declares a `thread_local` local variable can observe it in an uninitialized state if the thread has changed since the initialization, and I don't see anything in the standard that addresses this.) https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/rjmccall edited https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/rjmccall edited https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1084,20 @@
CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
// Mark as initialized before initializing anything else. If the
// initializers use previously-initialized thread_local vars, that's
// probably supposed to be OK, but the standard doesn't say.
- Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1),
Guard);
-
- // The guard variable can't ever change again.
+ // Get the thread-local address via intrinsic.
+ if (IsTLS)
+GuardAddr = GuardAddr.withPointer(
+Builder.CreateThreadLocalAddress(Guard.getPointer()),
+NotKnownNonNull);
+ Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1),
+ GuardAddr);
+
+ // Emit invariant start for TLS guard address.
EmitInvariantStart(
Guard.getPointer(),
CharUnits::fromQuantity(
- CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(;
+ CGM.getDataLayout().getTypeAllocSize(GuardVal->getType())),
+ IsTLS);
nikola-tesic-ns wrote:
Ok, thanks for your effort and detailed explanation!
Do you think the best we can do (in this case) is to get `threadlocal.address`
from `__tls_guard` at the first access in `__tls_init` function and reuse it in
the whole function (example below)?
```
define internal void @__tls_init() {
%tls_addr = call align 1 ptr @llvm.threadlocal.address.p0(ptr align 1
@__tls_guard)
%1 = load i8, ptr %1, align 1
%2 = icmp eq i8 %1, 0
br i1 %2, label %3, label %5
3:
store i8 1, ptr %tls_addr, align 1
%4 = call ptr @llvm.invariant.start.p0(i64 1, ptr %tls_addr)
call void @__cxx_global_var_init()
br label %5
5:
ret void
}
```
https://github.com/llvm/llvm-project/pull/96633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
@@ -1070,13 +1084,20 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn, // Mark as initialized before initializing anything else. If the // initializers use previously-initialized thread_local vars, that's // probably supposed to be OK, but the standard doesn't say. - Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); - - // The guard variable can't ever change again. + // Get the thread-local address via intrinsic. + if (IsTLS) +GuardAddr = GuardAddr.withPointer( +Builder.CreateThreadLocalAddress(Guard.getPointer()), +NotKnownNonNull); + Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(), 1), + GuardAddr); + + // Emit invariant start for TLS guard address. EmitInvariantStart( Guard.getPointer(), CharUnits::fromQuantity( - CGM.getDataLayout().getTypeAllocSize(GuardVal->getType(; + CGM.getDataLayout().getTypeAllocSize(GuardVal->getType())), + IsTLS); rjmccall wrote: I feel like we just need to file a DR with WG21 on this general question. The alternatives I can see: 1. Forbid TLVs in coroutines. Source-breaking. 2. Continue to eagerly initialize local TLVs in coroutines, but bind the name permanently to the TLV that was initialized. This means it is potentially a cross-thread reference after a suspension. It is the user's responsibility to make that not a problem. A semantics change for most (if not at all) existing compilers. 3. Like #2, but give references to the TLV undefined behavior if the thread has changed. The most compatible option, sadly. 4. Like #2, but make the program statically ill-formed if a suspension potentially intervenes between the initialization and a reference to the TLV. Source-breaking. Also a significant new implementation burden for compilers. 5. Switch local TLVs in coroutines to the same lazy-initialization model as global TLVs. Every reference to the variable dynamically resolves to the TLV for the current thread. Because of the model change, the variable is always initialized. A semantics change for most (if not all) existing compilers. 6. Like #5, but require the initializer to be a constant expression just to try to minimize the semantic impact. Note that a cross-thread reference can race not just with read/write or write/write conflicts between threads, but also with the destruction of the other thread. Because of the latter, it will be a potentially-dangling reference for most async-like use cases. https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/rjmccall edited https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/rjmccall edited https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Access tls_guard via llvm.threadlocal.address (PR #96633)
https://github.com/rjmccall edited https://github.com/llvm/llvm-project/pull/96633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
