[PATCH] D122922: [lld][common][lld-macho][lld-coff] Support per-thread allocators and StringSavers

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

Can someone summarize the high level roadmap of this feature? If threadsafe allocation is going to be optional/opt-in, lld can't generally rely on that, right? Or is it meant as a gradual way of merging the code and trying it out, and after it's ready to be enabled unconditionally, it can be relied upon in lld in general?

(So far I would expect that lld doesn't do any allocations from threaded code? And for the recent/upcoming refactorings to make lld usable as a library, the per-session context should hold the allocator, right?)

Secondly, about the choice of mechanism for the thread local data, I'm not familiar with the implications of `llvm::sys::ThreadLocal`, but for plain C++ `thread_local`, it's generally preferable if the thread local variable is made e.g. static within one single source file (with an accessor), instead of being declared in a header, accessed from any translation unit. If being accessed directly from multiple translation units, the use of the thread local variable incurs some amount of extra tls wrapper functions and weak symbols, which are occasionally broken in e.g. GCC on Windows. (See e.g. D111779 <https://reviews.llvm.org/D111779>, where a TLS variable in LLDB was moved this way, to fix GCC builds of it.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122922