[AMDGPU] Remove return VGPRs from callee save list

[critson]

Summary

There is no need to generate spill/restore for registers used in
return value. This matters for amdgpu_gfx calling convention
where CSR and Ret definitions overlap.

[critson]

Rebase on top of pre-commit test.

[critson]

As pointed out by @sebastian-ne the other way to solve this is to remove the overlap of CSRs and return registers in amdgpu_gfx.

[jsilvanus]

llvm/lib/Target/AMDGPU/SIFrameLowering.cpp

1527

"[...] because these do not [...]"?

1529

"[...] of equal type / size"?

Maybe add an assertion above when finding multiple returns?

[critson]

• Fix comment
• Add assertion that all returns are of equal size

[jsilvanus]

Thanks for the update. Looks good to me, but I'm not very familiar with this, so I'm leaving approval for someone else.

[cdevadas]

llvm/lib/Target/AMDGPU/SIFrameLowering.cpp

1521

I assume this is only a short-term fix until revising the gfx ABI to move return VGPRs into the scratch regs range to avoid the CSR spills/restores.

[sebastian-ne]

LGTM if there are no further concerns from @cdevadas.

Just to check, I suppose calls to functions with that many return values are already handled fine?

llvm/lib/Target/AMDGPU/SIFrameLowering.cpp

1521

I think this is a long-term solution :)
Callee-saves have the striped pattern, which is good and what we want.
But if a function decides that it wants to return a lot of values – and they fit into registers – then why not use these registers for the return value, even if they are callee-save (just that they cannot be callee-save for this function return).

[cdevadas]

In D152892#4421104, @sebastian-ne wrote:

LGTM if there are no further concerns from @cdevadas.

No other concerns.

llvm/lib/Target/AMDGPU/SIFrameLowering.cpp

1521

Ok.

[critson]

In D152892#4421104, @sebastian-ne wrote:

Just to check, I suppose calls to functions with that many return values are already handled fine?

As far as I can tell, calls are working fine as-is.

llvm/lib/Target/AMDGPU/SIFrameLowering.cpp

void SIFrameLowering::determineCalleeSaves(MachineFunction &MF,
   TargetFrameLowering::determineCalleeSaves(MF, SavedVGPRs, RS);
   SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
   if (MFI->isEntryFunction())
     return;
 
   const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
   const SIRegisterInfo *TRI = ST.getRegisterInfo();
 
+  MachineInstr *ReturnMI = nullptr;
   for (MachineBasicBlock &MBB : MF) {
     for (MachineInstr &MI : MBB) {
       // WRITELANE instructions used for SGPR spills can overwrite the inactive
       // lanes of VGPRs and callee must spill and restore them even if they are
       // marked Caller-saved.
 
       // TODO: Handle this elsewhere at an early point. Walking through all MBBs
       // here would be a bad heuristic. A better way should be by calling
       // allocateWWMSpill during the regalloc pipeline whenever a physical
       // register is allocated for the intended virtual registers. That will
       // also help excluding the general use of WRITELANE/READLANE intrinsics
       // that won't really need any such special handling.
       if (MI.getOpcode() == AMDGPU::V_WRITELANE_B32)
         MFI->allocateWWMSpill(MF, MI.getOperand(0).getReg());
       else if (MI.getOpcode() == AMDGPU::V_READLANE_B32)
         MFI->allocateWWMSpill(MF, MI.getOperand(1).getReg());
+      else if (MI.getOpcode() == AMDGPU::SI_RETURN ||
+               MI.getOpcode() == AMDGPU::SI_RETURN_TO_EPILOG)
+        ReturnMI = &MI;
+    }
+  }
+
+  // Remove any VGPRs used in the return value these do not need to be saved.
+  // This prevents CSR restore from clobbering return VGPRs.
+  // Note: this assumes all return instructions are of equal.
+  if (ReturnMI) {
+    for (auto &Op : ReturnMI->operands()) {
+      if (Op.isReg())
+        SavedVGPRs.reset(Op.getReg());
     }
   }
 
   // Ignore the SGPRs the default implementation found.
   SavedVGPRs.clearBitsNotInMask(TRI->getAllVectorRegMask());
 
   // Do not save AGPRs prior to GFX90A because there was no easy way to do so.
   // In gfx908 there was do AGPR loads and stores and thus spilling also