[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:695-699
+    if (UseMI.isPHI()) {
+      // If Reg is used in a phi then it is live-out of the corresponding
+      // predecessor.
+      unsigned Idx = UseMI.getOperandNo(&UseMO);
+      LiveOutBlocks.push_back(UseMI.getOperand(Idx + 1).getMBB());
----------------
foad wrote:
> ruiling wrote:
> > This is not quite accurate. if the user is a PHI. we need to handle differently if the user appears in the same block as the define block.
> > 
> > For the pattern drawn at line 148 (incoming block is the same as define block), we don't treat the Reg is alive through the incoming block, it is only alive from the definition to the end. If the incoming block is different from define block, we need to mark the Reg is alive through the incoming block.
> I think my code is correct. I am only saying it is live-**out** of the corresponding predecessor, not live-through. The "Iterate ..." loop below handles the case where Reg is live-out of DefBB.
I see the problem, your interpretation of `live-out` does not match what it means in LLVM. You can take a look at `LiveVariables::isLiveOut()`. We are not saying that a register is live-out of a block if it is only used by a PHI in successor block. I would suggest renaming `LiveOutBlocks` to `LiveToEndBlocks`. So we don't have conflicting interpretations of `live-out`. Please also fix the use of `live-out` in comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112731