[PATCH] D35531: [COFF, ARM64] Reserve X18 register by default

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: lib/Target/AArch64/AArch64Subtarget.cpp:176
+      ReserveX18(TT.isOSDarwin() ||
+                (TT.isOSWindows() && TT.isOSBinFormatCOFF())),
       IsLittle(LittleEndian), TargetTriple(TT), FrameLowering(),
----------------
The `TT.isOSBinFormatCOFF()` part feels a little superfluous here


================
Comment at: test/CodeGen/AArch64/arm64-platform-reg.ll:1
 ; RUN: llc -mtriple=arm64-apple-ios -mattr=+reserve-x18 -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE-X18
 ; RUN: llc -mtriple=arm64-freebsd-gnu -mattr=+reserve-x18 -o - %s | FileCheck %s --check-prefix=CHECK-RESERVE-X18
----------------
Unrelated to this particular patch, but shouldn't we drop the `-mattr=+reserve-x18` here, to also test that darwin actuall does this right by default? (I'm not sure if there's any other tests that explicitly test that?)


================
Comment at: test/CodeGen/AArch64/arm64-platform-reg.ll:28
+
+; CHECK-RESERVE-X18-COFF-NOT: ldr x18
+; CHECK-RESERVE-X18-COFF-NOT: str x18
----------------
Why the custom test prefix for windows/coff? Doesn't the existing CHECK-RESERVE-X18 also work for aarch64-windows?


Repository:
  rL LLVM

https://reviews.llvm.org/D35531