[PATCH] D83504: [PowerPC] Implement R_PPC64_REL24_NOTOC local calls. callee has a TOC

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: lld/ELF/Thunks.cpp:283
+// PPC64 R12 Setup Stub
+// When the caller does not use TOC and does not preserve R2 makes a local call
+// to a callee that requires a TOC pointer then we need this stub to place
----------------
minor nit: 
``` // When the caller does not use TOC and does not preserve R2 makes a local call to a callee that requires a TOC pointer ... ```
 --> 

``` // When a caller that does not maintain a toc-pointer  performs a local call to a callee which requires a toc-pointer ... ```


================
Comment at: lld/ELF/Thunks.cpp:838
+void PPC64R12SetupStub::writeTo(uint8_t *buf) {
+  int64_t offset = destination.getVA() - getThunkTargetSym()->getVA();
+  uint64_t prefix = 0x04100000 | ((offset >> 16) & 0x3ffff);
----------------
fatal error if the offset is to large to encode in the instruction.


================
Comment at: lld/ELF/Thunks.cpp:842
+
+  writePrefixedInstruction(buf + 0, (prefix << 32) | suffix); // pld r12, func at plt@pcrel
+  write32(buf + 8, 0x7d8903a6);                               // mtctr r12
----------------
Comment is wrong. `destination.getVA() - getThunkTargetSym()->getVA()` is the pc-relative offset to the function (`func at pcrel`), not the offset to the functions plt entry. 


================
Comment at: lld/ELF/Thunks.cpp:848
+void PPC64R12SetupStub::addSymbols(ThunkSection &isec) {
+  addSymbol(saver.save("__global_entry_point_setup_" + destination.getName()),
+                       STT_FUNC, 0, isec);
----------------
Good name. We could shorten to '__gep_setup' as I think anyone looking at a PPC64 binary is likely to understand gep is short for global entry point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83504