[PATCH] D134949: [AMDGPU][SIFrameLowering] Use the right frame register in CSR spills

Scott Linder via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 14 13:48:50 PST 2022


scott.linder added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:1108
+      SB.save();
+      FuncInfo->SpillInfoSetIgnored(FramePtrReg, true);
+      LiveRegs.addReg(SGPRForFPSaveRestoreCopy);
----------------
I agree with others that this `Ignored` mode is hard to reason about, although now that I think I understand it I'm not certain I have a suggestion to improve it.

One thought I had was to try to make the case it is needed for "just work" by changing the order SGPR->SGPR spills occur relative to the FP setup. Is there a reason we can't just emit all of the `COPY_TO_SCRATCH_SGPR` spills before we set up FP? In that version the only special case remaining is for managing `FramePtrRegScratchCopy`, and I don't see any way to simplify that further.

The whole thing would look like:

```
<SGPR->SGPR spills>
if (<FP is spilled, and the kind of that spill is not COPY_TO_SCRATCH_SGPR>) {
  <Do the FramePtrRegScratchCopy scavenging>
}
<FP and SP setup>
<All remaining (i.e. non SGPR->SGPR) spills, respecting FramePtrRegScratchCopy>
```

For the downstream case, can we just erase the entry in the map after emitting the spill?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134949



More information about the llvm-commits mailing list