[PATCH] D150381: [AMDGPU] Add optional tied-op for wwm-register's epilog spill restore

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:1683
+    // ...
+    if (!IsStore && MBB.isReturnBlock() && isReturnValueCCReg(*MI, SubReg)) {
+      MIB.addReg(SubReg, RegState::Implicit);
----------------
cdevadas wrote:
> arsenm wrote:
> > Should replace this with a liveness query instead 
> Using `LiveRegs` here won't help. The CSRs (including wwm-regs) are marked LiveOut by `initLiveRegs()` for an epilog block. In that sense, the liveness query will always hold true and we add the tied-op unconditionally for all spill reloads.
> These registers are, in fact, liveout only after the insertion of the spill-restore. But we are in the process of inserting one, and we need to look one level backward to get their actual liveness. `LiveRegs` in its present form won't be helpful for that.
> 
> The intention of this patch is to tie the return value register to its own spill restore (if have one) to avoid an incorrect optimization currently performed on it due to the presence of the wwm-restore inserted afterward. 
> Instead of walking through the operands of RETURN instruction, I would prefer if we have a utility function that returns a regmask or the list of return value registers allocated for this function. But not sure such a thing exists.
I'm having trouble following this. The live-outness is already represented by the $vgpr0 use on the return instruction. You're special casing looking for the return instruction in the low level return utility. Is this just to filter out spills in other contexts, and that's why the liveness query doesn't work? If you performed the liveness query in buildEpilogRestore and passed in that you needed the tied operand, would that let you use the liveness query?


================
Comment at: llvm/test/CodeGen/AMDGPU/tied-op-for-wwm-scratch-reg-spill-restore.mir:36
+    SI_RETURN implicit $vgpr0
+...
----------------
Can you add some additional tests with returned register tuples, and one where the def'd register is in the CSR range?


Also, test where the spill isn't used as the return value 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150381