[PATCH] D83748: Adding support for PCRel32GOTLoad in ELF x86 for the jitlinker

[Jared Wyles via Phabricator via llvm-commits]

jaredwy marked 3 inline comments as done.
jaredwy added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp:35
+  bool isGOTEdge(Edge &E) const {
+    return E.getKind() == PCRel32GOT || E.getKind() == PCRel32GOTLoad;
+  }
----------------
MaskRay wrote:
> PCRel32GOT & PCRel32GOTLoad seem to match the names of Mach-O X86_64_RELOC_GOT & X86_64_RELOC_GOT_LOAD. The ELF relocation types are named R_X86_64_GOTPCREL & R_X86_64_GOTPCRELX
This was done on purpose, this code is lifted verbatim from the Mach-O side. By normalising the edges of the graph it allows us to write more generic code.

The next commit after this, will merge all this to a base class that both Mach-O and elf can use.



================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp:489
       for (auto SymRef : *Symbols) {
+        SymbolIndex++;
         auto Type = SymRef.getType();
----------------
MaskRay wrote:
> LLVM coding style uses `++SymbolIndex`
Thanks. Fixed up. 


================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp:637
+      int64_t Value = E.getTarget().getAddress() + E.getAddend();
+      *(support::little64_t *)FixupPtr = Value;
+      break;
----------------
MaskRay wrote:
> `llvm/Support/Endian.h` llvm::support::endian::write64le
Thanks, That actually can remove the comment above as well. 


================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp:208
   Section *CommonSection = nullptr;
+  std::string raph;
   // TODO hack to get this working
----------------
Not sure what this got left in or not flagged as unused :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83748