This is an archive of the discontinued LLVM Phabricator instance.

[mac/lld] Fix scale computation for vector ops in PAGEOFF12 relocations
ClosedPublic

Authored by thakis on Mar 5 2021, 9:08 AM.

Details

Summary

With this, llvm-tblgen no longer tries and fails to allocate 7953 petabyte
when it runs during the build. Instead, check-llvm with lld/mac as host
linker now completes without any failures on an m1 mac.

This vector op handling code matches what happens in:

  • ld64's OutputFile::applyFixUps() in OutputFile.cpp for kindStoreARM64PageOff12
  • lld.ld64.darwinold's offset12KindFromInstruction() in lld/lib/ReaderWriter/MachO/ArchHandler_arm64.cpp for offset12scale16
  • RuntimeDyld's decodeAddend() in llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldMachOAArch64.h for ARM64_RELOC_PAGEOFF12

Fixes PR49444.

Diff Detail

Event Timeline

thakis created this revision.Mar 5 2021, 9:08 AM
Herald added a project: Restricted Project. · View Herald Transcript
thakis requested review of this revision.Mar 5 2021, 9:08 AM
thakis added inline comments.
lld/MachO/Arch/ARM64.cpp
130–137

(the type change in this line is inconsequential. All callers pass a uint32_t, it's conceptually a uint32_t, and on the return line it gets widened to an uin64_t. This part is a no-op, but it seemed semantically more correct -- without it, you'd technically have to do (base >> 30) & 3 if you didn't know that all callers passed in a widened uint32_t.)

int3 accepted this revision.Mar 5 2021, 9:21 AM

Nice fix!

lld/MachO/Arch/ARM64.cpp
132

TIL that C++ has digit separators

This revision is now accepted and ready to land.Mar 5 2021, 9:21 AM
This revision was landed with ongoing or failed builds.Mar 5 2021, 9:25 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2021, 9:25 AM
smeenai added inline comments.
lld/MachO/Arch/ARM64.cpp
134

If I'm reading the encoding table right (downloaded from https://developer.arm.com/documentation/ddi0487/ga/), then yup, this means a 128-bit operation.

int3 added inline comments.Mar 5 2021, 11:25 AM
lld/MachO/Arch/ARM64.cpp
134

I interpreted the question mark as "here we are checking if we have a vector op", not "I'm not entirely sure this is a vector op"... but I guess it's a bit confusing since line 132 doesn't have a question mark

thakis added inline comments.Mar 5 2021, 12:59 PM
lld/MachO/Arch/ARM64.cpp
134

Yes, that's how I meant it. This is "C7.2.191 LDR (immediate, SIMD&FP)" in the ARM ARM, "128-bit variant Applies when size == 00 && opc == 11."

I have to say I find https://developer.arm.com/documentation/ddi0487/ga/ pretty hard to read though. I ended up writing myself a decoder (first few lines of https://github.com/nico/mra_tools) to help me navigate that PDF. Maybe there's some easier way to answer "given 32-bit aarch64 instruction, find out what exactly it means", but that script works for me now that I've written it :)

thakis added inline comments.Mar 5 2021, 1:29 PM
lld/MachO/Arch/ARM64.cpp
134

I replaced the comment with "128-bit variant" in 7d26916859e9. That's the language used in the ARM ARM, and there are only 42 hits for that in it, and searching for that in it finds the relevant bits. Hopefully that's a less confusing comment :)

smeenai added inline comments.Mar 5 2021, 1:44 PM
lld/MachO/Arch/ARM64.cpp
134

Thanks!