[PATCH] D21291: [SimplifyCFG] Range reduce switches

[Eli Friedman via llvm-commits]

eli.friedman added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5029
@@ +5028,3 @@
+  for (auto &V : Values)
+    GCD = llvm::GreatestCommonDivisor64(GCD, (uint64_t)V);
+  if (GCD <= 1 || !llvm::isPowerOf2_64(GCD))
----------------
sanjoy wrote:
> This gcd computation looks like extra work -- why not:
> 
> ```
> uint64_t PotentialGCD = Values[1];
> if (!isPowerOf2(PotentialGCD)) return false;
> if (!all_of(Values, [](uint64_t V) { return V & (PotentialGCD - 1) == 0; }) return false;
> ```
> 
That isn't equivalent for something like "0, 8, 12, 16, 20".

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5056
@@ +5055,3 @@
+  OldBB->getTerminator()->eraseFromParent();
+  BranchInst::Create(SI->getParent(), SI->getDefaultDest(), Cmp, OldBB);
+
----------------
jmolloy wrote:
> eli.friedman wrote:
> > If we were doing this in SelectionDAG, we would be able to combine this branch with the jump table's bounds check: "ROTATE(X - Base, Shift) > Limit".
> Hmm, you're close to convincing me. I'm loath to give up the lookup table optimization though.
> 
> How gross (/ acceptable) would it be to implement this in SelectionDAG and then also teach lookup table lowering in SimplifyCFG this trick too?
It wouldn't be too terrible as long as the actual algorithm for finding a reducible switch is factored out into a utility, I think.


Repository:
  rL LLVM

http://reviews.llvm.org/D21291