[PATCH] D113939: [runtime-unroll] Remove restriction about unrolling multiple exit loops

Tue Nov 23 07:02:24 PST 2021

anna added a comment.

This change looks good to me and thank you for taking this through!

Philip, I think it is worth mentioning in the description for folks benchmarking this patch, that they can just work on ToT and compare baseline against `-unroll-runtime-multi-exit=true` (this is the option we are using for our benchmarking measurements).

================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp:422
   if (UnrollRuntimeMultiExit.getNumOccurrences())
     return UnrollRuntimeMultiExit;

----------------
reames wrote:
> nikic wrote:
> > The UnrollRuntimeMultiExit and UnrollRuntimeOtherExitPredictable options are now dead.
> Er, yeah.  I thought about removing them, but apparently didn't actually do so.  Oops.  :)
> 
> Once we have some sanity check benchmark scores, I'll update to remove these in the patch.  
Maybe we should keep the `UnrollRuntimeMultiExit` option for a week or so (after the change lands), to facilitate triaging of functional/performance regressions.
As mentioned in the description, the original case supported is very narrow - the single exit block other than the latchExit is a deopt block.
So `UnrollRuntimeMultiExit` specified through command line with 0 or 1, will turn ON or OFF the option without profitability considerations. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113939