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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 11 07:29:46 PST 2021


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:1065-1071
+    // FIXME LLVM may not know that VGPR is live in other lanes, we need to mark
+    // it as live, otherwise the MachineIR verifier complains.
+    // Only add this if VGPR is currently not live.
+    BuildMI(*MBB, MI, DL, TII->get(AMDGPU::INLINEASM))
+        .addExternalSymbol("")
+        .addImm(0) // flags
+        .addReg(VGPR, RegState::ImplicitDefine);
----------------
sebastian-ne wrote:
> arsenm wrote:
> > Definitely should not be introducing inline asm. Can't you just add an implicit def on the first instruction in the sequence, or introduce a special purpose pseudo?
> That sounds better, thanks. For stores, the first instruction is a `buffer_store VGPR, …`, so there is no previous instruction I can add a define to.
> For a pseudo, what would be the best pass to remove it again? (Maybe SIInsertWaitcntsPass?)
It doesn't need to be removed, it can just be emitted as a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96336



More information about the llvm-commits mailing list