[PATCH] D83730: [AMDGPU] Spill CSR VGPR which is reserved for SGPR spills

Thu Jul 23 19:52:56 PDT 2020

kerbowa marked an inline comment as done.
kerbowa added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp:361
+  Optional<int> CSRSpillFI;
+  if ((FrameInfo.hasCalls() || !isEntryFunction()) && CSRegs &&
+      isCalleeSavedReg(CSRegs, LaneVGPR)) {
----------------
arsenm wrote:
> I think checking for isEntryFunction is redundant with checking for CSRegs. Why does this need to worry about calls? This will miss the tail call case.
> 
> Can you also comment this?
The logic is the same for other CSRegs being reserved for SGPR spills.

I'm not sure in what situation this might miss the tail call case? If there is a tail call and we don't clobber any SGPRs there will be no reserved VGPR and no unnecessary spill.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83730