[PATCH] D112731: [AMDGPU] Really preserve LiveVariables in SILowerControlFlow

[Ruiling, Song via Phabricator via llvm-commits]

ruiling added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveVariables.cpp:732
+    MachineBasicBlock &UseBB = *MF->getBlockNumbered(UseBBNum);
+    // Only add kills in DefBB if all uses are in DefBB.
+    if (&UseBB == &DefBB && UseBlocks.count() != 1)
----------------
why do we need this?


================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:719-720
   Register CountReg = MRI->createVirtualRegister(&AMDGPU::SGPR_32RegClass);
   auto BfeMI = BuildMI(*MBB, FirstMI, DL, TII->get(AMDGPU::S_BFE_U32), CountReg)
                    .addReg(InputReg)
                    .addImm((MI.getOperand(1).getImm() & Mask) | 0x70000);
----------------
foad wrote:
> ruiling wrote:
> > Here I think the use of `InputReg` can be a possible kill. Actually we should inherit the possible KILL state from original pseudo instruction. and also replace the `Kills` of `InputReg`.
> The problem here is that we are inserting new instructions in a different place from the original  SI_INIT_EXEC_FROM_INPUT instruction. For example, before this pass:
> ```
> bb.0.main_body:
>   liveins: $sgpr0, $vgpr0
>   %1:vgpr_32 = COPY killed $vgpr0
>   %0:sgpr_32 = COPY killed $sgpr0
>   %3:vgpr_32 = V_ADD_U32_e32 %0:sgpr_32, killed %1:vgpr_32, implicit $exec
>   %4:vgpr_32 = V_CVT_F32_I32_e32 killed %3:vgpr_32, implicit $mode, implicit $exec
>   SI_INIT_EXEC_FROM_INPUT killed %0:sgpr_32, 19, implicit-def dead $exec
>   $vgpr0 = COPY killed %4:vgpr_32
>   SI_RETURN_TO_EPILOG killed $vgpr0
> ```
> Note that SI_INIT_EXEC_FROM_INPUT kills %0. But if we copy the kill flag to the S_BFE_U32 instruction we get this:
> ```
> bb.0.main_body:
>   liveins: $sgpr0, $vgpr0
>   %0:sgpr_32 = COPY killed $sgpr0
>   %5:sgpr_32 = S_BFE_U32 killed %0:sgpr_32, 458771, implicit-def $scc
>   $exec = S_BFM_B64 %5:sgpr_32, 0
>   S_CMP_EQ_U32 killed %5:sgpr_32, 64, implicit-def $scc
>   $exec = S_CMOV_B64 -1, implicit $scc
>   %1:vgpr_32 = COPY killed $vgpr0
>   %3:vgpr_32 = V_ADD_U32_e32 %0:sgpr_32, killed %1:vgpr_32, implicit $exec
>   %4:vgpr_32 = V_CVT_F32_I32_e32 killed %3:vgpr_32, implicit $mode, implicit $exec
>   $vgpr0 = COPY killed %4:vgpr_32
>   SI_RETURN_TO_EPILOG killed $vgpr0
> ```
> which is wrong, because %0 is used in the V_ADD_U32 instruction after the kill.
> 
> What do you think we should do here? Is it OK to drop the kill flag?
I did not notice that, re-computing like in this new version sounds good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112731