[PATCH] D132450: StructurizeCFG: Set Undef for non-predecessors in setPhiValues()

[Ruiling, Song via Phabricator via llvm-commits]

ruiling added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/StructurizeCFG.cpp:687-689
+              // If this is not flow node and not the incoming block before
+              // structurization, then this block does not contribute any value
+              // to the phi.
----------------
sameerds wrote:
> This is the core idea. But it's not obvious how the code implements this. Would be good to have an explanation of the Stack and the Incomings set. For example, why do we need to erase Current from Incomings? Somewhere in the traversal, the Stack seems to ensure that undefs are added to phis at intermediate Flow blocks ... needs explanation of why it always works.
Sorry I don't quite get your point. The reason we need to erase Current from I


================
Comment at: llvm/lib/Transforms/Scalar/StructurizeCFG.cpp:688
+              // If this is not flow node and not the incoming block before
+              // structurization, then this block does not contribute any value
+              // to the phi.
----------------
sameerds wrote:
> On second thoughts, "this block" contributes an undefined value. That's the whole point of this change, right?
Yes, the core idea is the block existing before structurization but not the predecessor block does not contribute any value even after structurization. I am thinking about whether I can refine the code to be less confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132450