[MS] Implement on-demand TLS initialization for Microsoft CXX ABI

[momo5502]

Summary

TLS initializers, for example constructors of thread-local variables, don't necessarily get called. If a thread was created before a module is loaded, the module's TLS initializers are not executed for this particular thread.

This is why Microsoft added support for dynamic TLS initialization. Before every use of thread-local variables, a check is added that runs the module's TLS initializers on-demand.

To do this, the method __dyn_tls_on_demand_init gets called. Internally, it simply calls __dyn_tls_init.

No additional TLS initializer that sets the guard needs to be emitted, as the guard always gets set by __dyn_tls_init.
The guard is also checked again within __dyn_tls_init. This makes our check redundant, however, as Microsoft's compiler also emits this check, the behaviour is adopted here.

[rnk]

I probably don't have time to review this, so let me redirect to @hans.

clang/lib/CodeGen/MicrosoftCXXABI.cpp

2400

My understanding is that every DLL has exactly one __tls_guard variable. All TLS variables in a DLL are initialized as soon as a thread accesses one TLS variable for a DLL. Is that accurate? Can you add comments about the way this is intended to work here?

[majnemer]

Is this a new feature in MSVC? Seems like it might be. If so, should it be predicated on isCompatibleWithMSVC(1925) or something?

[momo5502]

A call to isCompatibleWithMSVC was added with the proper version that introduced the change (1925) and comments describing the intention of the change were added.

Additionally, a bug was fixed. Setting the guard variable to true before doing the initialization breaks the feature, as the guard needs to be false for __dyn_tls_init to work.
Not emitting the store to the guard should be fine, as it will be set by __dyn_tls_init anyways. The behaviour is now identical to the way Microsoft's compiler does it.

[majnemer]

clang/lib/CodeGen/MicrosoftCXXABI.cpp

404

Should this depend on the MSVC compatibility version?

[aganea]

clang/lib/CodeGen/MicrosoftCXXABI.cpp

2467

s/varibale/variable/

2474

Missing period at the end of the sentence please.

clang/test/CodeGenCXX/ms-thread_local.cpp

2

Also please check with -fms-compatibility-version=1920 to ensure that the dynamic initialization isn't generated.

[momo5502]

Comments were addressed

[majnemer]

This is looking great! Just a few more questions.

What is the behavior with something like:

thread_local int x = 2;
int f() {
  return x;
}

I'm wondering if we need to move this logic into the generic C++ ABI implementation.

[momo5502]

In D115456#3216811, @majnemer wrote:

This is looking great! Just a few more questions.

What is the behavior with something like:

thread_local int x = 2;
int f() {
  return x;
}

I'm wondering if we need to move this logic into the generic C++ ABI implementation.

The MS compiler only emits the dynamic initializers for variables with constructors/destructors, just like it is currently done here for the Itanium ABI.
I also thought about adopting that behaviour, but I think threre are edge-cases when triggering dynamic TLS initialization even for constant variables is useful.
For example there might be custom TLS callbacks that can affect the value of this variable.

If desired, I can change it to match the behaviour of MS, but I thought it could be beneficial to diverge in this case.

[majnemer]

In D115456#3217595, @momo5502 wrote:

In D115456#3216811, @majnemer wrote:

This is looking great! Just a few more questions.

What is the behavior with something like:

thread_local int x = 2;
int f() {
  return x;
}

I'm wondering if we need to move this logic into the generic C++ ABI implementation.

The MS compiler only emits the dynamic initializers for variables with constructors/destructors, just like it is currently done here for the Itanium ABI.
I also thought about adopting that behaviour, but I think threre are edge-cases when triggering dynamic TLS initialization even for constant variables is useful.
For example there might be custom TLS callbacks that can affect the value of this variable.

If desired, I can change it to match the behaviour of MS, but I thought it could be beneficial to diverge in this case.

IMO, it is probably best to match behavior here within reason (ignoring bugs latent in MSVC) here. My thinking is that in the face of COMDATs, we cannot ensure which copy of an inline function will make its way to the binary and different link orders would provide different behavior.

[momo5502]

In D115456#3217857, @majnemer wrote:

In D115456#3217595, @momo5502 wrote:

In D115456#3216811, @majnemer wrote:

This is looking great! Just a few more questions.

What is the behavior with something like:

thread_local int x = 2;
int f() {
  return x;
}

I'm wondering if we need to move this logic into the generic C++ ABI implementation.

The MS compiler only emits the dynamic initializers for variables with constructors/destructors, just like it is currently done here for the Itanium ABI.
I also thought about adopting that behaviour, but I think threre are edge-cases when triggering dynamic TLS initialization even for constant variables is useful.
For example there might be custom TLS callbacks that can affect the value of this variable.

If desired, I can change it to match the behaviour of MS, but I thought it could be beneficial to diverge in this case.

IMO, it is probably best to match behavior here within reason (ignoring bugs latent in MSVC) here. My thinking is that in the face of COMDATs, we cannot ensure which copy of an inline function will make its way to the binary and different link orders would provide different behavior.

Fair enough. Logic has now moved to the generic C++ ABI implementation and the test was adjusted.

[majnemer]

Looks great! Please give others some time to review it as it is a holiday season...

[hans]

Looks great, thanks! Maurice, do you want to add a note about this to docs/ReleaseNotes.rst?

[momo5502]

In D115456#3248313, @hans wrote:

Looks great, thanks! Maurice, do you want to add a note about this to docs/ReleaseNotes.rst?

Good point. I created a new change: https://reviews.llvm.org/D117500

[doughnutlifetime]

@dumb ways to die This article was very helpful to me.

clang/lib/CodeGen/MicrosoftCXXABI.cpp

llvm::Value *performReturnAdjustment(CodeGenFunction &CGF, Address Ret,
                                        const ReturnAdjustment &RA) override;
 
   void EmitThreadLocalInitFuncs(
       CodeGenModule &CGM, ArrayRef<const VarDecl *> CXXThreadLocals,
       ArrayRef<llvm::Function *> CXXThreadLocalInits,
       ArrayRef<const VarDecl *> CXXThreadLocalInitVars) override;
 
   bool usesThreadWrapperFunction(const VarDecl *VD) const override {
-    return false;
+    return true;
   }
   LValue EmitThreadLocalVarDeclLValue(CodeGenFunction &CGF, const VarDecl *VD,
                                       QualType LValType) override;
 
   void EmitGuardedInit(CodeGenFunction &CGF, const VarDecl &D,
                        llvm::GlobalVariable *DeclPtr,
                        bool PerformInit) override;
   void registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D,
llvm::Function *InitFunc = CGM.CreateGlobalInitOrCleanUpFunction(
         FTy, "__tls_init", CGM.getTypes().arrangeNullaryFunction(),
         SourceLocation(), /*TLS=*/true);
     CodeGenFunction(CGM).GenerateCXXGlobalInitFunc(InitFunc, NonComdatInits);
 
     AddToXDU(InitFunc);
   }
 }
 
+static llvm::GlobalValue *getTlsGuardVar(CodeGenModule &CGM) {
+  llvm::Type *VTy = llvm::Type::getInt8Ty(CGM.getLLVMContext());
+  llvm::Constant *TlsGuardConstant =
+      CGM.CreateRuntimeVariable(VTy, "__tls_guard");
+  llvm::GlobalValue *TlsGuard = cast<llvm::GlobalValue>(TlsGuardConstant);
+
+  TlsGuard->setThreadLocal(true);
+
+  return TlsGuard;
+}
+
+static llvm::FunctionCallee getDynTlsOnDemandInitFn(CodeGenModule &CGM) {
+  llvm::FunctionType *FTy =
+      llvm::FunctionType::get(llvm::Type::getVoidTy(CGM.getLLVMContext()), {},
+                              /*isVarArg=*/false);
+  return CGM.CreateRuntimeFunction(
+      FTy, "__dyn_tls_on_demand_init",
+      llvm::AttributeList::get(CGM.getLLVMContext(),
+                               llvm::AttributeList::FunctionIndex,
+                               llvm::Attribute::NoUnwind),
+      /*Local=*/true);
+}
+
+static void emitTlsGuardCheck(CodeGenFunction &CGF, llvm::GlobalValue *TlsGuard,
+                              llvm::BasicBlock *DynInitBB,
+                              llvm::BasicBlock *ContinueBB) {
+  llvm::LoadInst *TlsGuardValue =
+      CGF.Builder.CreateLoad(Address(TlsGuard, CharUnits::One()));
+  llvm::Value *CmpResult =
+      CGF.Builder.CreateICmpEQ(TlsGuardValue, CGF.Builder.getInt8(0));
+  CGF.Builder.CreateCondBr(CmpResult, DynInitBB, ContinueBB);
+}
+
+static void emitDynamicTlsInitialization(CodeGenFunction &CGF,
+                                         llvm::GlobalValue *TlsGuard,
+                                         llvm::BasicBlock *ContinueBB) {
+  CGF.Builder.CreateStore(CGF.Builder.getInt8(1),
+                          Address(TlsGuard, CharUnits::One()));
+
+  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 emitDynamicTlsInitializationSequence(CodeGenFunction &CGF) {
+  llvm::BasicBlock *DynInitBB =
+      CGF.createBasicBlock("dyntls.dyn_init", CGF.CurFn);
+  llvm::BasicBlock *ContinueBB =
+      CGF.createBasicBlock("dyntls.continue", CGF.CurFn);
+
+  llvm::GlobalValue *TlsGuard = getTlsGuardVar(CGF.CGM);
+
+  emitTlsGuardCheck(CGF, TlsGuard, DynInitBB, ContinueBB);
+  CGF.Builder.SetInsertPoint(DynInitBB);
+  emitDynamicTlsInitialization(CGF, TlsGuard, ContinueBB);
+  CGF.Builder.SetInsertPoint(ContinueBB);
+}
+
 LValue MicrosoftCXXABI::EmitThreadLocalVarDeclLValue(CodeGenFunction &CGF,
                                                      const VarDecl *VD,
                                                      QualType LValType) {
-  CGF.CGM.ErrorUnsupported(VD, "thread wrappers");
-  return LValue();
+  emitDynamicTlsInitializationSequence(CGF);
+
+  llvm::Value *V = CGF.CGM.GetAddrOfGlobalVar(VD);
+  llvm::Type *RealVarTy = CGF.getTypes().ConvertTypeForMem(VD->getType());
+
+  unsigned AS = cast<llvm::PointerType>(V->getType())->getAddressSpace();
+  V = CGF.Builder.CreateBitCast(V, RealVarTy->getPointerTo(AS));
+
+  CharUnits Alignment = CGF.getContext().getDeclAlign(VD);
+  Address Addr(V, Alignment);
+
+  LValue LV = VD->getType()->isReferenceType()
+                ? CGF.EmitLoadOfReferenceLValue(Addr, VD->getType(),
+                                                AlignmentSource::Decl)
+                : CGF.MakeAddrLValue(Addr, LValType, AlignmentSource::Decl);
+  return LV;
 }
 
 static ConstantAddress getInitThreadEpochPtr(CodeGenModule &CGM) {
   StringRef VarName("_Init_thread_epoch");
   CharUnits Align = CGM.getIntAlign();
   if (auto *GV = CGM.getModule().getNamedGlobal(VarName))
     return ConstantAddress(GV, Align);
   auto *GV = new llvm::GlobalVariable(

clang/test/CodeGenCXX/ms-thread_local.cpp

 // RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s  -std=c++1y -triple=i686-pc-win32 -ftls-model=local-dynamic -emit-llvm -o - | FileCheck %s -check-prefix=CHECK-LD
 
 struct A {
   A();
   ~A();
 };
 
 // CHECK-DAG: $"??$a@X@@3UA@@A" = comdat any
 // CHECK-DAG: @"??$a@X@@3UA@@A" = linkonce_odr dso_local thread_local global %struct.A zeroinitializer, comdat, align 1
 // CHECK-DAG: @"??__E?$a@X@@YAXXZ$initializer$" = internal constant void ()* @"??__E?$a@X@@YAXXZ", section ".CRT$XDU", comdat($"??$a@X@@3UA@@A")
 // CHECK-LD-DAG: $"??$a@X@@3UA@@A" = comdat any
 // CHECK-LD-DAG: @"??$a@X@@3UA@@A" = linkonce_odr dso_local thread_local(localdynamic) global %struct.A zeroinitializer, comdat, align 1
 // CHECK-LD-DAG: @"??__E?$a@X@@YAXXZ$initializer$" = internal constant void ()* @"??__E?$a@X@@YAXXZ", section ".CRT$XDU", comdat($"??$a@X@@3UA@@A")
 template <typename T>
 thread_local A a = A();
 
 // CHECK-DAG: @"?b@@3UA@@A" = dso_local thread_local global %struct.A zeroinitializer, align 1
 // CHECK-DAG: @"__tls_init$initializer$" = internal constant void ()* @__tls_init, section ".CRT$XDU"
+// CHECK-DAG: @__tls_guard = external dso_local thread_local global i8
 // CHECK-LD-DAG: @"?b@@3UA@@A" = dso_local thread_local(localdynamic) global %struct.A zeroinitializer, align 1
 // CHECK-LD-DAG: @"__tls_init$initializer$" = internal constant void ()* @__tls_init, section ".CRT$XDU"
+// CHECK-LD-DAG: @__tls_guard = external dso_local thread_local global i8
 thread_local A b;
 
+// CHECK-LABEL: declare dso_local void @__dyn_tls_on_demand_init()
+// CHECK-LD-LABEL: declare dso_local void @__dyn_tls_on_demand_init()
+
+// CHECK-LABEL: define dso_local void @"?f@@YA?AUA@@XZ"(%struct.A* noalias sret(%struct.A) align 1 %agg.result)
+// CHECK: call void @__dyn_tls_on_demand_init()
+// CHECK-LD-LABEL: define dso_local void @"?f@@YA?AUA@@XZ"(%struct.A* noalias sret(%struct.A) align 1 %agg.result)
+// CHECK-LD: call void @__dyn_tls_on_demand_init()
+
 // CHECK-LABEL: define internal void @__tls_init()
 // CHECK: call void @"??__Eb@@YAXXZ"
 // CHECK-LD-LABEL: define internal void @__tls_init()
 // CHECK-LD: call void @"??__Eb@@YAXXZ"
 
 thread_local A &c = b;
 thread_local A &d = c;