[PATCH] D68131: Switch lowering: omit range check for bit tests when default is unreachable (PR43129)

[Hans Wennborg via Phabricator via llvm-commits]

hans updated this revision to Diff 222373.
hans marked 2 inline comments as done.
hans added a comment.

Rebasing and addressing comments.

Please take another look.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68131/new/

https://reviews.llvm.org/D68131

Files:
  llvm/include/llvm/CodeGen/SwitchLoweringUtils.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/CodeGen/X86/switch-bt.ll


Index: llvm/test/CodeGen/X86/switch-bt.ll
===================================================================
--- llvm/test/CodeGen/X86/switch-bt.ll
+++ llvm/test/CodeGen/X86/switch-bt.ll
@@ -157,13 +157,12 @@
 }
 
 
-; TODO: Omit the range check when the default case is unreachable, see PR43129.
+; Omit the range check when the default case is unreachable, see PR43129.
 declare void @g(i32)
 define void @test5(i32 %x) {
 
 ; CHECK-LABEL: test5
-; CHECK: cmpl $8, %edi
-; CHECK: ja
+; CHECK-NOT: cmp
 
 ; 73 = 2^0 + 2^3 + 2^6
 ; CHECK:      movl $73
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2660,16 +2660,20 @@
   addSuccessorWithProb(SwitchBB, MBB, B.Prob);
   SwitchBB->normalizeSuccProbs();
 
-  SDValue BrRange = DAG.getNode(ISD::BRCOND, dl,
-                                MVT::Other, CopyTo, RangeCmp,
-                                DAG.getBasicBlock(B.Default));
-
-  // Avoid emitting unnecessary branches to the next block.
-  if (MBB != NextBlock(SwitchBB))
-    BrRange = DAG.getNode(ISD::BR, dl, MVT::Other, BrRange,
-                          DAG.getBasicBlock(MBB));
-
-  DAG.setRoot(BrRange);
+  SDValue Root = CopyTo;
+  if (B.OmitRangeCheck) {
+    // Branch or fall through to MBB without any range check.
+    if (MBB != NextBlock(SwitchBB))
+      Root = DAG.getNode(ISD::BR, dl, MVT::Other, Root, DAG.getBasicBlock(MBB));
+  } else {
+    // Conditional branch to the default block.
+    Root = DAG.getNode(ISD::BRCOND, dl, MVT::Other, Root, RangeCmp,
+                       DAG.getBasicBlock(B.Default));
+    // Branch or fall through to MBB.
+    if (MBB != NextBlock(SwitchBB))
+      Root = DAG.getNode(ISD::BR, dl, MVT::Other, Root, DAG.getBasicBlock(MBB));
+  }
+  DAG.setRoot(Root);
 }
 
 /// visitBitTestCase - this function produces one "bit test"
@@ -10164,8 +10168,6 @@
         break;
       }
       case CC_BitTests: {
-        // FIXME: If Fallthrough is unreachable, skip the range check.
-
         // FIXME: Optimize away range check based on pivot comparisons.
         BitTestBlock *BTB = &SL->BitTestCases[I->BTCasesIndex];
 
@@ -10186,6 +10188,11 @@
           BTB->DefaultProb -= DefaultProb / 2;
         }
 
+        if (FallthroughUnreachable) {
+          // Skip the range check if the fallthrough block is unreachable.
+          BTB->OmitRangeCheck = true;
+        }
+
         // If we're in the right place, emit the bit test header right now.
         if (CurMBB == SwitchMBB) {
           visitBitTestHeader(*BTB, SwitchMBB);
Index: llvm/include/llvm/CodeGen/SwitchLoweringUtils.h
===================================================================
--- llvm/include/llvm/CodeGen/SwitchLoweringUtils.h
+++ llvm/include/llvm/CodeGen/SwitchLoweringUtils.h
@@ -212,13 +212,14 @@
   BitTestInfo Cases;
   BranchProbability Prob;
   BranchProbability DefaultProb;
+  bool OmitRangeCheck;
 
   BitTestBlock(APInt F, APInt R, const Value *SV, unsigned Rg, MVT RgVT, bool E,
                bool CR, MachineBasicBlock *P, MachineBasicBlock *D,
                BitTestInfo C, BranchProbability Pr)
       : First(std::move(F)), Range(std::move(R)), SValue(SV), Reg(Rg),
         RegVT(RgVT), Emitted(E), ContiguousRange(CR), Parent(P), Default(D),
-        Cases(std::move(C)), Prob(Pr) {}
+        Cases(std::move(C)), Prob(Pr), OmitRangeCheck(false) {}
 };
 
 /// Return the range of values within a range.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68131.222373.patch
Type: text/x-patch
Size: 3572 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190930/6566cbcb/attachment.bin>