From aca3d067efe194539efd1e0fcf03820a2c377753 Mon Sep 17 00:00:00 2001 From: James Y Knight Date: Wed, 27 May 2020 09:55:29 -0400 Subject: Fix Darwin 'constinit thread_local' variables. Unlike other platforms using ItaniumCXXABI, Darwin does not allow the creation of a thread-wrapper function for a variable in the TU of users. Because of this, it can set the linkage of the thread-local symbol to internal, with the assumption that no TUs other than the one defining the variable will need it. However, constinit thread_local variables do not require the use of the thread-wrapper call, so users reference the variable directly. Thus, it must not be converted to internal, or users will get a link failure. This was a regression introduced by the optimization in 00223827a952f66e7426c9881a2a4229e59bb019. Differential Revision: https://reviews.llvm.org/D80417 --- clang/lib/CodeGen/CodeGenModule.cpp | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) (limited to 'clang/lib/CodeGen/CodeGenModule.cpp') diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index f43bc6434daf..89a95db08680 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -4136,17 +4136,24 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D, GV->setAlignment(getContext().getDeclAlign(D).getAsAlign()); - // On Darwin, if the normal linkage of a C++ thread_local variable is - // LinkOnce or Weak, we keep the normal linkage to prevent multiple - // copies within a linkage unit; otherwise, the backing variable has - // internal linkage and all accesses should just be calls to the - // Itanium-specified entry point, which has the normal linkage of the - // variable. This is to preserve the ability to change the implementation - // behind the scenes. - if (!D->isStaticLocal() && D->getTLSKind() == VarDecl::TLS_Dynamic && + // On Darwin, unlike other Itanium C++ ABI platforms, the thread-wrapper + // function is only defined alongside the variable, not also alongside + // callers. Normally, all accesses to a thread_local go through the + // thread-wrapper in order to ensure initialization has occurred, underlying + // variable will never be used other than the thread-wrapper, so it can be + // converted to internal linkage. + // + // However, if the variable has the 'constinit' attribute, it _can_ be + // referenced directly, without calling the thread-wrapper, so the linkage + // must not be changed. + // + // Additionally, if the variable isn't plain external linkage, e.g. if it's + // weak or linkonce, the de-duplication semantics are important to preserve, + // so we don't change the linkage. + if (D->getTLSKind() == VarDecl::TLS_Dynamic && + Linkage == llvm::GlobalValue::ExternalLinkage && Context.getTargetInfo().getTriple().isOSDarwin() && - !llvm::GlobalVariable::isLinkOnceLinkage(Linkage) && - !llvm::GlobalVariable::isWeakLinkage(Linkage)) + !D->hasAttr()) Linkage = llvm::GlobalValue::InternalLinkage; GV->setLinkage(Linkage); -- cgit v1.2.3