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

[Victor Huang via Phabricator via llvm-commits]

NeHuang marked 4 inline comments as done.
NeHuang added inline comments.


================
Comment at: lld/ELF/Thunks.cpp:841
+    fatal("offset must fit in 34 bits to encode in the instruction");
+  uint64_t prefix = 0x04100000 | ((offset >> 16) & 0x3ffff);
+  uint32_t suffix = 0xE5800000 | (offset & 0xffff);
----------------
nemanjai wrote:
> Would anyone object to naming these in an `enum`?
> Perhaps something like:
> ```
> enum PPCInstrMasks : uint64_t {
>   PLD_NO_DISP = 0x04100000E5800000,
>   MTCTR_R12 = 0x7D8903A6,
>   ...
> };
> ```
> in `llvm/include/llvm/Object/ELF.h`?
Thanks for the advice. `enum` added in `ELF.h` 


================
Comment at: lld/ELF/Thunks.cpp:850
+void PPC64R12SetupStub::addSymbols(ThunkSection &isec) {
+  addSymbol(saver.save("__gep_setup_" + destination.getName()),
+                       STT_FUNC, 0, isec);
----------------
nemanjai wrote:
> I do not object to the name, but if there is precedent in the GNU linker, we should match it for consistency and familiarity.
GNU linker ld uses `long_branch.callee` as the name, which also used for r2 save stub as mentioned in https://reviews.llvm.org/D82950 . I would go with `__gep_steup_` as it represents the thunk type. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83504