[PATCH] D96336: [AMDGPU] Save VGPR of whole wave when spilling

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:1122
+    MachineInstrBuilder LastI;
+    for (unsigned I = 0; I < 2; I++) {
+      // Mark the second store as kill
----------------
arsenm wrote:
> I believe you don't need to split this when using scratch instructions
With scratch instructions, the code looks like this:
```
scratch_store_dword_saddr v0, s33, …
s_not_b32 exec_lo, exec_lo
scratch_store_dword_saddr v0, s33, …
s_not_b32 exec_lo, exec_lo
```

scratch instructions obey the exec mask, so I don’t think we can fuse this.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:1139-1140
+                                             RegScavenger *RS, bool IsLoad,
+                                             bool UseKillFromMI,
+                                             bool IsKill) const {
+  MachineBasicBlock *MBB = MI->getParent();
----------------
arsenm wrote:
> I don't follow this UseKillFromMI vs. isKill. Just use the isKill?
Fixed, should be more obvious now.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:1348
+    // Check if TmpVGPR is currently live according to LLVM liveness info
+    RegScavenger TmpRS;
+    TmpRS.enterBasicBlock(*MBB);
----------------
arsenm wrote:
> There shouldn't be any temporary reg scavenger created locally. Also, using forward scavenging is deprecated
I fixed that in D96517. It it should be part of this patch, I can move it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96336