[PATCH] D35792: [lld] [COFF, ARM64] Handle ADRP immediate offsets in relocations

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: COFF/Chunks.cpp:202
   uint32_t Orig = read32le(Off);
   uint32_t Size = Orig >> 30;
   // 0x04000000 indicates SIMD/FP registers
----------------
ruiu wrote:
> So this is more like a scale than a size?
> 
> (Is there any documentation about this relocation?)
Yes, it's actually a shift/scale. I picked the name `Size` originally, because that's the name of the bits in the opcode, from the ARMv8 architecture reference manual.

I don't know of any documentation of the relocation itself - the official pecoff document only lists the relocation names/numbers, but no description of what they should do. This code is based on testing how link.exe handles them.


================
Comment at: COFF/Chunks.cpp:211-213
+  Imm >>= Size;
+  Orig &= ~(0xFFF << 10);
+  write32le(Off, Orig | (Imm << 10));
----------------
ruiu wrote:
> You are shifting Imm to the right and then to the left, which looks a bit odd.
I can get reduce the amount of shifting by doing the calculation in scaled units instead of in bytes - I did that for clarity, but perhaps it only obscured things.

In that case, it's probably simplest to extend the existing `applyArm64Imm` with a parameter that limits the wraparound range/mask from `0xFFF` to `0xFFF >> Scale` - that's the actual effect of this change.


https://reviews.llvm.org/D35792