From 372aece777020ac19894b17b8c81d63f2ec77dad Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Wed, 2 Oct 2019 12:08:44 +0000 Subject: [PATCH] Revert r373431 "Switch lowering: omit range check for bit tests when default is unreachable (PR43129)" This broke http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19967 > Switch lowering: omit range check for bit tests when default is unreachable (PR43129) > > This is modeled after the same functionality for jump tables, which was > added in r357067. > > Differential revision: https://reviews.llvm.org/D68131 llvm-svn: 373454 --- .../llvm/CodeGen/SwitchLoweringUtils.h | 3 +- .../SelectionDAG/SelectionDAGBuilder.cpp | 40 ++++++++----------- llvm/test/CodeGen/X86/switch-bt.ll | 5 ++- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/llvm/include/llvm/CodeGen/SwitchLoweringUtils.h b/llvm/include/llvm/CodeGen/SwitchLoweringUtils.h index b8adcf759b19..31b5f794d90c 100644 --- a/llvm/include/llvm/CodeGen/SwitchLoweringUtils.h +++ b/llvm/include/llvm/CodeGen/SwitchLoweringUtils.h @@ -212,14 +212,13 @@ struct BitTestBlock { 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), OmitRangeCheck(false) {} + Cases(std::move(C)), Prob(Pr) {} }; /// Return the range of values within a range. diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 4a94d351b985..ff6358b442ad 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -2622,11 +2622,17 @@ void SelectionDAGBuilder::visitBitTestHeader(BitTestBlock &B, // Subtract the minimum value. SDValue SwitchOp = getValue(B.SValue); EVT VT = SwitchOp.getValueType(); - SDValue RangeSub = - DAG.getNode(ISD::SUB, dl, VT, SwitchOp, DAG.getConstant(B.First, dl, VT)); + SDValue Sub = DAG.getNode(ISD::SUB, dl, VT, SwitchOp, + DAG.getConstant(B.First, dl, VT)); + + // Check range. + const TargetLowering &TLI = DAG.getTargetLoweringInfo(); + SDValue RangeCmp = DAG.getSetCC( + dl, TLI.getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), + Sub.getValueType()), + Sub, DAG.getConstant(B.Range, dl, VT), ISD::SETUGT); // Determine the type of the test operands. - const TargetLowering &TLI = DAG.getTargetLoweringInfo(); bool UsePtrType = false; if (!TLI.isTypeLegal(VT)) { UsePtrType = true; @@ -2639,7 +2645,6 @@ void SelectionDAGBuilder::visitBitTestHeader(BitTestBlock &B, break; } } - SDValue Sub = RangeSub; if (UsePtrType) { VT = TLI.getPointerTy(DAG.getDataLayout()); Sub = DAG.getZExtOrTrunc(Sub, dl, VT); @@ -2655,24 +2660,16 @@ void SelectionDAGBuilder::visitBitTestHeader(BitTestBlock &B, addSuccessorWithProb(SwitchBB, MBB, B.Prob); SwitchBB->normalizeSuccProbs(); - SDValue Root = CopyTo; - if (!B.OmitRangeCheck) { - // Conditional branch to the default block. - SDValue RangeCmp = DAG.getSetCC(dl, - TLI.getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), - RangeSub.getValueType()), - RangeSub, DAG.getConstant(B.Range, dl, RangeSub.getValueType()), - ISD::SETUGT); - - Root = DAG.getNode(ISD::BRCOND, dl, MVT::Other, Root, RangeCmp, - DAG.getBasicBlock(B.Default)); - } + 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)) - Root = DAG.getNode(ISD::BR, dl, MVT::Other, Root, DAG.getBasicBlock(MBB)); + BrRange = DAG.getNode(ISD::BR, dl, MVT::Other, BrRange, + DAG.getBasicBlock(MBB)); - DAG.setRoot(Root); + DAG.setRoot(BrRange); } /// visitBitTestCase - this function produces one "bit test" @@ -10167,6 +10164,8 @@ void SelectionDAGBuilder::lowerWorkItem(SwitchWorkListItem W, Value *Cond, 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]; @@ -10187,11 +10186,6 @@ void SelectionDAGBuilder::lowerWorkItem(SwitchWorkListItem W, Value *Cond, 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); diff --git a/llvm/test/CodeGen/X86/switch-bt.ll b/llvm/test/CodeGen/X86/switch-bt.ll index 965cdbf17f50..797ad4bccfd0 100644 --- a/llvm/test/CodeGen/X86/switch-bt.ll +++ b/llvm/test/CodeGen/X86/switch-bt.ll @@ -157,12 +157,13 @@ sw.epilog: } -; Omit the range check when the default case is unreachable, see PR43129. +; TODO: 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-NOT: cmp +; CHECK: cmpl $8, %edi +; CHECK: ja ; 73 = 2^0 + 2^3 + 2^6 ; CHECK: movl $73