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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2489
+    int CostMultiplier = 1;
+    if (!isGuard(&TI)) {
+      auto *ParentL = L.getParentLoop();
----------------
The check here should be more accurate, at least it should consider conditional branches that has a dest outside the loop, because unswitching of such branches won't produce a sibling loop.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2513
+      LLVM_DEBUG(dbgs() << "  Computed multiplier  " << CostMultiplier
+                        << "(siblings " << SiblingsMultiplier
+                        << " * candidates " << (1 << CandidatesPower) << ")"
----------------
Space before `(`


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2518
+
+    CandidateCost *= std::max(CostMultiplier, 1);
     LLVM_DEBUG(dbgs() << "  Computed cost of " << CandidateCost
----------------
How can `CostMultiplier` be less than 1? Integer overflow is undefined, so you won't catch it, otherwise it's just impossible. :)


Repository:
  rL LLVM

https://reviews.llvm.org/D54223