[PATCH] D86706: [LLD][PowerPC] Add pc-rel based long branch thunks

[Victor Huang via Phabricator via llvm-commits]

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


================
Comment at: lld/ELF/Thunks.cpp:375
+// call must be local (a non-local call will have a PCRelPltCallStub instead)
+// the table stores the address of the callee's local entry point. For
+// position-independent code a corresponding relative dynamic relocation is
----------------
sfertile wrote:
> IIUC If there is a call between a pc-rel based caller and a toc-based callee (so that callee has global and local entry points) then we will have emitted an `PPC64R12SetupoStub`. Likewise, if its a toc-based caller and pc-rel based callee we use an R2SaveStub. So its only between pc-rel based caller and callee that we can have a pc rel based long branch thunk.
Yes. PC-Rel based long branch thunk is needed only between pc-rel based caller and callee. 
- PC-Rel based caller and toc-based callee will use `PPC64R12SetupStub`
- Toc-based caller and pc-rel based callee will use `PPC64R2SaveStub`


================
Comment at: lld/ELF/Thunks.cpp:993
+
+void PPC64PCRelLongBranchThunk::writeTo(uint8_t *buf) {
+  int64_t offset = in.ppc64LongBranchTarget->getEntryVA(&destination, addend) -
----------------
sfertile wrote:
> The R12Setup stub calculates the address of the callee relative to the program counter.
> 
> ```
>   writePrefixedInstruction(buf + 0, paddi); // paddi r12, 0, func at pcrel, 1
>   write32(buf + 8, MTCTR_R12);              // mtctr r12
>   write32(buf + 12, BCTR);                  // bctr
> ```
> 
> The toc-based long branch thunks have to use a table in the data segment because of limited address-ability. The pc-rel instructions and ABI doesn't have the same limitation, so why use the branch linkage table intead of doing the same thing the R12Setup stubs do?
Good point! Will update the patch without using branch_lt table. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86706