[PATCH] D40874: [LV][LoopInfo] Add irreducible CFG detection for outer loops

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added a comment.

Hi Daniel,

thanks for the detailed response!

The main goal of this patch is to extend the current irreducibility detection in LV, based on SCCs explicitly built for that purpose, to work on outer loops. We found the algorithm in ShrinkWrap and decided to generalize it to promote and reuse existing code in LLVM instead of re-implementing a LV-specific version.

> can you help me understand the algorithm?
>  IE What info you expect the loopinfo to provide vs the topo ordering.

LoopInfo analysis is “indirectly” computing CFG irreducibility by detecting irreducible loops and leaving them out of the loop forest. In the proposed algorithm, LoopInfo provides information about (already computed) reducible loop backedges. When a backedge is found during the CFG traversal, the algorithm checks that the backedge is one of the reducible backedges in LoopInfo. Otherwise, the CFG is irreducible. This logic is in function 'isProperBackedge'.

> What do you expect to happen here? What do you expect the loopinfo to provide vs the RPO.

For this example, LoopInfo wouln't contain any loop, so the algorithm will return that the CFG is irreducible when checking the B <- -> C backedge.

> It's true you can use loop info, but given the dom test is O(1) i'm not sure why it's worth computing to test irreducibility, especially if you are doing an RPO computation anyway.

Please, note that this patch is not intended to provide the best irreducibility algorithm for any arbitrary client. In both ShrinkWrap and LV, LoopInfo is an already required analysis. The rationale behind the proposed algorithm is that we don't want to compute irreducibility detection again when this information is already available in LoopInfo. This keeps the algorithm simple. I agree that the current documentation in 'containsIrreducibleCFG' might be confusing. Computing LoopInfo from scratch only to determine the irreducibility of a CFG might not be the best approach but if it's already computed, which is the case in ShrinkWrap and LV, I don't see why this approach is worse than using DT. I can improve the documentation in that regard.

Please, also note that the proposed algorithm also allows to reuse an existing RPO traversal, which is also important in terms of performance.

If a more generic algorithm is necessary for more general cases, I would suggest addressing that in a different patch. However, I think there is no silver bullet here. For a DT-based implementation one could argue that LoopInfo could be available and DT could have been invalidated and would need to be recomputed.

Pinging @qcolombet, the original author of this algorithm in ShrinkWrap, in case he wants to provide further information.

Does it sound reasonable?

Thanks, 
Diego


https://reviews.llvm.org/D40874