[PATCH] D41605: StructurizeCFG: Fix broken backedge detection

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

Reading the old code you are replacing I don't feel like I understand the graph theory behind the algorithm.

Reading the new code and analyzeLoops, et al

It is true the way this is structured requires RPO to detect back edges properly, but FWIW: it's very unclear to me why it's done this way.
The visitation order is unrelated to the back-edgeness, despite what analyzeLoops thinks.

In particular, an edge x->y is a backedge in the CFG  if rpo[x] >= rpo[y].
Period.
Nothing else is necessary. The visitation testing, etc, is redundant. It just discovers this fact (and makes you require visitation in RPO order). You could create the set of backedges directly from the RPO computation you do, and in fact, during it, if you wanted to.  It is already computing backedges as it is visiting.

This may or may not be what you want to consider loops.  In particular, it is not maximal (even in the current code).  SCCs are maximal, and include cycles that are irreducible.

The trivial example cfg being:

       a
    /    \
   /      \
  b  <- >  c

(IE a->b, a->c, b->c, c->b)

b and c form a cycle, and an SCC.
whether you detect b->c or c->b as the backedge is a coin flip depending on whether you visit left subtree or right subtree first.
It is not a natural loop.

You would currently detect this as some form of loop, but again, most of the algorithms used here do not look like the ones i'm familiar with (I feel like i can find a ton of edge cases in it).

In any case, it is definitely true that RPO is required for backedge detection as the rest of this code is currently written.


https://reviews.llvm.org/D41605