rsmith created this revision. rsmith added a reviewer: rjmccall. rsmith requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added a project: clang.
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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D108680 Files: clang/lib/CodeGen/CGDeclCXX.cpp clang/lib/CodeGen/ItaniumCXXABI.cpp clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp clang/test/CodeGenCXX/cxx11-thread-local.cpp clang/test/CodeGenCXX/cxx1z-inline-variables.cpp clang/test/OpenMP/threadprivate_codegen.cpp
Index: clang/test/OpenMP/threadprivate_codegen.cpp =================================================================== --- clang/test/OpenMP/threadprivate_codegen.cpp +++ clang/test/OpenMP/threadprivate_codegen.cpp @@ -3847,7 +3847,7 @@ // // // 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 @@ // // // 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 @@ // // // 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 @@ // // // 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]] Index: clang/test/CodeGenCXX/cxx1z-inline-variables.cpp =================================================================== --- clang/test/CodeGenCXX/cxx1z-inline-variables.cpp +++ clang/test/CodeGenCXX/cxx1z-inline-variables.cpp @@ -90,9 +90,7 @@ // 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) Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp =================================================================== --- clang/test/CodeGenCXX/cxx11-thread-local.cpp +++ clang/test/CodeGenCXX/cxx11-thread-local.cpp @@ -179,8 +179,7 @@ // 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 @@ // 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 @@ // 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 @@ // 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]], Index: clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp =================================================================== --- /dev/null +++ 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 Index: clang/lib/CodeGen/ItaniumCXXABI.cpp =================================================================== --- clang/lib/CodeGen/ItaniumCXXABI.cpp +++ clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -2445,11 +2445,6 @@ (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())); } Index: clang/lib/CodeGen/CGDeclCXX.cpp =================================================================== --- clang/lib/CodeGen/CGDeclCXX.cpp +++ clang/lib/CodeGen/CGDeclCXX.cpp @@ -581,6 +581,16 @@ // 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()) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits