[PATCH] D79037: [StructurizeCFG] Fix region nodes ordering

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/StructurizeCFG.cpp:393
+    // Keep processing loops while only going deeper (into inner loops).
+    do {
+      assert(I != POT.rend());
----------------
ekatz wrote:
> sameerds wrote:
> > Why not just have `while (CurrentLoopSize)` here? At least from what I could see, the value should be non-zero when this point is reached from above, right?
> The problem is that `CurrentLoopSize` may be null in case all of the nodes have a parent loop (none outside from every loop).
> In that case, `CurrentLoopSize` will only be valid from line 408; and remain that way all the way until the end.
I am not sure I follow. Can you please rephrase? An example might be useful.


================
Comment at: llvm/lib/Transforms/Scalar/StructurizeCFG.cpp:415
+      if (L != CurrentLoop) {
+        // If CurrentLoop is not ancestor of L, then skip RN.
+        if (!L || !pushPathToAncestor(L->getParentLoop(), CurrentLoop,
----------------
This line needs at least two tests: one where CurrentLoop is not an ancestor of L, and another where CurrentLoop is an ancestor of L, but not its immediate parent.


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

https://reviews.llvm.org/D79037