[llvm] r266479 - Switch lowering: don't add incoming PHI values from skipped bit test MBB's (PR27135)

[Hans Wennborg via llvm-commits]

Author: hans
Date: Fri Apr 15 16:45:30 2016
New Revision: 266479

URL: http://llvm.org/viewvc/llvm-project?rev=266479&view=rev
Log:
Switch lowering: don't add incoming PHI values from skipped bit test MBB's (PR27135)

After r245976, LLVM will skip the last bit test case if knows it will always be
true. However, we would still erroneously update PHI nodes with incoming values
from the MBB that would perform the final bit test, causing -verify-machineinstrs
to fail.

Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
    llvm/trunk/test/CodeGen/X86/switch.ll

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=266479&r1=266478&r2=266479&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Fri Apr 15 16:45:30 2016
@@ -1675,14 +1675,23 @@ SelectionDAGISel::FinishBasicBlock() {
       // If all cases cover a contiguous range, it is not necessary to jump to
       // the default block after the last bit test fails. This is because the
       // range check during bit test header creation has guaranteed that every
-      // case here doesn't go outside the range.
+      // case here doesn't go outside the range. In this case, there is no need
+      // to perform the last bit test, as it will always be true. Instead, make
+      // the second-to-last bit-test fall through to the target of the last bit
+      // test, and delete the last bit test.
+
       MachineBasicBlock *NextMBB;
-      if (BTB.ContiguousRange && j + 2 == ej)
+      if (BTB.ContiguousRange && j + 2 == ej) {
+        // Second-to-last bit-test with contiguous range: fall through to the
+        // target of the final bit test.
         NextMBB = BTB.Cases[j + 1].TargetBB;
-      else if (j + 1 != ej)
-        NextMBB = BTB.Cases[j + 1].ThisBB;
-      else
+      } else if (j + 1 == ej) {
+        // For the last bit test, fall through to Default.
         NextMBB = BTB.Default;
+      } else {
+        // Otherwise, fall through to the next bit test.
+        NextMBB = BTB.Cases[j + 1].ThisBB;
+      }
 
       SDB->visitBitTestCase(BTB, NextMBB, UnhandledProb, BTB.Reg, BTB.Cases[j],
                             FuncInfo->MBB);
@@ -1691,8 +1700,11 @@ SelectionDAGISel::FinishBasicBlock() {
       SDB->clear();
       CodeGenAndEmitDAG();
 
-      if (BTB.ContiguousRange && j + 2 == ej)
+      if (BTB.ContiguousRange && j + 2 == ej) {
+        // Since we're not going to use the final bit test, remove it.
+        BTB.Cases.pop_back();
         break;
+      }
     }
 
     // Update PHI Nodes
@@ -1703,12 +1715,14 @@ SelectionDAGISel::FinishBasicBlock() {
       assert(PHI->isPHI() &&
              "This is not a machine PHI node that we are updating!");
       // This is "default" BB. We have two jumps to it. From "header" BB and
-      // from last "case" BB.
-      if (PHIBB == BTB.Default)
-        PHI.addReg(FuncInfo->PHINodesToUpdate[pi].second)
-           .addMBB(BTB.Parent)
-           .addReg(FuncInfo->PHINodesToUpdate[pi].second)
-           .addMBB(BTB.Cases.back().ThisBB);
+      // from last "case" BB, unless the latter was skipped.
+      if (PHIBB == BTB.Default) {
+        PHI.addReg(FuncInfo->PHINodesToUpdate[pi].second).addMBB(BTB.Parent);
+        if (!BTB.ContiguousRange) {
+          PHI.addReg(FuncInfo->PHINodesToUpdate[pi].second)
+              .addMBB(BTB.Cases.back().ThisBB);
+         }
+      }
       // One of "cases" BB.
       for (unsigned j = 0, ej = BTB.Cases.size();
            j != ej; ++j) {

Modified: llvm/trunk/test/CodeGen/X86/switch.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/switch.ll?rev=266479&r1=266478&r2=266479&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/switch.ll (original)
+++ llvm/trunk/test/CodeGen/X86/switch.ll Fri Apr 15 16:45:30 2016
@@ -1,5 +1,5 @@
-; RUN: llc -mtriple=x86_64-linux-gnu %s -o - -jump-table-density=40 | FileCheck %s
-; RUN: llc -mtriple=x86_64-linux-gnu %s -o - -O0 -jump-table-density=40 | FileCheck --check-prefix=NOOPT %s
+; RUN: llc -mtriple=x86_64-linux-gnu %s -o - -jump-table-density=40 -verify-machineinstrs | FileCheck %s
+; RUN: llc -mtriple=x86_64-linux-gnu %s -o - -O0 -jump-table-density=40 -verify-machineinstrs | FileCheck --check-prefix=NOOPT %s
 
 declare void @g(i32)
 
@@ -748,3 +748,33 @@ return: ret void
 ; Don't assert due to truncating the bitwidth (64) to i4 when checking
 ; that the bit-test range fits in a word.
 }
+
+
+define i32 @pr27132(i32 %i) {
+entry:
+  br i1 undef, label %sw, label %end
+sw:
+  switch i32 %i, label %end [
+    i32 99,  label %sw.bb
+    i32 98,  label %sw.bb
+    i32 101, label %sw.bb
+    i32 97,  label %sw.bb2
+    i32 96,  label %sw.bb2
+    i32 100, label %sw.bb2
+  ]
+sw.bb:
+  unreachable
+sw.bb2:
+  unreachable
+end:
+  %p = phi i32 [ 1, %sw ], [ 0, %entry ]
+  ret i32 %p
+
+; CHECK-LABEL: pr27132:
+; The switch is lowered with bit tests. Since the case range is contiguous, the
+; second bit test is redundant and can be skipped. Check that we don't update
+; the phi node with an incoming value from the MBB of the skipped bit test
+; (-verify-machine-instrs cathces this).
+; CHECK: btl
+; CHECK-NOT: btl
+}