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

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added a comment.

In D122922#3467647 <https://reviews.llvm.org/D122922#3467647>, @aganea wrote:

>> Right, but if the MachO part of lld does allocations from multiple threads, then it's essentially unreliable unless this option is enabled? So anyone building lld wanting to use the MachO part of it would need to enable it.
>
> This question was left unanswered -- how do you plan on using `LLD_THREAD_SAFE_MEMORY`? Will it be enabled only when building for Darwin? What happens for cross-compilation or if `LLD_THREAD_SAFE_MEMORY` isn't enabled? The make/makePerThread/makeUnsafe APIs seem a bit error-prone to me. Ideally it is a choice that should be avoided, if possible, by the business logic/driver developer.

Sorry, forgot to follow up on this. From offline chat with int3, I think decided to remove this and to just use the existing `LLD_ENABLE_THREADS` instead. That is to say, code would have thread safe saver/make() by default, unless it goes out of its way to call the unsafe() variants.

It also wasn't clear from @MaskRay's comment whehter the 1% slow down was for llvm::ThreadLocal alone or if it  also applied to the native TLS approach. Was not able to reproduce the regression from my ends with either approach, and I didn't want to keep updating the patch .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122922