[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:57
+             "when epilog is generated"));
+static cl::opt<bool> UnrollRuntimeEnableCutoffPoint(
+    "unroll-runtime-enable-cutoff-point", cl::init(false), cl::Hidden,
----------------
bmahjour wrote:
> Whitney wrote:
> > bmahjour wrote:
> > > Whitney wrote:
> > > > bmahjour wrote:
> > > > > I think we can avoid this extra option by initializing the Cutoff to 0. Any number larger than 0 would mean a valid cutoff. (ie 0 means no cutoff enabled, 1 means no multi-exit, 2 means at most 2 exits, etc). It would also render `UnrollRuntimeMultiExit` obsolete, which can be replaced in this patch or a subsequent one.
> > > > Why `UnrollRuntimeMultiExit` is obsolete? Do you mean the user of `UnrollRuntimeMultiExit` can put a very big number?
> > > Yes. I suppose a value of say 10 would be big enough for most cases....the bigger the number of exiting blocks, the less likely it will be profitable to unroll.
> > ok, I can put a review for that after this patch get landed.
> > Yes. I suppose a value of say 10 would be big enough for most cases....the bigger the number of exiting blocks, the less likely it will be profitable to unroll.
> 
> ...just to clarify this, as Michael pointed out my original intention was to have 0 mean no cutoff (ie any number of exit/exiting blocks are allowed)....but the same thing can effectively be achieved with a large value specified for the option. 
IMHO having 0 as no cutoff is more idiomatic than having an arbitrary-large cutoff (of 0 has otherwise no meaning).

Alternatively, `-1`, interpreted as an unsigned number, would be the largest possible value, sometimes used to represent "[[ https://stackoverflow.com/questions/19578949/in-waitforsingleobject-is-timeout-infinite-the-same-as-timeout-1 | infinity ]]".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97747