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

[Maurice Heumann via Phabricator via cfe-commits]

momo5502 updated this revision to Diff 397107.
momo5502 added a comment.

In D115456#3217857 <https://reviews.llvm.org/D115456#3217857>, @majnemer wrote:

> 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.

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


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

https://reviews.llvm.org/D115456

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/ms-thread_local.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115456.397107.patch
Type: text/x-patch
Size: 14368 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220103/cac7f886/attachment-0001.bin>