[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Neat use of tablegen; will take me a bit to wrap my head around it. I'll give this a shot on some kernel builds first thing tomorrow!



================
Comment at: llvm/include/llvm/CodeGen/MachineRegisterInfo.h:232-240
+  /// Returns true if a register can be used as an argument to a function.
+  bool isArgumentRegister(const MachineFunction &MF, MCRegister Reg) const;
+
+  /// Returns true if a register is a fixed register.
+  bool isFixedRegister(const MachineFunction &MF, MCRegister Reg) const;
+
+  /// Returns true if a register is a general purpose register.
----------------
are these methods on `MachineRegisterInfo` used anywhere? It looks like only the ones on `TargetRegisterInfo` are, IIUC?

Oh, are these related to the .td additions? Something seems asymmetric with these three. Like perhaps we only need `isFixedRegister` here?


================
Comment at: llvm/lib/CodeGen/MachineRegisterInfo.cpp:655-668
+bool MachineRegisterInfo::isArgumentRegister(const MachineFunction &MF,
+                                             MCRegister Reg) const {
+  return getTargetRegisterInfo()->isArgumentRegister(MF, Reg);
+}
+
+bool MachineRegisterInfo::isFixedRegister(const MachineFunction &MF,
+                                          MCRegister Reg) const {
----------------
are these methods on `MachineRegisterInfo` used anywhere? It looks like only the ones on `TargetRegisterInfo` are, IIUC?


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1216-1218
+    llvm::for_each(MF, [&](const MachineBasicBlock &MBB) {
+      llvm::for_each(MBB, [&](const MachineInstr &MI) {
+        llvm::for_each(MI.operands(), [&](MachineOperand MO) {
----------------
while I'm definitely a fan of the functional abstractions in llvm/include/llvm/ADT/STLExtras.h, `llvm::for_each` is perhaps my least favorite (when compared to `llvm::all_of`, `llvm::any_of`, or `llvm::none_of`). Consider using just a range-for here (and below) if it would be more concise.


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1257
+
+      for (auto MO : MI.operands()) {
+        if (!MO.isReg())
----------------
```
for (const MachineOperand &MO : MI.operands()) {
```


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1269
+  const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering();
+  for (auto RestoreBlock : RestoreBlocks)
+    TFI.emitZeroCallUsedRegs(RegsToZero, *RestoreBlock);
----------------
```
for (MachineBasicBlock *RestoreBlock : RestoreBlocks)
```


================
Comment at: llvm/lib/Target/X86/X86RegisterInfo.cpp:621
 
+BitVector X86RegisterInfo::getArgumentRegs(const MachineFunction &MF) const {
+  const X86Subtarget &Subtarget = MF.getSubtarget<X86Subtarget>();
----------------
pengfei wrote:
> Can we get this info from calling conversion?
> Different calling conversions may use different argument registers.
when working on the ppc issue recently (https://reviews.llvm.org/D116424), I recall there being an existing check for whether a register was callee saved.

RegisterClassInfo::getLastCalleeSavedAlias

Can that be reused here or in any of the below?

Do we need to zero stack slots when arguments are passed on the stack (after exhausting the "argument registers?"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869