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

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 12 03:12:23 PST 2018


mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:75
 
+static cl::opt<bool> EnableUnswitchCostMultiplier(
+    "enable-unswitch-cost-multiplier", cl::init(true), cl::Hidden,
----------------
That name is not super-informative. Maybe something like `LimitUnswitchingFromExponentExplosion` or something of this variety?


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2286
+
+  if (!EnableUnswitchCostMultiplier)
+    return 1;
----------------
I'd suggest to make this check outside `calculateUnswitchCostMultiplier` and only multiply cost by this factor if needed.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2546
+    assert((CostMultiplier > 0 && CostMultiplier <= UnswitchThreshold) &&
+           "cost multiplier needs to be in the range of 0..UnswitchThreshold");
+    CandidateCost *= CostMultiplier;
----------------
Maybe `1..UnswitchThreshold`?


Repository:
  rL LLVM

https://reviews.llvm.org/D54223





More information about the llvm-commits mailing list