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

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Makes sense to me generally. Some questions below really.



================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:87
+    "unswitch-num-initial-unscaled-candidates", cl::init(8), cl::Hidden,
+    cl::desc("Amount of unswitch candidates that are ignored when calculating "
+             "cost multiplier."));
----------------
nit: s/Amount/Number/


================
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.
----------------
Not sure how simple this is now?


================
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;
----------------
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?


Repository:
  rL LLVM

https://reviews.llvm.org/D54223