[PATCH] D114172: [ARM] implement support for ALU/LDR PC-relative group relocations

[Peter Smith via Phabricator via llvm-commits]

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

LGTM, thanks for working on this. I've marked approved from an Arm architecture perspective. Would also be good to wait a few days to see if @MaskRay is happy from a code-owner perspective.

For the benefit of other reviewers. A possible example of these relocations being used is https://github.com/ARM-software/abi-aa/blob/main/aaelf32/aaelf32.rst#73svr4-dso-like-plt-linkage in LLD we don't use the relocations directly and just hard code the positions.

The group relocations are defined in https://github.com/ARM-software/abi-aa/blob/main/aaelf32/aaelf32.rst#56relocation search for group relocations.



================
Comment at: lld/ELF/Arch/ARM.cpp:425
 
-// Rotate a 32-bit unsigned value left by a specified amt of bits.
-static uint32_t rotl32(uint32_t val, uint32_t amt) {
-  assert(amt < 32 && "Invalid rotate amount");
-  return (val << amt) | (val >> ((32 - amt) & 31));
+static unsigned getRemainderForGroup(unsigned group, uint32_t val,
+                                     uint32_t &rem) {
----------------
Was about to comment that this was more like getLeadingZerosForGroup until seeing the reference parameter. One possibility is to return std::pair<rem, lz> and then use something like:
```
std::tie<imm, lz> = getRemAndLZForGroup(group, val);
```
In the main body.


================
Comment at: lld/ELF/Arch/ARM.cpp:451
+    imm = rotr32(imm, 24 - lz);
+    rot = ((lz + 8) & 30) << 7;
+  }
----------------
IIUC I think the & 30 isn't strictly necessary as lz is always even when returned from getRemainderForGroup and lz + 8 can't be more than 30. No objections to keeping it in though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114172