[PATCH] D26005: AMDGPU: Don't use stack space for SGPR->VGPR spills

[Matt Arsenault via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIFrameLowering.cpp:117-118
+
   // Skip the last 2 elements because the last one is reserved for VCC, and
   // this is the 2nd to last element already.
+  for (MCPhysReg Reg : AllSGPR128s) {
----------------
nhaehnle wrote:
> The comment doesn't match the code anymore. Was the drop_back(2) removed on purpose? It seems like it would still be needed.
Yes, although I'm having trouble remembering why exactly. I think it should be redundant with the isReserved check


================
Comment at: lib/Target/AMDGPU/SIRegisterInfo.cpp:979-988
+    // If more SGPRs are required to support the input user/system SGPRs,
+    // increase to accomodate them.
+    //
+    // FIXME: This really ends up using the requested number of SGPRs + number
+    // of reserved special registers in total. Theoretically you could re-use
+    // the last input registers for these special registers, but this would
+    // require a lot of complexity to deal with the weird aliasing.
----------------
nhaehnle wrote:
> I wonder if this shouldn't be an error condition. After all, the frontend explicitly requested some maximum number of SGPRs, and now we break that request. It depends a bit on the use case for amdgpu-num-sgpr of course.
I'm not really sure about this. I think we agreed that the other higher level attributes are what people should be using instead of directly specifying the number of registers. These I think were left for compiler stressing, so it probably doesn't really matter


https://reviews.llvm.org/D26005