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

Vy Nguyen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 7 08:05:02 PDT 2022


oontvoo added inline comments.


================
Comment at: lld/Common/Memory.cpp:19
+// Pointer to this thread's context.
+thread_local PerThreadContext *threadContext = nullptr;
+} // namespace
----------------
aganea wrote:
> As things stand, you could also do `LLVM_THREAD_LOCAL PerThreadContext *threadContext = nullptr;`
> However since this is really just like a `static` it goes against the "*LinkerContext" concept, and since you already added some functionality in the context, you could just as well move this in `CommonLinkerContext`. You could do something like `ThreadLocal AllocContext;` (see llvm/include/llvm/Support/ThreadLocal.h). That would allocate a dynamic TLS slot on the first use, as opposed to `thread_local` which points to a pre-allocated slot in the PE static TLS table.
thanks! I've moved the variable to the LinkerContext class but wasn't able to use LLVM's ThreadLocal because I needed to hold on to the variable's address (to reset its value from another thread).

Would this "just work" with ThreadLocal?

ie:
```
ThreadLocal<AllocationContext> currentThreadCtxt;


std::vector<ThreadLocal<AllocationContext>> allThreadContexts;

// collecting all the thread-local ctxt's addresses
allThreadContext.push_back(&currentThreadCtxt);

// in destroy(), reset them to sentinel value (nullptr)
for (auto& localCtxt : allThreadContexts) {
  delete localCtxt.get();
  localCtxt = nullptr;
}

```



================
Comment at: lld/include/lld/Common/Memory.h:79
+// This is NOT thread safe.
+template <typename T, typename... U> T *makeUnsafe(U &&...args) {
+  return new (getSpecificAllocSingleton<T>().Allocate())
----------------
aganea wrote:
> Is there a need for "unsafe" or "perthread*"?
I didn't want to change all the ports to the thread-safe version because I'm not familiar with all of them. (and was only able to do benchmarking for the macho port).
If you all think it's "safe" to do this, then yeah, it'd simplify this patch a bit! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122922



More information about the llvm-commits mailing list