[llvm] r355483 - [CodeGen] Omit range checks from jump tables when lowering switches with unreachable default
Alexander Kornienko via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 6 07:10:51 PST 2019
I see that the commit was reverted and recommitted. The new one is still
causing the same problem.
On Wed, Mar 6, 2019 at 4:05 PM Alexander Kornienko <alexfh at google.com>
wrote:
> This commit causes an assertion failure when compiling LLVM sources. A
> small repro:
> $ cat repro.cpp
> class QQQ {
>
>
> public:
> bool x() const;
> bool y() const;
> unsigned getSizeInBits() const {
> if (y() || x())
> return getScalarSizeInBits();
> return getScalarSizeInBits() * 2;
> }
> unsigned getScalarSizeInBits() const;
> };
> int f(const QQQ &Ty) {
> switch (Ty.getSizeInBits()) {
> case 1:
> case 8:
> return 0;
> case 16:
> return 1;
> case 32:
> return 2;
> case 64:
> return 3;
> default:
> __builtin_unreachable();
> }
> }
> $ clang -O2 -o repro.o repro.cpp
> assert.h assertion failed at llvm/include/llvm/ADT/ilist_iterator.h:139 in
> llvm::ilist_iterator::reference
> llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::MachineInstr,
> true, true, void>, true, false>::operator*() const [OptionsT =
> llvm::ilist_detail::node_options<llvm::MachineInstr, true, true, void>,
> IsReverse = true, IsConst = false]: !NodePtr->i
> sKnownSentinel()
> *** Check failure stack trace: ***
> @ 0x558aab4afc10 __assert_fail
> @ 0x558aa885479b llvm::ilist_iterator<>::operator*()
> @ 0x558aa8854715 llvm::MachineInstrBundleIterator<>::operator*()
> @ 0x558aa92c33c3 llvm::X86InstrInfo::optimizeCompareInstr()
> @ 0x558aa9a9c251 (anonymous
> namespace)::PeepholeOptimizer::optimizeCmpInstr()
> @ 0x558aa9a9b371 (anonymous
> namespace)::PeepholeOptimizer::runOnMachineFunction()
> @ 0x558aa99a4fc8 llvm::MachineFunctionPass::runOnFunction()
> @ 0x558aab019fc4 llvm::FPPassManager::runOnFunction()
> @ 0x558aab01a3a5 llvm::FPPassManager::runOnModule()
> @ 0x558aab01aa9b (anonymous
> namespace)::MPPassManager::runOnModule()
> @ 0x558aab01a635 llvm::legacy::PassManagerImpl::run()
> @ 0x558aab01afe1 llvm::legacy::PassManager::run()
> @ 0x558aa5914769 (anonymous
> namespace)::EmitAssemblyHelper::EmitAssembly()
> @ 0x558aa5910f44 clang::EmitBackendOutput()
> @ 0x558aa5906135 clang::BackendConsumer::HandleTranslationUnit()
> @ 0x558aa6d165ad clang::ParseAST()
> @ 0x558aa6a94e22 clang::ASTFrontendAction::ExecuteAction()
> @ 0x558aa590255d clang::CodeGenAction::ExecuteAction()
> @ 0x558aa6a94840 clang::FrontendAction::Execute()
> @ 0x558aa6a38cca clang::CompilerInstance::ExecuteAction()
> @ 0x558aa4e2294b clang::ExecuteCompilerInvocation()
> @ 0x558aa4df6200 cc1_main()
> @ 0x558aa4e1b37f ExecuteCC1Tool()
> @ 0x558aa4e1a725 main
> @ 0x7ff20d56abbd __libc_start_main
> @ 0x558aa4df51c9 _start
>
> Please revert or fix.
>
> On Wed, Mar 6, 2019 at 8:26 AM Ayonam Ray via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: ayonam
>> Date: Tue Mar 5 23:27:45 2019
>> New Revision: 355483
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=355483&view=rev
>> Log:
>> [CodeGen] Omit range checks from jump tables when lowering switches with
>> unreachable default
>>
>> During the lowering of a switch that would result in the generation of a
>> jump table, a range check is performed before indexing into the jump
>> table, for the switch value being outside the jump table range and a
>> conditional branch is inserted to jump to the default block. In case the
>> default block is unreachable, this conditional jump can be omitted. This
>> patch implements omitting this conditional branch for unreachable
>> defaults.
>>
>> Differential Revision: https://reviews.llvm.org/D52002
>> Reviewers: Hans Wennborg, Eli Freidman, Roman Lebedev
>>
>> Added:
>> llvm/trunk/test/CodeGen/AArch64/switch-unreachable-default.ll
>> Modified:
>> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
>> llvm/trunk/test/CodeGen/X86/pr38743.ll
>> llvm/trunk/test/CodeGen/X86/switch-jump-table.ll
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=355483&r1=355482&r2=355483&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Tue Mar
>> 5 23:27:45 2019
>> @@ -2388,24 +2388,31 @@ void SelectionDAGBuilder::visitJumpTable
>> JumpTableReg, SwitchOp);
>> JT.Reg = JumpTableReg;
>>
>> - // Emit the range check for the jump table, and branch to the default
>> block
>> - // for the switch statement if the value being switched on exceeds the
>> largest
>> - // case in the switch.
>> - SDValue CMP = DAG.getSetCC(
>> - dl, TLI.getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(),
>> - Sub.getValueType()),
>> - Sub, DAG.getConstant(JTH.Last - JTH.First, dl, VT), ISD::SETUGT);
>> -
>> - SDValue BrCond = DAG.getNode(ISD::BRCOND, dl,
>> - MVT::Other, CopyTo, CMP,
>> - DAG.getBasicBlock(JT.Default));
>> -
>> - // Avoid emitting unnecessary branches to the next block.
>> - if (JT.MBB != NextBlock(SwitchBB))
>> - BrCond = DAG.getNode(ISD::BR, dl, MVT::Other, BrCond,
>> - DAG.getBasicBlock(JT.MBB));
>> -
>> - DAG.setRoot(BrCond);
>> + if (!JTH.OmitRangeCheck) {
>> + // Emit the range check for the jump table, and branch to the
>> default block
>> + // for the switch statement if the value being switched on exceeds
>> the
>> + // largest case in the switch.
>> + SDValue CMP = DAG.getSetCC(
>> + dl, TLI.getSetCCResultType(DAG.getDataLayout(),
>> *DAG.getContext(),
>> + Sub.getValueType()),
>> + Sub, DAG.getConstant(JTH.Last - JTH.First, dl, VT), ISD::SETUGT);
>> +
>> + SDValue BrCond = DAG.getNode(ISD::BRCOND, dl,
>> + MVT::Other, CopyTo, CMP,
>> + DAG.getBasicBlock(JT.Default));
>> +
>> + // Avoid emitting unnecessary branches to the next block.
>> + if (JT.MBB != NextBlock(SwitchBB))
>> + BrCond = DAG.getNode(ISD::BR, dl, MVT::Other, BrCond,
>> + DAG.getBasicBlock(JT.MBB));
>> +
>> + DAG.setRoot(BrCond);
>> + } else {
>> + SDValue BrCond = DAG.getNode(ISD::BR, dl, MVT::Other, CopyTo,
>> + DAG.getBasicBlock(JT.MBB));
>> + DAG.setRoot(BrCond);
>> + SwitchBB->removeSuccessor(JT.Default, true);
>> + }
>> }
>>
>> /// Create a LOAD_STACK_GUARD node, and let it carry the target specific
>> global
>> @@ -9790,10 +9797,13 @@ bool SelectionDAGBuilder::buildJumpTable
>> ->createJumpTableIndex(Table);
>>
>> // Set up the jump table info.
>> + bool UnreachableDefault =
>> + isa<UnreachableInst>(SI->getDefaultDest()->getFirstNonPHIOrDbg());
>> + bool OmitRangeCheck = UnreachableDefault;
>> JumpTable JT(-1U, JTI, JumpTableMBB, nullptr);
>> JumpTableHeader JTH(Clusters[First].Low->getValue(),
>> Clusters[Last].High->getValue(),
>> SI->getCondition(),
>> - nullptr, false);
>> + nullptr, false, OmitRangeCheck);
>> JTCases.emplace_back(std::move(JTH), std::move(JT));
>>
>> JTCluster = CaseCluster::jumpTable(Clusters[First].Low,
>> Clusters[Last].High,
>> @@ -10599,38 +10609,6 @@ void SelectionDAGBuilder::visitSwitch(co
>> // if there are many clusters.
>> sortAndRangeify(Clusters);
>>
>> - if (TM.getOptLevel() != CodeGenOpt::None) {
>> - // Replace an unreachable default with the most popular destination.
>> - // FIXME: Exploit unreachable default more aggressively.
>> - bool UnreachableDefault =
>> - isa<UnreachableInst>(SI.getDefaultDest()->getFirstNonPHIOrDbg());
>> - if (UnreachableDefault && !Clusters.empty()) {
>> - DenseMap<const BasicBlock *, unsigned> Popularity;
>> - unsigned MaxPop = 0;
>> - const BasicBlock *MaxBB = nullptr;
>> - for (auto I : SI.cases()) {
>> - const BasicBlock *BB = I.getCaseSuccessor();
>> - if (++Popularity[BB] > MaxPop) {
>> - MaxPop = Popularity[BB];
>> - MaxBB = BB;
>> - }
>> - }
>> - // Set new default.
>> - assert(MaxPop > 0 && MaxBB);
>> - DefaultMBB = FuncInfo.MBBMap[MaxBB];
>> -
>> - // Remove cases that were pointing to the destination that is now
>> the
>> - // default.
>> - CaseClusterVector New;
>> - New.reserve(Clusters.size());
>> - for (CaseCluster &CC : Clusters) {
>> - if (CC.MBB != DefaultMBB)
>> - New.push_back(CC);
>> - }
>> - Clusters = std::move(New);
>> - }
>> - }
>> -
>> // The branch probablity of the peeled case.
>> BranchProbability PeeledCaseProb = BranchProbability::getZero();
>> MachineBasicBlock *PeeledSwitchMBB =
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h?rev=355483&r1=355482&r2=355483&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h Tue Mar 5
>> 23:27:45 2019
>> @@ -277,11 +277,12 @@ private:
>> const Value *SValue;
>> MachineBasicBlock *HeaderBB;
>> bool Emitted;
>> + bool OmitRangeCheck;
>>
>> JumpTableHeader(APInt F, APInt L, const Value *SV, MachineBasicBlock
>> *H,
>> - bool E = false)
>> + bool E = false, bool ORC = false)
>> : First(std::move(F)), Last(std::move(L)), SValue(SV),
>> HeaderBB(H),
>> - Emitted(E) {}
>> + Emitted(E), OmitRangeCheck(ORC) {}
>> };
>> using JumpTableBlock = std::pair<JumpTableHeader, JumpTable>;
>>
>>
>> Added: llvm/trunk/test/CodeGen/AArch64/switch-unreachable-default.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/switch-unreachable-default.ll?rev=355483&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/AArch64/switch-unreachable-default.ll (added)
>> +++ llvm/trunk/test/CodeGen/AArch64/switch-unreachable-default.ll Tue
>> Mar 5 23:27:45 2019
>> @@ -0,0 +1,62 @@
>> +; RUN: llc -O3 -o - %s | FileCheck %s
>> +
>> +; Test that the output in the presence of an unreachable default does
>> not have
>> +; a compare and branch at the top of the switch to handle the default
>> case.
>> +
>> +target triple = "aarch64-unknown-linux-gnu"
>> +
>> +; Function Attrs: nounwind
>> +define void @fn(i4) {
>> + switch i4 %0, label %default [
>> + i4 0, label %case_0
>> + i4 1, label %case_1
>> + i4 2, label %case_2
>> + i4 3, label %case_3
>> + i4 4, label %case_4
>> + i4 5, label %case_5
>> + ]
>> +
>> +; CHECK-LABEL: fn:
>> +; CHECK-NOT: sub
>> +; CHECK-NOT: cmp
>> +; CHECK-NOT: b.hi
>> +; CHECK: ldr {{x[0-9]+}}, [{{x[0-9]+}}, {{x[0-9]+}}, lsl #3]
>> +; CHECK: br {{x[0-9]+}}
>> +
>> +default:
>> + unreachable
>> +
>> +case_0:
>> + tail call void @handle_case_00(i4 %0) #2
>> + br label %return_label
>> +
>> +case_1:
>> + tail call void @handle_case_01(i4 %0) #2
>> + br label %return_label
>> +
>> +case_2:
>> + tail call void @handle_case_02(i4 %0) #2
>> + br label %return_label
>> +
>> +case_3:
>> + tail call void @handle_case_03(i4 %0) #2
>> + br label %return_label
>> +
>> +case_4:
>> + tail call void @handle_case_04(i4 %0) #2
>> + br label %return_label
>> +
>> +case_5:
>> + tail call void @handle_case_05(i4 %0) #2
>> + br label %return_label
>> +
>> +return_label:
>> + ret void
>> +}
>> +
>> +declare void @handle_case_00(i4)
>> +declare void @handle_case_01(i4)
>> +declare void @handle_case_02(i4)
>> +declare void @handle_case_03(i4)
>> +declare void @handle_case_04(i4)
>> +declare void @handle_case_05(i4)
>>
>> Modified: llvm/trunk/test/CodeGen/X86/pr38743.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr38743.ll?rev=355483&r1=355482&r2=355483&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/pr38743.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/pr38743.ll Tue Mar 5 23:27:45 2019
>> @@ -18,41 +18,43 @@ declare void @llvm.memcpy.p0i8.p0i8.i64(
>>
>> define void @pr38743() #1 align 2 {
>> ; CHECK-LABEL: pr38743:
>> -; CHECK: # %bb.0: # %bb
>> -; CHECK-NEXT: cmpl $3, %eax
>> -; CHECK-NEXT: je .LBB0_4
>> -; CHECK-NEXT: # %bb.1: # %bb
>> -; CHECK-NEXT: cmpl $1, %eax
>> -; CHECK-NEXT: je .LBB0_2
>> -; CHECK-NEXT: # %bb.3: # %bb5
>> -; CHECK-NEXT: movzwl .str.17+{{.*}}(%rip), %eax
>> -; CHECK-NEXT: movw %ax, -{{[0-9]+}}(%rsp)
>> -; CHECK-NEXT: movq {{.*}}(%rip), %rax
>> -; CHECK-NEXT: jmp .LBB0_5
>> -; CHECK-NEXT: .LBB0_4: # %bb8
>> -; CHECK-NEXT: movq .str.18+{{.*}}(%rip), %rax
>> -; CHECK-NEXT: movq %rax, -{{[0-9]+}}(%rsp)
>> -; CHECK-NEXT: movq {{.*}}(%rip), %rax
>> -; CHECK-NEXT: jmp .LBB0_5
>> -; CHECK-NEXT: .LBB0_2: # %bb2
>> -; CHECK-NEXT: movq .str.16+{{.*}}(%rip), %rax
>> -; CHECK-NEXT: movq %rax, -{{[0-9]+}}(%rsp)
>> -; CHECK-NEXT: movq {{.*}}(%rip), %rax
>> -; CHECK-NEXT: .LBB0_5: # %bb12
>> -; CHECK-NEXT: movq %rax, -{{[0-9]+}}(%rsp)
>> -; CHECK-NEXT: movq -{{[0-9]+}}(%rsp), %rax
>> -; CHECK-NEXT: movq %rax, (%rax)
>> -; CHECK-NEXT: movb -{{[0-9]+}}(%rsp), %al
>> -; CHECK-NEXT: movq -{{[0-9]+}}(%rsp), %rcx
>> -; CHECK-NEXT: movzwl -{{[0-9]+}}(%rsp), %edx
>> -; CHECK-NEXT: movl -{{[0-9]+}}(%rsp), %esi
>> -; CHECK-NEXT: movb -{{[0-9]+}}(%rsp), %dil
>> -; CHECK-NEXT: movb %al, (%rax)
>> -; CHECK-NEXT: movq %rcx, 1(%rax)
>> -; CHECK-NEXT: movw %dx, 9(%rax)
>> -; CHECK-NEXT: movl %esi, 11(%rax)
>> -; CHECK-NEXT: movb %dil, 15(%rax)
>> -; CHECK-NEXT: retq
>> +; CHECK: # %bb.0: # %bb
>> +; CHECK-NEXT: xorl %eax, %eax
>> +; CHECK-NEXT: jmpq *.LJTI0_0(,%rax,8)
>> +; CHECK-NEXT: .[[LABEL1:[A-Z_0-9]+]]: #
>> %bb5
>> +; CHECK-NEXT: movzwl .str.17+{{.*}}(%rip), %eax
>> +; CHECK-NEXT: movw %ax, -{{[0-9]+}}(%rsp)
>> +; CHECK-NEXT: movq .str.17(%rip), %rax
>> +; CHECK-NEXT: jmp .[[LABEL4:[A-Z_0-9]+]]
>> +; CHECK-NEXT: .[[LABEL2:[A-Z_0-9]+]]: #
>> %bb2
>> +; CHECK-NEXT: movq .str.16+{{.*}}(%rip), %rax
>> +; CHECK-NEXT: movq %rax, -{{[0-9]+}}(%rsp)
>> +; CHECK-NEXT: movq .str.16(%rip), %rax
>> +; CHECK-NEXT: jmp .[[LABEL4]]
>> +; CHECK-NEXT: .[[LABEL3:[A-Z_0-9]+]]: #
>> %bb8
>> +; CHECK-NEXT: movq .str.18+{{.*}}(%rip), %rax
>> +; CHECK-NEXT: movq %rax, -{{[0-9]+}}(%rsp)
>> +; CHECK-NEXT: movq .str.18(%rip), %rax
>> +; CHECK-NEXT: .[[LABEL4]]: # %bb12
>> +; CHECK-NEXT: movq %rax, -{{[0-9]+}}(%rsp)
>> +; CHECK-NEXT: movq -{{[0-9]+}}(%rsp), %rax
>> +; CHECK-NEXT: movq %rax, (%rax)
>> +; CHECK-NEXT: movb -{{[0-9]+}}(%rsp), %al
>> +; CHECK-NEXT: movq -{{[0-9]+}}(%rsp), %rcx
>> +; CHECK-NEXT: movzwl -{{[0-9]+}}(%rsp), %edx
>> +; CHECK-NEXT: movl -{{[0-9]+}}(%rsp), %esi
>> +; CHECK-NEXT: movb -{{[0-9]+}}(%rsp), %dil
>> +; CHECK-NEXT: movb %al, (%rax)
>> +; CHECK-NEXT: movq %rcx, {{[0-9]+}}(%rax)
>> +; CHECK-NEXT: movw %dx, {{[0-9]+}}(%rax)
>> +; CHECK-NEXT: movl %esi, {{[0-9]+}}(%rax)
>> +; CHECK-NEXT: movb %dil, {{[0-9]+}}(%rax)
>> +; CHECK-NEXT: retq
>> +; CHECK-LABEL: .LJTI0_0:
>> +; CHECK: .quad .[[LABEL2]]
>> +; CHECK-NEXT: .quad .[[LABEL1]]
>> +; CHECK-NEXT: .quad .[[LABEL3]]
>> +; CHECK-NEXT: .quad .[[LABEL1]]
>> bb:
>> %tmp = alloca %0, align 16
>> %tmp1 = bitcast %0* %tmp to i8*
>>
>> Modified: llvm/trunk/test/CodeGen/X86/switch-jump-table.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/switch-jump-table.ll?rev=355483&r1=355482&r2=355483&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/switch-jump-table.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/switch-jump-table.ll Tue Mar 5 23:27:45
>> 2019
>> @@ -2,14 +2,12 @@
>> ; RUN: llc -mtriple=i686-pc-gnu-linux
>> -print-machineinstrs=expand-isel-pseudos %s -o /dev/null 2>&1 | FileCheck
>> %s -check-prefix=CHECK-JT-PROB
>>
>>
>> -; An unreachable default destination is replaced with the most popular
>> case label.
>> +; An unreachable default destination is ignored and no compare and branch
>> +; is generated for the default values.
>>
>> define void @foo(i32 %x, i32* %to) {
>> ; CHECK-LABEL: foo:
>> ; CHECK: movl 4(%esp), [[REG:%e[a-z]{2}]]
>> -; CHECK: cmpl $3, [[REG]]
>> -; CHECK: ja .LBB0_6
>> -; CHECK-NEXT: # %bb.1:
>> ; CHECK-NEXT: jmpl *.LJTI0_0(,[[REG]],4)
>> ; CHECK: movl $4
>> ; CHECK: retl
>> @@ -45,10 +43,12 @@ default:
>>
>> ; The jump table has four entries.
>> ; CHECK-LABEL: .LJTI0_0:
>> +; CHECK-NEXT: .long .LBB0_1
>> ; CHECK-NEXT: .long .LBB0_2
>> ; CHECK-NEXT: .long .LBB0_3
>> ; CHECK-NEXT: .long .LBB0_4
>> ; CHECK-NEXT: .long .LBB0_5
>> +; CHECK-NEXT: .long .LBB0_5
>> }
>>
>> ; Check if branch probabilities are correctly assigned to the jump table.
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190306/2baa0f83/attachment.html>
More information about the llvm-commits
mailing list