[PATCH] D54223: [SimpleLoopUnswitch] adding cost multiplier to cap exponential unswitch with

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev added inline comments.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2281-2283
+/// cluster of loops. Formula is kept intentionally simple since it is designed
+/// to limit the worst case behavior. It is not an attempt to provide a detailed
+/// heuristic size prediction.
----------------
chandlerc wrote:
> Not sure how simple this is now?
Err... intent was for it to be simple :)


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2307-2308
+                               : std::distance(LI.begin(), LI.end()));
+  // Count candidates - branch/guard counts as 1, switch counts as
+  // log2 of its cases.
+  int CandidatesNum = 0;
----------------
chandlerc wrote:
> I'd change the name here. You're not counting candidates, you're counting unswitched clones I think.
> 
> To that end, should you do the same filtering here that you do above for guards and exiting conditions?
Hmm... yes, filtering them here makes sense.
Speaking of which, I dont believe my filtering is right for the switch.
Even if switch has one exiting case it still can have many other cases remaining for the unswitch (and thus duplication of loops).
Will try to do something here, though for now I'm still going to keep it "simple" ... at least to the extent possible.


Repository:
  rL LLVM

https://reviews.llvm.org/D54223