[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. Looks great! Please give others some time to review it as it is a holiday season... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115456/new/ https://reviews.llvm.org/D115456 __

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

momo5502 updated this revision to Diff 397107. momo5502 added a comment. In D115456#3217857 , @majnemer wrote: > In D115456#3217595 , @momo5502 > wrote: > >> In D115456#3216811

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. 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: >>

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

momo5502 added a comment. 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 nee

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. 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

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

momo5502 updated this revision to Diff 396783. momo5502 marked 4 inline comments as done. momo5502 added a comment. Comments were addressed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115456/new/ https://reviews.llvm.org/D115456 Files: clang/include/clang/Basic/LangOptions.h clan

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

aganea added inline comments. Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:2470 +// Dynamic TLS initialization works by checking the state of a +// guard varibale (__tls_guard) to see whether TLS initialization +// for a thread has happend yet. s

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added inline comments. Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:404 bool usesThreadWrapperFunction(const VarDecl *VD) const override { -return false; +return true; } Should this depend on the MSVC compatibility version? CHANGES S

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

momo5502 updated this revision to Diff 395856. momo5502 edited the summary of this revision. momo5502 added a comment. Herald added a subscriber: dexonsmith. A call to `isCompatibleWithMSVC` was added with the proper version that introduced the change (1925) and comments describing the intention

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. Is this a new feature in MSVC? Seems like it might be. If so, should it be predicated on `isCompatibleWithMSVC(1925)` or something? CHANGES SINCE LAS

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

rnk added a comment. I probably don't have time to review this, so let me redirect to @hans. Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:2400 +static llvm::GlobalValue *getTlsGuardVar(CodeGenModule &CGM) { + llvm::Type *VTy = llvm::Type::getInt8Ty(CGM.getLLVMContext())

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

momo5502 updated this revision to Diff 393247. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115456/new/ https://reviews.llvm.org/D115456 Files: clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/test/CodeGenCXX/ms-thread_local.cpp Index: clang/test/CodeGenCXX/ms-thread_local.cpp =

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

momo5502 created this revision. momo5502 added reviewers: rnk, eugenis. momo5502 added a project: clang. momo5502 requested review of this revision. Herald added a subscriber: cfe-commits. TLS initializers, for example constructors of thread-local variables, don't necessarily get called. If for e