[llvm] r235771 - SimplifyCFG: Correctly handle switch lookup tables which fully cover the input type and use bit tests to check for holes

[Hans Wennborg]

Author: hans
Date: Fri Apr 24 15:57:56 2015
New Revision: 235771

URL: http://llvm.org/viewvc/llvm-project?rev=235771&view=rev
Log:
SimplifyCFG: Correctly handle switch lookup tables which fully cover the input type and use bit tests to check for holes

When using bit tests for hole checks, we call AddPredecessorToBlock to give the
phi node a value from the bit test block. This would break if we've
previously called removePredecessor on the default destination because the
switch is fully covered.

Test case by Mark Lacey.

Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=235771&r1=235770&r2=235771&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Fri Apr 24 15:57:56 2015
@@ -4159,10 +4159,8 @@ static bool SwitchToLookupTable(SwitchIn
 
   if (!DefaultIsReachable || GeneratingCoveredLookupTable) {
     Builder.CreateBr(LookupBB);
-    // We cached PHINodes in PHIs, to avoid accessing deleted PHINodes later,
-    // do not delete PHINodes here.
-    SI->getDefaultDest()->removePredecessor(SI->getParent(),
-                                            /*DontDeleteUselessPHIs=*/true);
+    // Note: We call removeProdecessor later since we need to be able to get the
+    // PHI value for the default case in case we're using a bit mask.
   } else {
     Value *Cmp = Builder.CreateICmpULT(TableIndex, ConstantInt::get(
                                        MinCaseVal->getType(), TableSize));
@@ -4214,6 +4212,13 @@ static bool SwitchToLookupTable(SwitchIn
     AddPredecessorToBlock(SI->getDefaultDest(), MaskBB, SI->getParent());
   }
 
+  if (!DefaultIsReachable || GeneratingCoveredLookupTable) {
+    // We cached PHINodes in PHIs, to avoid accessing deleted PHINodes later,
+    // do not delete PHINodes here.
+    SI->getDefaultDest()->removePredecessor(SI->getParent(),
+                                            /*DontDeleteUselessPHIs=*/true);
+  }
+
   bool ReturnedEarly = false;
   for (size_t I = 0, E = PHIs.size(); I != E; ++I) {
     PHINode *PHI = PHIs[I];

Modified: llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll?rev=235771&r1=235770&r2=235771&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll (original)
+++ llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll Fri Apr 24 15:57:56 2015
@@ -1273,3 +1273,32 @@ end:
 ; CHECK-LABEL: @pr20210
 ; CHECK: switch i8 %x
 }
+
+; Make sure we do not crash due to trying to generate an unguarded
+; lookup (since i3 can only hold values in the range of explicit
+; values) and simultaneously trying to generate a branch to deal with
+; the fact that we have holes in the range.
+define i32 @covered_switch_with_bit_tests(i3) {
+entry:
+  switch i3 %0, label %l6 [
+    i3 -3, label %l5
+    i3 -4, label %l5
+    i3 3, label %l1
+    i3 2, label %l1
+  ]
+
+l1: br label %l2
+
+l2:
+  %x = phi i32 [ 1, %l1 ], [ 2, %l5 ]
+  br label %l6
+
+l5: br label %l2
+
+l6:
+  %r = phi i32 [ %x, %l2 ], [ 0, %entry ]
+  ret i32 %r
+; CHECK-LABEL: @covered_switch_with_bit_tests
+; CHECK: entry
+; CHECK-NEXT: switch
+}