[PATCH] D39386: [Power9] Allow gpr callee saved spills in prologue to vector registers rather than stack

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added a subscriber: MatzeB.
thegameg added inline comments.


================
Comment at: include/llvm/CodeGen/MachineFrameInfo.h:36
   int FrameIdx;
+  unsigned DstReg;
   /// Flag indicating whether the register is actually restored in the epilog.
----------------
How about

```
union {
  int FrameIdx;
  unsigned DstReg;
};
```

to make it more obvious that you can't have both?


================
Comment at: include/llvm/CodeGen/MachineFrameInfo.h:263
   /// epilog code inserter, this data used for debug info and exception
   /// handling.
   std::vector<CalleeSavedInfo> CSInfo;
----------------
This should also be updated to warn users that not all CSRs are saved in the frame (and that something like `CSInfo.size() * CSRSize` is not totally accurate anymore, even though I'm not sure if any part of the code relies on this to allocate / estimate stack size).


================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:1990
+            MachineBasicBlock *BB = &*I;
+            BB->addLiveIn(VolatileVFRegNo[i]);
+          }
----------------
I may be missing something, but why is this needed? Won't it be def'd anyway?


================
Comment at: lib/Target/PowerPC/PPCFrameLowering.h:102
                                  const TargetRegisterInfo *TRI) const override;
+  /// This function will assign callee saved grps to volatile vector registers
+  /// for prologue spills when applicable. If it cannot spill to a volatile
----------------
grps -> gprs


================
Comment at: test/CodeGen/PowerPC/prolog_vec_spills.ll:19
+  %1 = load i32, i32* %b.addr, align 4
+  %2 = call i32 asm "add $0, $1, $2", "=r,r,r,~{r14},~{r15},~{r16}"(i32 %0, i32 %1)
+  store i32 %2, i32* %dst, align 4
----------------
You can also write MIR tests with stuff like:

```
%R14 = IMPLICIT_DEF
%R15 = IMPLICIT_DEF
%R16 = IMPLICIT_DEF
```

to have more clear tests (with `-run-pass prologepilog`).


https://reviews.llvm.org/D39386