[PATCH] D64685: ELF: Add support for remaining R_AARCH64_MOVW* relocations.

Mon Jul 15 04:17:13 PDT 2019

peter.smith added a comment.

Functionally looks good. To my eyes it matches what is in the Arm ARM and ELF for the 64-bit architecture:

https://developer.arm.com/docs/ihi0056/latest/elf-for-the-arm-64-bit-architecture-aarch64-abi-2019q2-documentation
https://developer.arm.com/docs/ddi0487/latest/arm-architecture-reference-manual-armv8-for-armv8-a-architecture-profile

================
Comment at: lld/ELF/Arch/AArch64.cpp:257

+// Update the immediate field in an AArch64 movk, movn or movz instruction
+// for a signed relocation, and update the opcode of a movn or movz instruction
----------------

Really trivial nit, we use lower case in the comment before the function but upper case in the body. Is it worth choosing one? The rest of the file seems to mostly use lower case although there are some uses of MOVZ and MOVK in relaxTlsIeToLE.

================
Comment at: lld/ELF/Arch/AArch64.cpp:262
+  uint32_t inst = read32le(l);
+  if (!(inst & (1 << 29))) {
+    // MOVN or MOVZ.
----------------
To save a lookup in the Arm ARM, is it worth a quick comment along the lines:
// opcode field is bits 30, 29, with 10 = movz, 00 = movn and 11 = movk.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64685