[PATCH] D74989: [LoopTerminology] Loop Simplify Form

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/docs/LoopTerminology.rst:152
+* A preheader.
+* A latch which must also be the only backedge.
+* Dedicated exists. That is, no exit block for the loop
----------------
baziotis wrote:
> Meinersbur wrote:
> > [serious] There can be a single latch with multiple edges to the header (e.g. a switch)
> Like this?
> ```
> 3:
>   // whatever
>   switch i32 %something, label %whatever [
>     i32 1, label %3
>     i32 2, label %3
>   ]
> ```
> In that case though, isn't the doc in `LoopSimplify.h` wrong?
> "This pass also guarantees that loops will have exactly one backedge."
> Assuming that a latch is a backedge, I figured it should be the only one, what do I miss?
The wording should clarify that whether the simplified form implies a single backedge or a single latch (with potentially multiple edges to the header as in your example). Having a look at the implementation:
```
bool Loop::isLoopSimplifyForm() const {
  // Normal-form loops have a preheader, a single backedge, and all of their
  // exits have all their predecessors inside the loop.
  return getLoopPreheader() && getLoopLatch() && hasDedicatedExits();
}

/// getLoopLatch - If there is a single latch block for this loop, return it.
/// A latch block is a block that contains a branch back to the header.
template <class BlockT, class LoopT>
BlockT *LoopBase<BlockT, LoopT>::getLoopLatch() const {
  assert(!isInvalid() && "Loop not in a valid state!");
  BlockT *Header = getHeader();
  BlockT *Latch = nullptr;
  for (const auto Pred : children<Inverse<BlockT *>>(Header)) {
    if (contains(Pred)) {
      if (Latch)
        return nullptr;
      Latch = Pred;
    }
  }

  return Latch;
}
```

And from LoopSimplify:
```
  BasicBlock *LoopLatch = L->getLoopLatch();
  if (!LoopLatch) {
    ...
    LoopLatch = insertUniqueBackedgeBlock(L, Preheader, DT, LI, MSSAU);
    if (LoopLatch)
      Changed = true;
  }
```

That is, it requires a single backedge (which implies a single latch). Your example is not in simplified form.

My wording "There can be a single latch with multiple edges to the header (e.g. a switch)" was not meant to imply that such a case is in simplified form, but that this is possible to happen, i.e. single backedge is not equivalent to single latch.


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

https://reviews.llvm.org/D74989