[PATCH] D70937: [ELF][PPC64] Support long branch thunks with addends

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 5 inline comments as done.
MaskRay added a comment.

.got is not always created. .got will be created when there is at least .TOC. related relocation:

  // Relocations.cpp:1308
  } else if (oneof<R_GOTONLY_PC, R_GOTREL, R_PPC64_TOCBASE, R_PPC64_RELAX_TOC>(
                 expr)) {
    in.got->hasGotOffRel = true;
  }

Secondly, it works arounds a small issue that will not be fixed by this patch. If .rela.dyn has no entries before thunk creation.

  Writer<ELFT>::run
    Writer<ELFT>::finalizeSections
        forEachRelSec(scanRelocations<ELFT>) // Most dynamic relocations are added in this pass.
      removeUnusedSyntheticSections
        // The synthetic section .rela.dyn is removed from its parent (OutputSection .rela.dyn) because it has no entries.
      Writer<ELFT>::finalizeAddressDependentContent
        ThunkCreator::createThunks
          // Add a dynamic relocation to mainPart->relaDyn

This can affect all targets that add dynamic relocations in ThunkCreator::createThunks. Currently only PPC64 is affected, because only PPC64 needs .branch_lt to work around the lack of PC-relative addressing before Power ISA v3.0.

Fortunately, this pass ordering problem usually does not matter in practice. PPC64PILongBranchThunk is used by `config->isPic` cases. In PIC cases, there are almost assuredly some dynamic relocations here and there.



================
Comment at: lld/ELF/Relocations.cpp:511
   sym.gotIndex = old.gotIndex;
   sym.verdefIndex = old.verdefIndex;
   sym.exportDynamic = true;
----------------
sfertile wrote:
> On trunk I still see `sym.ppc64BranchltIndex = old.ppc64BranchltIndex;` here. I take it deleting that is meant to be part of this patch.
I intend to delete this line in a separate change.

`replaceWithDefined` is used by canonical PLT and copy relocations, which implies that the symbol is preemptable. `ppc64BranchltIndex` is only used by non-preemptable cases. So the line is not needed.


================
Comment at: lld/test/ELF/ppc64-long-branch-pi.s:86
+
+## Force creation of .got
+.section .data
----------------
sfertile wrote:
> Odd, I though we would always create a  .got section with the tocbase as the first entry. And the `ppc64-long-branch.s` doesn't seem to need it.  
```
## Force creation of .got
.section .data
.quad .TOC. at tocbase
```

in `ppc64-long-branch-pi.s` serves two purposes. I've put more information in the main comment (which is editable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70937