[PATCH] D109293: [JITLink][WIP] Add initial native TLS support to ELFNix platform

Lang Hames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Sep 5 15:49:07 PDT 2021


lhames added a comment.

> To allocate the TLS descriptor in GOT, I need to get the edge kind information in PerGraphGOTAndPLTStubBuilder, So I add a Edge::Kind K argument in some functions in PerGraphGOTAndPLTStubBuilder.h. If it is not
> suitable, I can think further to solve this problem.

Is there a good reason to put these in the GOT? I would create and manage a different section for these, rather than re-using the GOT.

Side note: Re-using the GOT in MachO is safe, but was a bit lazy. We could probably come up with a generic TableSection<T> utility that we could re-use for GOTs, PLTs, and these new thread data structures.

In D109293#2984221 <https://reviews.llvm.org/D109293#2984221>, @MaskRay wrote:

> TLS descriptors refer to an alternative ABI to traditional general dynamic/local dynamic TLS models. The name cannot be repurposed to a different usage.

LinkGraphs are not ELF graphs so we're free to redefine terms to a certain extent, but I agree that it's good to avoid confusion where possible. Do you have a a suggested alternative?



================
Comment at: compiler-rt/lib/orc/elfnix_platform.cpp:113
+  Expected<std::pair<const char *, size_t>>
+  getTHreadDataSectionFor(const char *ThreadData);
+
----------------
The capitalization of THread should be fixed here. 


================
Comment at: compiler-rt/lib/orc/elfnix_platform.cpp:255
+Expected<std::pair<const char *, size_t>>
+ELFNixPlatformRuntimeState::getTHreadDataSectionFor(const char *ThreadData) {
+  std::lock_guard<std::mutex> Lock(ThreadDataSectionsMutex);
----------------
The capitalization of THread should be fixed here too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109293



More information about the llvm-commits mailing list