[PATCH] D117796: AMDGPU: Fix LiveVariables error after lowering SI_END_CF

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 21 08:20:01 PST 2022


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:516
+    if (SplitBB != &MBB) {
+      for (unsigned i = 0, e = MRI->getNumVirtRegs(); i != e; ++i) {
+        Register Reg = Register::index2VirtReg(i);
----------------
foad wrote:
> Would it make sense to move this code into splitAt, i.e. have it update LV just like it updates LIS?
Probably


================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:522
+          VI.AliveBlocks.set(SplitBB->getNumber());
+        else if (LV->isPHIJoin(Reg)) {
+          // Pass through live phis
----------------
foad wrote:
> This is a shame. isPHIJoin hasn't been used since rGfac770b865f59cbe615241dad153ad20d5138b9e 9 years ago and I was hoping to remove it.
Well we were hoping to remove LiveVariables completely, so I'm not worried about using a specific piece of it.


================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:526
+            if (Kill->getParent() == SplitBB)
+              VI.AliveBlocks.set(MBB.getNumber());
+        }
----------------
foad wrote:
> Why do you only need to do this for phi join regs? Couldn't any random register have its last use in SplitBB?
I derived this based on the phi representation described in the comment for VarInfo. Other registers should have the block already in AliveBlocks


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

https://reviews.llvm.org/D117796



More information about the llvm-commits mailing list