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

[Florian Hahn via Phabricator via llvm-commits]

fhahn requested changes to this revision.
fhahn added a comment.
This revision now requires changes to proceed.

In D80477#2054279 <https://reviews.llvm.org/D80477#2054279>, @Meinersbur wrote:

> Could you clarify what "multi-exiting blocks" means?


I think it is supposed to refer to loops with "multiple exiting blocks". Might be good to update/clarify

> The test case if for the exiting block being neither the header nor the latch. If any other restriction is lifted, please add a test case for it.

Yes I think this would need a range of tests with various scenarios with multiple exiting blocks (with/without  constant exit take counts).

IIUC the patch currently aims to remove 2 restrictions: 1) single exiting block required and 2) exiting block must be either header or latch. It might be good to lift those 2 restrictions in separate patches.



================
Comment at: llvm/lib/Transforms/Utils/LoopUnroll.cpp:322
+              dyn_cast_or_null<BranchInst>(ExitingBlock->getTerminator())) {
+        if (ExitingBI->isUnconditional())
+          continue;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80477