[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:412
+  Order.clear();
+  for (auto SCCI = scc_begin(ParentRegion); !SCCI.isAtEnd(); ++SCCI) {
+    const std::vector<RegionNode *> &SCC = *SCCI;
----------------
ekatz wrote:
> sameerds wrote:
> > Do we really need this for loop? If I understand correctly, orderSCC() works on a subgraph that is guranteed to not be an SCC. Can the same function just take whole region as the starting point?
> `orderSCC` does something pretty close, but works on `SubGraphNode`s, while this function works on `RegionNode`s. There is no need to create a subgraph if there is no SCC larger than 2, to work on.
This seems like a good use-case for the WrappedSuccIterator class introduced in LoopIterator.h ... that version limits the graph to the body of a loop, but you could build a similar iterator_adapter that limits the graph to a given set of nodes. With such a succ iterator, you could just eliminate the SubGraphNode class itself and let orderSCC do all the work on the entire input region. That will unify the two for-loops which are actually identical in their intention.


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

https://reviews.llvm.org/D79037