[PATCH] D155672: [JITLink][PowerPC] Correct handling of R_PPC64_REL24_NOTOC

[Lang Hames via Phabricator via llvm-commits]

lhames accepted this revision.
lhames added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks @lkail!



================
Comment at: llvm/include/llvm/ExecutionEngine/JITLink/ppc64.h:54-57
 extern const char PointerJumpStubContent_big[20];
 extern const char PointerJumpStubContent_little[20];
+extern const char PointerJumpStubNoTOCContent_big[32];
+extern const char PointerJumpStubNoTOCContent_little[32];
----------------
You could further unify some of the code paths using a pair of template variables:

```
template <support::endianness Endianness>
ArrayRef<char> PtrJumpStubContent;

template<>
ArrayRef<char> PtrJumpStubContent<support::endianness::big> =
    PtrJumpStubContent_big;
template<>
ArrayRef<char> PtrJumpStubContent<support::endianness::litttle> =
    PtrJumpStubContent_little;

template <support::endianness Endianness>
ArrayRef<char> PtrJumpStubNotTOCContent;

template<>
ArrayRef<char> PtrJumpStubNoTOCContent<support::endianness::big> =
    PtrJumpStubNoTOCContent_big;
template<>
ArrayRef<char> PtrJumpStubNoTOCContent<support::endianness::litttle> =
    PtrJumpStubNoTOCContent_little;
```

`pickStub`'s cases then look like:
```
template <support::endianness Endianness>
inline PLTCallStubInfo pickStub(PLTCallStubKind StubKind) {
  switch (StubKind) {
  case LongBranch: {
    auto Content = PointerJumpStubContent<Endianness>;
    // Skip save r2.
    Content = Content.slice(4);
    return PLTCallStubInfo{
        Content,
        {{TOCDelta16HA, 0, 0}, {TOCDelta16LO, 4, 0}},
    };
  }
  case LongBranchSaveR2: {
    auto Content = PointerJumpStub<Endianness>;
    return PLTCallStubInfo{
        Content,
        {{TOCDelta16HA, 4, 0}, {TOCDelta16LO, 8, 0}},
    };
  }
  case LongBranchNoTOC: {
    auto Content = PointerJumpStubNoTOCContent<Endianness>;
    return PLTCallStubInfo{
        Content,
        {{Delta16HA, 16, 8}, {Delta16LO, 20, 12}},
    };
  }
  }
}
...
```

If other clients are likely to use the stub content arrays this may be worth it. If everyone is going to use `pickStub` then I think it's fine either way. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155672