[PATCH] D80477: [LoopUnroll] Support loop with multiple exiting blocks

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:322
+              dyn_cast_or_null<BranchInst>(ExitingBlock->getTerminator())) {
+        if (ExitingBI->isUnconditional())
+          continue;
----------------
fhahn wrote:
> efriedma wrote:
> > An exiting block can't end in an unconditional branch.
> > 
> > In general, we don't know which exit the loop is actually going to use. Setting BI to refer to an arbitrary non-latch branch is suspicious; is it really necessary?
> > 
> > If you want to support loops with multiple exits, we need some appropriate testcases.
> > In general, we don't know which exit the loop is actually going to use. Setting BI to refer to an arbitrary non-latch branch is suspicious; is it really necessary?
> 
> IIUC the exiting block (without this patch either latch or header) is only used to optimize away the conditional branch to the exit block, if possible. This should be an optimizations only, but we should apply it to all exiting block which conditions could be eliminated based on the unroll count.  I've put up D80544 to hopefully make this more clearer in the code that actually handles connecting latches & headers.
> An exiting block can't end in an unconditional branch.

That's true, removed the check.

> In general, we don't know which exit the loop is actually going to use. Setting BI to refer to an arbitrary non-latch branch is suspicious; is it really necessary?

The code below expects one exiting block to optimize. We still give highest priority to the latch branch.

> If you want to support loops with multiple exits, we need some appropriate testcases.

Right, I agree more testcases are needed.  I was hoping to get some suggestions during the review process, which I actually got some, I will add those.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:339-340
   }
+  LLVM_DEBUG(dbgs() << "  Exiting Block = " << BI->getParent()->getName()
+                    << "\n");
 
----------------
Meinersbur wrote:
> If this supports any number of exits, why the need to find a special one?
The code expects one exiting block to be used to optimize away the conditional branch to the exit block.
As mentioned by Florain, ideally we should apply it to all exiting block which conditions could be eliminated based on the unroll count.
However, before this patch, if the latch is not one of the exiting blocks and there exists more than one exiting blocks, then it won't unroll.
Now, these kind of loops can still be unrolled, and one of the exiting block conditional branch is optimized. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80477