llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Sp0tless <details> <summary>Changes</summary> When targeting the Microsoft C++ ABI with `-femulated-tls`, Clang currently mixes two TLS models. User `thread_local` variables are lowered through emutls, but the Microsoft ABI initialization paths still directly access native CRT TLS variables: - thread-safe local static initialization reads `_Init_thread_epoch`; - dynamic TLS initialization reads `__tls_guard`. These CRT-owned variables do not have emutls descriptors, so lowering produces unresolved references such as `__emutls_v._Init_thread_epoch` and `__emutls_v.__tls_guard`. This is reproducible with ordinary Clang, without clang-repl or ORC. This Compiler Explorer example uses `x86-64 clang 22.1.0 --target=x86_64-pc-windows-msvc -femulated-tls` and shows both invalid emutls references in the generated assembly: https://godbolt.org/z/hseq4TK4T Avoid both direct native-TLS accesses when emulated TLS is enabled. Dynamic TLS calls `__dyn_tls_on_demand_init` directly; the CRT helper already performs the guard check. Local static initialization enters `_Init_thread_header` on every access and retains the existing second guard check, initialization, abort, and footer paths. The native TLS code path is unchanged. The local-static fallback trades the native per-thread epoch fast path for correctness and may add synchronization overhead on repeated access. The affected combination, Microsoft ABI with explicitly enabled emulated TLS, appears uncommon. I would appreciate guidance on whether this fallback is acceptable or whether an emulated epoch fast path should be introduced instead. I originally found this while testing emulated TLS in an experimental Windows `clang-repl` configuration. Fixing the general Clang CodeGen behavior is also a prerequisite for using emulated TLS there, but this patch does not contain interpreter or ORC integration changes. Related investigation: #<!-- -->213568. Testing: Added `clang/test/CodeGenCXX/microsoft-abi-emulated-tls.cpp`; the new test and the neighboring native MSVC TLS tests pass. Assisted-by: OpenAI Codex --- Full diff: https://github.com/llvm/llvm-project/pull/220215.diff 2 Files Affected: - (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+25-13) - (added) clang/test/CodeGenCXX/microsoft-abi-emulated-tls.cpp (+28) ``````````diff diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index 8b43dd887573d..4ae9ea7ec18c9 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -2531,19 +2531,23 @@ static void emitTlsGuardCheck(CodeGenFunction &CGF, llvm::GlobalValue *TlsGuard, CGF.Builder.CreateCondBr(CmpResult, DynInitBB, ContinueBB); } -static void emitDynamicTlsInitializationCall(CodeGenFunction &CGF, - llvm::GlobalValue *TlsGuard, - llvm::BasicBlock *ContinueBB) { +static void emitDynamicTlsInitializationCall(CodeGenFunction &CGF) { llvm::FunctionCallee Initializer = getDynTlsOnDemandInitFn(CGF.CGM); llvm::Function *InitializerFunction = cast<llvm::Function>(Initializer.getCallee()); llvm::CallInst *CallVal = CGF.Builder.CreateCall(InitializerFunction); CallVal->setCallingConv(InitializerFunction->getCallingConv()); - - CGF.Builder.CreateBr(ContinueBB); } static void emitDynamicTlsInitialization(CodeGenFunction &CGF) { + if (CGF.CGM.getCodeGenOpts().EmulatedTLS) { + // __tls_guard is native TLS owned by the MSVC runtime and does not have an + // emutls descriptor. The runtime helper performs the same guard check, so + // call it directly rather than emitting a reference to __tls_guard. + emitDynamicTlsInitializationCall(CGF); + return; + } + llvm::BasicBlock *DynInitBB = CGF.createBasicBlock("dyntls.dyn_init", CGF.CurFn); llvm::BasicBlock *ContinueBB = @@ -2553,7 +2557,8 @@ static void emitDynamicTlsInitialization(CodeGenFunction &CGF) { emitTlsGuardCheck(CGF, TlsGuard, DynInitBB, ContinueBB); CGF.Builder.SetInsertPoint(DynInitBB); - emitDynamicTlsInitializationCall(CGF, TlsGuard, ContinueBB); + emitDynamicTlsInitializationCall(CGF); + CGF.Builder.CreateBr(ContinueBB); CGF.Builder.SetInsertPoint(ContinueBB); } @@ -2791,13 +2796,20 @@ void MicrosoftCXXABI::EmitGuardedInit(CodeGenFunction &CGF, const VarDecl &D, // The algorithm is almost identical to what can be found in the appendix // found in N2325. - // This BasicBLock determines whether or not we have any work to do. - llvm::LoadInst *FirstGuardLoad = Builder.CreateLoad(GuardAddr); - FirstGuardLoad->setOrdering(llvm::AtomicOrdering::Unordered); - llvm::LoadInst *InitThreadEpoch = - Builder.CreateLoad(getInitThreadEpochPtr(CGM)); - llvm::Value *IsUninitialized = - Builder.CreateICmpSGT(FirstGuardLoad, InitThreadEpoch); + llvm::Value *IsUninitialized; + if (CGM.getCodeGenOpts().EmulatedTLS) { + // _Init_thread_epoch is native TLS owned by the MSVC runtime. It does + // not have an emutls descriptor. Without its per-thread epoch, enter the + // runtime on every access to preserve its synchronization guarantees. + IsUninitialized = Builder.getTrue(); + } else { + // This test determines whether or not we have any work to do. + llvm::LoadInst *FirstGuardLoad = Builder.CreateLoad(GuardAddr); + FirstGuardLoad->setOrdering(llvm::AtomicOrdering::Unordered); + llvm::LoadInst *InitThreadEpoch = + Builder.CreateLoad(getInitThreadEpochPtr(CGM)); + IsUninitialized = Builder.CreateICmpSGT(FirstGuardLoad, InitThreadEpoch); + } llvm::BasicBlock *AttemptInitBlock = CGF.createBasicBlock("init.attempt"); llvm::BasicBlock *EndBlock = CGF.createBasicBlock("init.end"); CGF.EmitCXXGuardedInitBranch(IsUninitialized, AttemptInitBlock, EndBlock, diff --git a/clang/test/CodeGenCXX/microsoft-abi-emulated-tls.cpp b/clang/test/CodeGenCXX/microsoft-abi-emulated-tls.cpp new file mode 100644 index 0000000000000..0579660dbf5c2 --- /dev/null +++ b/clang/test/CodeGenCXX/microsoft-abi-emulated-tls.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -std=c++11 \ +// RUN: -fms-compatibility-version=19.25 -femulated-tls \ +// RUN: -emit-llvm -o - %s | FileCheck \ +// RUN: --implicit-check-not=_Init_thread_epoch \ +// RUN: --implicit-check-not=__tls_guard %s + +int make_value(); + +// CHECK-DAG: @"__tls_init$initializer$" = internal constant ptr @__tls_init, section ".CRT$XDU" +// CHECK-LABEL: define dso_local noundef i32 @"?guarded_value@@YAHXZ"() +// CHECK: br i1 true, label %[[ATTEMPT:[a-z.]+]], label %[[END:[a-z.]+]] +// CHECK: [[ATTEMPT]]: +// CHECK: call void @_Init_thread_header +// CHECK: call void @_Init_thread_footer +// CHECK: [[END]]: +int guarded_value() { + static int value = make_value(); + return value; +} + +int make_tls_value(); +thread_local int dynamic_tls = make_tls_value(); + +// CHECK-LABEL: define dso_local noundef i32 @"?read_dynamic_tls@@YAHXZ"() +// CHECK-NEXT: entry: +// CHECK-NEXT: call void @__dyn_tls_on_demand_init() +// CHECK: call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @"?dynamic_tls@@3HA") +int read_dynamic_tls() { return dynamic_tls; } `````````` </details> https://github.com/llvm/llvm-project/pull/220215 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
