[PATCH] D85080: [lld-macho] Support static linking of thread-locals

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: lld/MachO/Arch/X86_64.cpp:284
+
+    // Convert the movq to a leaq.
+    assert(isa<Defined>(&sym));
----------------
What do you think about moving this outside of this function?  This way this function remains a read-only operation which is what one would expect as being a getter.  Alternatively, you could rename the function if you think that this should be done in a single shot (I don't think that the access pattern is such that it would cause any performance implication).


================
Comment at: lld/MachO/Arch/X86_64.cpp:288
+      error("X86_64_RELOC_TLV must be used with movq instructions");
+    buf[-2] = 0x8d;
+    return sym.getVA();
----------------
This seems architecture specific.  Could you please base this on the architecture via a switch?


================
Comment at: lld/MachO/InputSection.h:42
+
+inline bool isThreadLocalVariables(uint8_t flags) {
+  return (flags & llvm::MachO::SECTION_TYPE) ==
----------------
nit: I think `isThreadLocalVariable` is a better name (the flag is checked on a single symbol)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85080