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

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In D115456#3217595 <https://reviews.llvm.org/D115456#3217595>, @momo5502 wrote:

> In D115456#3216811 <https://reviews.llvm.org/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 <https://github.com/llvm/llvm-project/blob/1f07a4a5699b73582461880e716e6692cbe3d6a6/clang/lib/CodeGen/ItaniumCXXABI.cpp#L391-L392> 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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115456/new/

https://reviews.llvm.org/D115456