[PATCH] D124196: [AMDGPU][SILowerSGPRSpills] Spill SGPRs to virtual VGPRs

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:1992
+        BuildMI(MBB, MBBI, DL, TII->get(OrSaveExec), Reg).addImm(-1);
+    SaveExec->getOperand(3).setIsDead(); // Mark SCC as dead.
+  }
----------------
Pierre-vh wrote:
> cdevadas wrote:
> > Pierre-vh wrote:
> > > Why does SCC need to be dead? What happens if another instruction right after uses it?
> > The code here is only to manipulate exec mask and no other instruction depends on the SCC that it produces, and we should mark it dead to avoid unwanted side effects. We don't have an alternate instruction that doesn't clobber SCC.
> Ah that makes sense, but shouldn't this check that it's not inserting in a place where SCC is alive?
> I was trying out this patch and I have a case where it's causing issues:
> ```
>   S_CMP_EQ_U32 killed renamable $sgpr6, killed renamable $sgpr7, implicit-def $scc
>   renamable $vgpr0 = V_WRITELANE_B32 killed $sgpr4, 4, $vgpr0(tied-def 0), implicit-def $sgpr4_sgpr5, implicit $sgpr4_sgpr5
>   renamable $vgpr0 = V_WRITELANE_B32 killed $sgpr5, 5, $vgpr0(tied-def 0), implicit killed $sgpr4_sgpr5
>   $sgpr10_sgpr11 = S_OR_SAVEEXEC_B64 -1, implicit-def $exec, implicit-def dead $scc, implicit $exec
>   $agpr4 = V_ACCVGPR_WRITE_B32_e64 killed $vgpr0, implicit $exec
>   $exec = S_MOV_B64 killed $sgpr10_sgpr11
>   S_CBRANCH_SCC1 %bb.5, implicit killed $scc
> ```
> 
> Insertion is between the S_CMP and the S_CBRANCH.
Yes, the check is already in place. See the code above, the if condition, that inserts two separate move instructions when SCC is live and the else part uses SCC when it is free.
Not sure why RegScavenger returned false. It should have returned SCC as clobbered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124196