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

[Ehud Katz via Phabricator via llvm-commits]

ekatz marked an inline comment as done.
ekatz added inline comments.


================
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,
----------------
sameerds wrote:
> sameerds wrote:
> > 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.
> The second case is particularly important, where CurrentLoop is not an immediate parent of L. This situation happens when some node N in CurrentLoop has a successor S which is the header of L. If S was not the header, then L would become an irreducible region and hence not represented as a loop in LLVM. But that contradicts the assumption that CurrentLoop is not the parent of L. If there was a parent P of L (where P is itself a child of CurrentLoop), then P's header does not dominate S, and hence it is also an irreducible region. Again, any such P would not be represented in LLVM. I am not sure if I missed anything here ... if I am right, then the function pushPathToAncestor() would be a whole lot simpler.
That is a great observation!
Thanks!


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

https://reviews.llvm.org/D79037