[PATCH] D143759: [AMDGPU] Implement whole wave register spill

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

lgtm besides a few nits



================
Comment at: llvm/include/llvm/CodeGen/MachineRegisterInfo.h:60
     virtual void MRI_NoteNewVirtualRegister(Register Reg) = 0;
-    virtual void MRI_NotecloneVirtualRegister(Register NewReg,
                                               Register SrcReg) {
----------------
Didn't realize this already existed 


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:4985
+                              const DebugLoc &DL, Register Reg) const {
+  const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
+  const SIInstrInfo *TII = ST.getInstrInfo();
----------------
ST is already a member


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:4986
+  const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
+  const SIInstrInfo *TII = ST.getInstrInfo();
+  unsigned ExecMov = ST.isWave32() ? AMDGPU::S_MOV_B32 : AMDGPU::S_MOV_B64;
----------------
This is just this already, you don't need to query it


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:4987-4988
+  const SIInstrInfo *TII = ST.getInstrInfo();
+  unsigned ExecMov = ST.isWave32() ? AMDGPU::S_MOV_B32 : AMDGPU::S_MOV_B64;
+  MCRegister Exec = ST.isWave32() ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
+  BuildMI(MBB, MBBI, DL, TII->get(ExecMov), Exec).addReg(Reg, RegState::Kill);
----------------



================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2218-2219
+      if (IsWWMRegSpill)
+      TII->insertScratchExecCopy(*MF, *MBB, MI, DL, MFI->getSGPRForEXECCopy(),
+                                 RS->isRegUsed(AMDGPU::SCC));
       buildSpillLoadStore(
----------------
Indentation's off


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2225
+      if (IsWWMRegSpill)
+      TII->restoreExec(*MF, *MBB, MI, DL, MFI->getSGPRForEXECCopy());
+
----------------
Indentation's off


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143759