[PATCH] D97747: [LoopUnrollRuntime] Add option to unroll loops with at most a specified number of exit/exiting blocks.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp:491-493
+  if (ExitingBlocks.size() <= UnrollRuntimeCutoffPoint &&
+      OtherExits.size() < UnrollRuntimeCutoffPoint)
+    return true;
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97747