[PATCH] D35544: [COFF, ARM64] Force ADRP relocations

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp:585
+                             const MCValue &Target) override {
+    // The same reason as in ELFAArch64AsmBackend above
+    if ((uint32_t)Fixup.getKind() == AArch64::fixup_aarch64_pcrel_adrp_imm21)
----------------
mgrang wrote:
> Period at the end of comment.
I'll move the ELFAArch64AsmBackend implementation up to the superclass, so this comment will go away altogether.


================
Comment at: test/MC/AArch64/adrp-relocation-coff.s:1
+// RUN: llvm-mc -triple=aarch64-windows -filetype=obj -o - %s| llvm-readobj -r - | FileCheck %s
+        .text
----------------
mgrang wrote:
> There's already a test case for coff relocations: test/MC/AArch64/coff-relocations.s. Would you mind adding to the same file instead of creating a new one just for adrp relocs?
Sure, I can do that. Although since this fix is getting generalized for MachO as well, I'd like to test that as well. Should I try to add that into some MachO specific file as well, or have an adrp specific test file? The current one (adrp-relocation.s) tests some relocations that I don't think all are supported in COFF at least, which was why I tried to create a new one here.


https://reviews.llvm.org/D35544