[PATCH] D97747: [LoopUnrollRuntime] Add option to assume the non latch exit block to be predictable.

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp:491-493
+  if (ExitingBlocks.size() <= UnrollRuntimeCutoffPoint &&
+      OtherExits.size() < UnrollRuntimeCutoffPoint)
+    return true;
----------------
Meinersbur wrote:
> I think @bmahjour's suggestion (and mine) was to completely disable UnrollRuntimeCutoffPoint if it is `0`
> ```
> if (UnrollRuntimeCutoffPoint == 0 ||
>       (ExitingBlocks.size() <= UnrollRuntimeCutoffPoint &&
>       OtherExits.size() < UnrollRuntimeCutoffPoint))
>     return true;
> ```
> 
> I don't really understand why it bounds both, ExitingBlock and OtherExists (instead of only one of them or the sum of such blocks). Could you explain?
> 
> @bmahjour's other point is that it makes `UnrollRuntimeMultiExit` redundant: `-unroll-runtime-multi-exit=true` is equivalent to `-unroll-runtime-cutoff-point=0` and  -unroll-runtime-multi-exit=false` is equivalent to `-unroll-runtime-cutoff-point=1` (I think in this case I think `canProfitablyUnrollMultiExitLoop` would not be called anyway). Do I understand correctly that you would remove the `-unroll-runtime-multi-exit` in a followup-patch?
Current behaviour:
-unroll-runtime-multi-exit==true => return true
-unroll-runtime-multi-exit==false => return false
-unroll-runtime-multi-exit not specified => return (ExitingBlocks.size() <= 2 && OtherExits.size() == 1 && OtherExits[0]->getTerminatingDeoptimizeCall())

If we use this code:
```
if (UnrollRuntimeCutoffPoint == 0 ||
      (ExitingBlocks.size() <= UnrollRuntimeCutoffPoint &&
      OtherExits.size() < UnrollRuntimeCutoffPoint))
    return true;
```
-unroll-runtime-cutoff-point==0 => return true
-unroll-runtime-cutoff-point==1 => return (ExitingBlocks.size() <= 1 OtherExits.size() < 1) || (ExitingBlocks.size() <= 2 && OtherExits.size() == 1 && OtherExits[0]->getTerminatingDeoptimizeCall())
There is no initial value that can keep the current behaviour of (ExitingBlocks.size() <= 2 && OtherExits.size() == 1 && OtherExits[0]->getTerminatingDeoptimizeCall()).

You are right that there is no direct relation between number of exiting blocks and other exits, so we would have to use two cutoff points.

After rethinking about it more, to achieve my original need, which is to allow unrolling a loop with at most 2 exiting blocks and at most 1 other exit block, I added an option to assume the other exit is predictable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97747