[PATCH] D74989: [LoopTerminology] Loop Simplify Form

[Nikita Popov via Phabricator via llvm-commits]

nikic 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
----------------
Meinersbur wrote:
> 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.
See also D72519...


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

https://reviews.llvm.org/D74989