[PATCH] D55498: [PPC64] Support R_PPC64_GOT16 Relocations

[Sean Fertile via Phabricator via llvm-commits]

sfertile added a comment.

In D55498#1339415 <https://reviews.llvm.org/D55498#1339415>, @martell wrote:

> This was already implemented in rL349511 <https://reviews.llvm.org/rL349511> and rL349772 <https://reviews.llvm.org/rL349772>.
>  Thanks @sfertile and sorry for the noise.
>  I don't know if there are any notes here that are useful. `--got-optimize` ?


Hi @martell, Thanks for creating this patch. If I was aware of it I would have skipped mine and helped review this one. I've left a few comments below related to some of your comments.



================
Comment at: ELF/Arch/PPC64.cpp:23
 
 static uint64_t PPC64TocOffset = 0x8000;
 static uint64_t DynamicThreadPointerOffset = 0x8000;
----------------
martell wrote:
> Any suggestions on a rename here?
> (It is no longer just toc but is got also)
I think we should update this name possibly to `PPC64TocBaseOffset` or `PPC64TocBaseBias` . TOC/toc is somewhat overloaded in the ABI, a clearer name makes it explicit that this is an offset used in building the TocBase value.


================
Comment at: ELF/Arch/PPC64.cpp:63
 
 uint64_t elf::getPPC64TocBase() {
   // The TOC consists of sections .got, .toc, .tocbss, .plt in that order. The
----------------
martell wrote:
> Ditto
I think this name is good as is. This calculates the address the toc pointer points at which is typically known as the tocbase. 


================
Comment at: ELF/Arch/PPC64.cpp:67
   // .got when we see a relocation that uses it, so for us the start is always
   // the .got.
   uint64_t TocVA = In.Got->getVA();
----------------
This comment is out of date. I'm guessing this description is for the V1 abi. The V2 abi is unfortunately somewhat ambiguous on what exactly is and isn't contained in the Table-Of-Contents.


================
Comment at: ELF/Arch/PPC64.cpp:600
 
 static bool isTocRelType(RelType Type) {
   return Type == R_PPC64_TOC16_HA || Type == R_PPC64_TOC16_LO_DS ||
----------------
martell wrote:
> I don't think I can just optimize got relocations like toc relocations
> but I'm not as familiar with the ISA / symbol groups as others.
> Is a `--got-optimize` useful?
> 
> Take `R_PPC64_ADDR16_HA`  for `R_PPC64_GOT16_HA` as an example
> 
> ```
> case R_PPC64_ADDR16_HA:
>     if (Config->GotOptimize && IsGotRelType && ha(Val) == 0)
>       writeInstrFromHalf16(Loc, 0x60000000);
>     else
>       write16(Loc, ha(Val));
>     break;
> ```
The optimization done here is strictly the transformation of a 2 instruction access sequence into a single instruction access sequence when the relocation offset fits in a single signed 16-bit immediate. It works similarly for got based relocation's as well as the toc based relocation (except it will work on the first 2 instructions of a 3 instruction got-indirect access sequence).  For example a got-indirect access where the high-adjusted part of the access is zero gets transformed as such:


```
addis  r3, r2, sym at got@ha     --> nop
ld     r3, sym at got@l(r3)      --> ld r3, sym at got@l(r2)
lwa    r3, 0(r3)              --> lwa r3, 0(r3)
```

As for naming, gold does all these optimization with the `--toc-optimize` flag (likely because the optimization's are related to the toc-pointer )


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D55498