[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:415
+      if (L != CurrentLoop) {
+        // If CurrentLoop is not ancestor of L, then skip RN.
+        if (!L || !pushPathToAncestor(L->getParentLoop(), CurrentLoop,
----------------
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.


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

https://reviews.llvm.org/D79037