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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added inline comments.


================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:42
+
+static const MCPhysReg VolatileVFRegNo[] = {
+    PPC::F0,   PPC::F1,   PPC::F2,   PPC::F3,   PPC::F4,   PPC::F5,   PPC::F6,
----------------
I am really not a fan of defining this here. This should be available from the .td files. Can we not get it from there?


================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:1982
+    if (PPC::G8RCRegClass.contains(Reg) || PPC::GPRCRegClass.contains(Reg)) {
+      for (unsigned i = VolatileVFRegNoIdx; i < 34; ++i) {
+        if (!MRI.isPhysRegUsed(VolatileVFRegNo[i])) {
----------------
No magic numbers please. If you are using the static array you defined above, you can insert a marker at the end and loop until you get to it (perhaps `PPC::NoRegister`)


================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:1993
+    // If CSR not spilled to volatile vector register, assign a frame index.
+    if (!CS.isSpilledToReg()) {
+      int FrameIdx;
----------------
I'm just curious about this entire section. I imagine this is just a copy of the generic implementation. If that is the case, can we just return `false` from this function and modify the generic implementation to just ignore any that are `isSpilledToReg()` (assuming the target has already done that)?
Perhaps if we do that, the generic code can also assert that the `DestReg` is set for any such CSI objects.


https://reviews.llvm.org/D39386