[PATCH] D72519: [LoopInfo] Support multi edge in getLoopLatch()

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

I think this may break a fairly major assumption that there's a single backedge if there is a latch block.  As a simple example, here's the comment from LoopSimplify:

  // If the header has more than two predecessors at this point (from the
  // preheader and from multiple backedges), we must adjust the loop.
  BasicBlock *LoopLatch = L->getLoopLatch();
  if (!LoopLatch) {

The entire point of this code is to insert a single backedge for the canonicalized loop.

I'm open to being convinced that the change in canonical form to allow multiple identical backedges is reasonable, but I'd expect to see an argument to that form, not simple a code change.

I took a quick skim through IndVars, and there were initially some patterns which looked concerning around values on incoming edges until I remembered that duplicate edges require identical values in phis.  But I could see consumer code which didn't properly account for that, so an audit is probably warranted.

For instance, it looks like anything using removeIncomingValue(BB, ...) is probably wrong after this change.  A quick skim of LoopUnroll turned up at least one suspicious place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72519