[clang] cd4d6d7 - PR48030: Fix COMDAT-related linking problem with C++ thread_local static data members.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 24 19:54:05 PDT 2021
Author: Richard Smith
Date: 2021-08-24T19:53:44-07:00
New Revision: cd4d6d718b2e51ed830a28d01d765da2a220afd3
URL: https://github.com/llvm/llvm-project/commit/cd4d6d718b2e51ed830a28d01d765da2a220afd3
DIFF: https://github.com/llvm/llvm-project/commit/cd4d6d718b2e51ed830a28d01d765da2a220afd3.diff
LOG: PR48030: Fix COMDAT-related linking problem with C++ thread_local static data members.
Previously when emitting a C++ guarded initializer, we tried to work out what
the enclosing function would be used for and added it to the COMDAT containing
the variable if we thought that doing so would be correct. But this was done
from a context in which we didn't -- and realistically couldn't -- correctly
infer how the enclosing function would be used.
Instead, add the initialization function to a COMDAT from the code that
creates it, in the case where it makes sense to do so: when we know that
the one and only reference to the initialization function is in
@llvm.global.ctors and that reference is in the same COMDAT.
Reviewed By: rjmccall
Differential Revision: https://reviews.llvm.org/D108680
Added:
clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp
Modified:
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/cxx11-thread-local.cpp
clang/test/CodeGenCXX/cxx1z-inline-variables.cpp
clang/test/OpenMP/threadprivate_codegen.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 553fedebfe56b..d22f9dc3b68cd 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -581,6 +581,16 @@ CodeGenModule::EmitCXXGlobalVarDeclInitFunc(const VarDecl *D,
// llvm.used to prevent linker GC.
addUsedGlobal(COMDATKey);
}
+
+ // If we used a COMDAT key for the global ctor, the init function can be
+ // discarded if the global ctor entry is discarded.
+ // FIXME: Do we need to restrict this to ELF and Wasm?
+ llvm::Comdat *C = Addr->getComdat();
+ if (COMDATKey && C &&
+ (getTarget().getTriple().isOSBinFormatELF() ||
+ getTarget().getTriple().isOSBinFormatWasm())) {
+ Fn->setComdat(C);
+ }
} else {
I = DelayedCXXInitPosition.find(D); // Re-do lookup in case of re-hash.
if (I == DelayedCXXInitPosition.end()) {
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index d3dc0e6212b8c..8ddc2f1885eb1 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2445,11 +2445,6 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
(CGM.getTarget().getTriple().isOSBinFormatELF() ||
CGM.getTarget().getTriple().isOSBinFormatWasm())) {
guard->setComdat(C);
- // An inline variable's guard function is run from the per-TU
- // initialization function, not via a dedicated global ctor function, so
- // we can't put it in a comdat.
- if (!NonTemplateInline)
- CGF.CurFn->setComdat(C);
} else if (CGM.supportsCOMDAT() && guard->isWeakForLinker()) {
guard->setComdat(CGM.getModule().getOrInsertComdat(guard->getName()));
}
diff --git a/clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp b/clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp
new file mode 100644
index 0000000000000..3c78c7f7df902
--- /dev/null
+++ b/clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+
+// PR48030
+
+template<typename T> struct TLS { static thread_local T *mData; };
+inline decltype(nullptr) non_constant_initializer() { return nullptr; }
+template<typename T> thread_local T *TLS<T>::mData = non_constant_initializer();
+struct S {};
+S *current() { return TLS<S>::mData; };
+
+// CHECK-DAG: @_ZN3TLSI1SE5mDataE = linkonce_odr thread_local global {{.*}}, comdat,
+// CHECK-DAG: @_ZGVN3TLSI1SE5mDataE = linkonce_odr thread_local global {{.*}}, comdat($_ZN3TLSI1SE5mDataE),
+// CHECK-DAG: @_ZTHN3TLSI1SE5mDataE = linkonce_odr alias {{.*}} @__cxx_global_var_init
+
+// CHECK-LABEL: define {{.*}} @_Z7currentv()
+// CHECK: call {{.*}} @_ZTWN3TLSI1SE5mDataE()
+
+// CHECK-LABEL: define weak_odr hidden {{.*}} @_ZTWN3TLSI1SE5mDataE() {{.*}} comdat {
+// CHECK: call void @_ZTHN3TLSI1SE5mDataE()
+// CHECK: ret {{.*}} @_ZN3TLSI1SE5mDataE
+
+// Unlike for a global, the global initialization function must not be in a
+// COMDAT with the variable, because it is referenced from the _ZTH function
+// which is outside that COMDAT.
+//
+// CHECK-NOT: define {{.*}} @__cxx_global_var_init{{.*}}comdat
diff --git a/clang/test/CodeGenCXX/cxx11-thread-local.cpp b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
index f3146715b9214..e63762ee7bfa0 100644
--- a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -179,8 +179,7 @@ int f() {
// LINUX_AIX: define internal void @[[VF_M_INIT]]()
// DARWIN: define internal cxx_fast_tlscc void @[[VF_M_INIT]]()
-// LINUX-SAME: comdat($_ZN1VIfE1mE)
-// DARWIN-NOT: comdat
+// CHECK-NOT: comdat
// CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIfE1mE to i8*)
// CHECK: %[[VF_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
// CHECK: br i1 %[[VF_M_INITIALIZED]],
@@ -192,8 +191,7 @@ int f() {
// LINUX_AIX: define internal void @[[XF_M_INIT]]()
// DARWIN: define internal cxx_fast_tlscc void @[[XF_M_INIT]]()
-// LINUX-SAME: comdat($_ZN1XIfE1mE)
-// DARWIN-NOT: comdat
+// CHECK-NOT: comdat
// CHECK: load i8, i8* bitcast (i64* @_ZGVN1XIfE1mE to i8*)
// CHECK: %[[XF_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
// CHECK: br i1 %[[XF_M_INITIALIZED]],
@@ -313,8 +311,7 @@ void set_anon_i() {
// LINUX_AIX: define internal void @[[V_M_INIT]]()
// DARWIN: define internal cxx_fast_tlscc void @[[V_M_INIT]]()
-// LINUX-SAME: comdat($_ZN1VIiE1mE)
-// DARWIN-NOT: comdat
+// CHECK-NOT: comdat
// CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIiE1mE to i8*)
// CHECK: %[[V_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
// CHECK: br i1 %[[V_M_INITIALIZED]],
@@ -326,8 +323,7 @@ void set_anon_i() {
// LINUX_AIX: define internal void @[[X_M_INIT]]()
// DARWIN: define internal cxx_fast_tlscc void @[[X_M_INIT]]()
-// LINUX-SAME: comdat($_ZN1XIiE1mE)
-// DARWIN-NOT: comdat
+// CHECK-NOT: comdat
// CHECK: load i8, i8* bitcast (i64* @_ZGVN1XIiE1mE to i8*)
// CHECK: %[[X_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
// CHECK: br i1 %[[X_M_INITIALIZED]],
diff --git a/clang/test/CodeGenCXX/cxx1z-inline-variables.cpp b/clang/test/CodeGenCXX/cxx1z-inline-variables.cpp
index a616e95a2be55..896db4acc1318 100644
--- a/clang/test/CodeGenCXX/cxx1z-inline-variables.cpp
+++ b/clang/test/CodeGenCXX/cxx1z-inline-variables.cpp
@@ -90,9 +90,7 @@ const int &yib = Y<int>::b;
// CHECK-LABEL: define {{.*}}global_var_init
// CHECK: call i32 @_Z1fv
-// CHECK-LABEL: define {{.*}}global_var_init
-// CHECK-NOT: comdat
-// CHECK-SAME: {{$}}
+// CHECK-LABEL: define {{.*}}global_var_init{{.*}} comdat($b)
// CHECK: load atomic {{.*}} acquire, align
// CHECK: br
// CHECK: __cxa_guard_acquire(i64* @_ZGV1b)
diff --git a/clang/test/OpenMP/threadprivate_codegen.cpp b/clang/test/OpenMP/threadprivate_codegen.cpp
index c07ffe1a28ccb..f1ccb80cb3ab4 100644
--- a/clang/test/OpenMP/threadprivate_codegen.cpp
+++ b/clang/test/OpenMP/threadprivate_codegen.cpp
@@ -3847,7 +3847,7 @@ int foobar() {
//
//
// CHECK-TLS1-LABEL: define {{[^@]+}}@__cxx_global_var_init.3
-// CHECK-TLS1-SAME: () #[[ATTR0]] comdat($_ZN2STI2S4E2stE) {
+// CHECK-TLS1-SAME: () #[[ATTR0]] {
// CHECK-TLS1-NEXT: entry:
// CHECK-TLS1-NEXT: [[TMP0:%.*]] = load i8, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8
// CHECK-TLS1-NEXT: [[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0
@@ -4373,7 +4373,7 @@ int foobar() {
//
//
// CHECK-TLS2-LABEL: define {{[^@]+}}@__cxx_global_var_init.3
-// CHECK-TLS2-SAME: () #[[ATTR6]] comdat($_ZN2STI2S4E2stE) {
+// CHECK-TLS2-SAME: () #[[ATTR6]] {
// CHECK-TLS2-NEXT: entry:
// CHECK-TLS2-NEXT: [[TMP0:%.*]] = load i8, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8
// CHECK-TLS2-NEXT: [[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0
@@ -4906,7 +4906,7 @@ int foobar() {
//
//
// CHECK-TLS3-LABEL: define {{[^@]+}}@__cxx_global_var_init.3
-// CHECK-TLS3-SAME: () #[[ATTR0]] comdat($_ZN2STI2S4E2stE) !dbg [[DBG289:![0-9]+]] {
+// CHECK-TLS3-SAME: () #[[ATTR0]] !dbg [[DBG289:![0-9]+]] {
// CHECK-TLS3-NEXT: entry:
// CHECK-TLS3-NEXT: [[TMP0:%.*]] = load i8, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8, !dbg [[DBG290:![0-9]+]]
// CHECK-TLS3-NEXT: [[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0, !dbg [[DBG290]]
@@ -5459,7 +5459,7 @@ int foobar() {
//
//
// CHECK-TLS4-LABEL: define {{[^@]+}}@__cxx_global_var_init.3
-// CHECK-TLS4-SAME: () #[[ATTR7]] comdat($_ZN2STI2S4E2stE) !dbg [[DBG289:![0-9]+]] {
+// CHECK-TLS4-SAME: () #[[ATTR7]] !dbg [[DBG289:![0-9]+]] {
// CHECK-TLS4-NEXT: entry:
// CHECK-TLS4-NEXT: [[TMP0:%.*]] = load i8, i8* bitcast (i64* @_ZGVN2STI2S4E2stE to i8*), align 8, !dbg [[DBG290:![0-9]+]]
// CHECK-TLS4-NEXT: [[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0, !dbg [[DBG290]]
More information about the cfe-commits
mailing list