[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:682
+    // The called routine is expected to preserve r19-r28
+    // r29 and r30 are used as frame pointer and link register resp.
+    return 0;
----------------
What happens if `-fomit-frame-pointer` is specified? Is X29 used as a GPR then?


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:757
+                                                MachineBasicBlock &MBB) const {
+  const MachineFunction &MF = *MBB.getParent();
+
----------------
sink this closer to first use, L767


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:776-778
+      // For GPRs, we only care to clear out the 64-bit register.
+      if (MCRegister XReg = getRegisterOrZero(Reg))
+        GPRsToZero.set(XReg);
----------------
so for 32b registers, we clear the whole 64b register?


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:787-792
+  for (MCRegister Reg : GPRsToZero.set_bits())
+    BuildMI(MBB, MBBI, DL, TII->get(AArch64::MOVi64imm), Reg).addImm(0);
+
+  // Zero out FP/vector registers.
+  for (MCRegister Reg : FPRsToZero.set_bits())
+    BuildMI(MBB, MBBI, DL, TII->get(AArch64::MOVID), Reg).addImm(0);
----------------
isn't it more canonical on ARM to move from the dedicated zero register XZR rather than use an immediate?


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.td:1404
+}
+def FPR32_ARG : RegisterClass<"AArch64", [f32, i32], 32, (trunc FPR32, 7)>;
+def FPR64_ARG : RegisterClass<"AArch64",
----------------
is `i32` correct here?


================
Comment at: llvm/lib/Target/X86/X86RegisterInfo.cpp:659
 
-  return false;
+  return X86GenRegisterInfo::isArgumentRegister(MF, Reg);
 }
----------------
Does this allow us to clean up anything else in the body of this method?

Consider making this and the tablegen related patch a distinct child patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836