[PATCH] D11995: [SimplifyCFG] Prune code from a provably unreachable switch default

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3255
@@ +3254,3 @@
+    !isa<UnreachableInst>(SI->getDefaultDest()->getFirstNonPHIOrDbg());
+  if (HasDefault && DeadCases.empty() && 
+      SI->getNumCases() >= pow(2,Bits)) {
----------------
hans wrote:
> hmm, why does DeadCases have to be empty?
My original reasoning was that a dead case didn't cover the input, but thinking about it, I think that's false.  By proving that it's dead, it still covered that value in the input space.  So, it can be removed.  :)

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3256
@@ +3255,3 @@
+  if (HasDefault && DeadCases.empty() && 
+      SI->getNumCases() >= pow(2,Bits)) {
+    DEBUG(dbgs() << "SimplifyCFG: switch default is dead.\n");
----------------
majnemer wrote:
> sanjoy wrote:
> > majnemer wrote:
> > > reames wrote:
> > > > sanjoy wrote:
> > > > > hans wrote:
> > > > > > sanjoy wrote:
> > > > > > > Can `SI->getNumCases()` ever be `> pow(2, Bits)`?
> > > > > > > 
> > > > > > > I don't think checking for `>=` is sufficient if `switch`es allow duplicate cases, if that's how number of case can be greater than 2^Bits.
> > > > > > ultra nit: space between , and Bits
> > > > > > I don't think checking for >= is sufficient if switches allow duplicate cases, if that's how number of case can be greater than 2^Bits.
> > > > > 
> > > > > I had missed your note in the commit message -- given that duplicate values are prohibited, I think it should be sufficient to check for equality.
> > > > I'll switch to the equality before submission.
> > > `pow` uses doubles, was this intentional?
> > > 
> > > Could we use `1U << Bits` or `1ULL << Bits`?
> > > Could we use 1U << Bits or 1ULL << Bits?
> > 
> > Won't that break if `Bits` is > 64?
> Right, I guess we'd need to do something like `Log2_32(SI->getNumCases()) >= Bits`.
I'm tempted to use the shift by Bits with an additional constraint that Bits is <= 63.  I can't imagine we're every going to see a switch with more than 2^63 cases.  :)


http://reviews.llvm.org/D11995