[PATCH] D70379: [AMDGPU] Reserving VGPR for future SGPR Spill

Fri May 8 08:33:07 PDT 2020

arsenm added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp:315

-    if (VGPRIndex == 0) {
+    if (VGPRIndex == 0 && !FuncInfo->VGPRReservedForSGPRSpill) {
       LaneVGPR = TRI->findUnusedRegister(MRI, &AMDGPU::VGPR_32RegClass, MF);
----------------
kerbowa wrote:
> saiislam wrote:
> > cdevadas wrote:
> > > Doesn't it limit the total allowable SGPR spills to 64? 
> > > What happens if more than 64 callee-saved SGPRs used in the functions?
> > Yes, this patch is limited to 64 SGPR spills only.
> > Does it make sense to handle that case in a separate patch?
> It seems like there is a fallback to spilling to VMEM if there are no free lanes. Doesn't this also limit the number of SGPR to VGPR spills to 32 on Navi?
That fallback is 90% broken. This is ultimately still a workaround for the fact that SGPRs and VGPRs are allocated at the same time, so we can run out of VGPRs before we see all the SGPR spills that need to be handled

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70379