[PATCH] D95806: [LoopUnrollAndJam] Do not allows loops which have no exit(ing) blocks or multiple exit(ing) blocks

[Sidharth Baveja via Phabricator via llvm-commits]

sidbav marked an inline comment as done.
sidbav added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp:842
+    // Only loops with a single exiting block can be unrolled and jammed.
+    if (!L->getExitingBlock()) {
+      LLVM_DEBUG(dbgs() << "Won't unroll-and-jam; only loops with single "
----------------
Whitney wrote:
> sidbav wrote:
> > Meinersbur wrote:
> > > sidbav wrote:
> > > > Meinersbur wrote:
> > > > > It is possible for a single exiting block to have multiple outgoing edges (likely with a switch terminator). Do you intend to allow this/is it checked somewhere else already?
> > > > Yes, this should be allowed (assuming such a loop is following all other unroll and jam requirements) as long as there is only 1 edge  going to an exit block. If a loop has multiple exit blocks, this will be caught in the above check.  
> > > The "there is only 1 edge going to an exit block" is exactly what I was wondering. Consider:
> > > ```
> > > header:
> > >   br label %exiting
> > > 
> > > exiting:
> > >    switch i32 %a, label %header [
> > >       i32 0, label %exit
> > >       i32 1, label %exit
> > >    ]
> > > 
> > > exit:
> > >   ret void
> > > ```
> > > 
> > > This loop has one exiting block, one exit block. but two exiting edges. My question is whether this is either handled correctly by UnrollAndJam or if there is something else checking for this.
> > > 
> > > `L->getExitingBlock()` does return `%exiting` in this case, but it looks like `L->getExitBlock()` returns nullptr for this case (`getUniqueExitBlock()` would return `%exit`), so seems I answered my own question.
> > This is similar test case which I described below (unique exit, but multiple exit edges)... this would not be handled.
> Can you edit the comment to indicate that we intentionally use getExitBlock to disallow multiple exit edges?
Added this comment in the committed change. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95806