From: Francis Visoiu Mistrih Date: Wed, 3 Jul 2019 17:16:45 +0000 (+0000) Subject: [CodeGen] Make branch funnels pass the machine verifier X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0eaa652ef013bef422b6055abac7bd296c463bc6;p=llvm [CodeGen] Make branch funnels pass the machine verifier We previously marked all the tests with branch funnels as `-verify-machineinstrs=0`. This is an attempt to fix it. 1) `ICALL_BRANCH_FUNNEL` has no defs. Mark it as `let OutOperandList = (outs)` 2) After that we hit an assert: ``` Assertion failed: (Op.getValueType() != MVT::Other && Op.getValueType() != MVT::Glue && "Chain and glue operands should occur at end of operand list!"), function AddOperand, file /Users/francisvm/llvm/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp, line 461. ``` The chain operand was added at the beginning of the operand list. Move that to the end. 3) After that we hit another verifier issue in the pseudo expansion where the registers used in the cmps and jmps are not added to the livein lists. Add the `EFLAGS` to all the new MBBs that we create. PR39436 Differential Review: https://reviews.llvm.org/D54155 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365058 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Target/Target.td b/include/llvm/Target/Target.td index 43b2a2a2858..d58662e128e 100644 --- a/include/llvm/Target/Target.td +++ b/include/llvm/Target/Target.td @@ -1207,7 +1207,7 @@ def FENTRY_CALL : StandardPseudoInstruction { let hasSideEffects = 1; } def ICALL_BRANCH_FUNNEL : StandardPseudoInstruction { - let OutOperandList = (outs unknown:$dst); + let OutOperandList = (outs); let InOperandList = (ins variable_ops); let AsmString = ""; let hasSideEffects = 1; diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index aa6435f3349..cf5e9031478 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -6746,7 +6746,6 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, case Intrinsic::icall_branch_funnel: { SmallVector Ops; - Ops.push_back(DAG.getRoot()); Ops.push_back(getValue(I.getArgOperand(0))); int64_t Offset; @@ -6789,6 +6788,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, Ops.push_back(T.Target); } + Ops.push_back(DAG.getRoot()); // Chain SDValue N(DAG.getMachineNode(TargetOpcode::ICALL_BRANCH_FUNNEL, getCurSDLoc(), MVT::Other, Ops), 0); diff --git a/lib/Target/X86/X86ExpandPseudo.cpp b/lib/Target/X86/X86ExpandPseudo.cpp index 6cda7ad1c3b..b8624b40f2f 100644 --- a/lib/Target/X86/X86ExpandPseudo.cpp +++ b/lib/Target/X86/X86ExpandPseudo.cpp @@ -87,6 +87,8 @@ void X86ExpandPseudo::ExpandICallBranchFunnel( const GlobalValue *CombinedGlobal = JTInst->getOperand(1).getGlobal(); auto CmpTarget = [&](unsigned Target) { + if (Selector.isReg()) + MBB->addLiveIn(Selector.getReg()); BuildMI(*MBB, MBBI, DL, TII->get(X86::LEA64r), X86::R11) .addReg(X86::RIP) .addImm(1) @@ -102,6 +104,8 @@ void X86ExpandPseudo::ExpandICallBranchFunnel( auto CreateMBB = [&]() { auto *NewMBB = MF->CreateMachineBasicBlock(BB); MBB->addSuccessor(NewMBB); + if (!MBB->isLiveIn(X86::EFLAGS)) + MBB->addLiveIn(X86::EFLAGS); return NewMBB; }; diff --git a/test/CodeGen/X86/icall-branch-funnel.ll b/test/CodeGen/X86/icall-branch-funnel.ll index 6d7e0c3d2c4..010734cd856 100644 --- a/test/CodeGen/X86/icall-branch-funnel.ll +++ b/test/CodeGen/X86/icall-branch-funnel.ll @@ -1,5 +1,4 @@ -; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39436. -; RUN: llc -mtriple=x86_64-unknown-linux -verify-machineinstrs=0 < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-unknown-linux < %s | FileCheck %s @g = external global i8 diff --git a/test/ThinLTO/X86/cfi-devirt.ll b/test/ThinLTO/X86/cfi-devirt.ll index 2ea6fc4cac0..760c02a24ea 100644 --- a/test/ThinLTO/X86/cfi-devirt.ll +++ b/test/ThinLTO/X86/cfi-devirt.ll @@ -5,9 +5,7 @@ ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s ; Legacy PM -; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39436. ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \ -; RUN: -verify-machineinstrs=0 \ ; RUN: -o %t3 \ ; RUN: -r=%t.o,test,px \ ; RUN: -r=%t.o,_ZN1A1nEi,p \ @@ -24,9 +22,7 @@ ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR ; New PM -; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39436. ; RUN: llvm-lto2 run %t.o -save-temps -use-new-pm -pass-remarks=. \ -; RUN: -verify-machineinstrs=0 \ ; RUN: -o %t3 \ ; RUN: -r=%t.o,test,px \ ; RUN: -r=%t.o,_ZN1A1nEi,p \ @@ -50,7 +46,6 @@ ; to ensure it is being caught in the thin link. ; RUN: opt -thinlto-bc -o %t2.o %S/Inputs/empty.ll ; RUN: not llvm-lto2 run %t.o %t2.o -thinlto-distributed-indexes \ -; RUN: -verify-machineinstrs=0 \ ; RUN: -o %t3 \ ; RUN: -r=%t.o,test,px \ ; RUN: -r=%t.o,_ZN1A1nEi,p \ diff --git a/test/ThinLTO/X86/devirt-after-icp.ll b/test/ThinLTO/X86/devirt-after-icp.ll index fd5dcb728aa..8edb8698198 100644 --- a/test/ThinLTO/X86/devirt-after-icp.ll +++ b/test/ThinLTO/X86/devirt-after-icp.ll @@ -45,9 +45,7 @@ ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s ; Legacy PM -; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39436. ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \ -; RUN: -verify-machineinstrs=0 \ ; RUN: -o %t3 \ ; RUN: -r=%t.o,_Z3bazP1A,px \ ; RUN: -r=%t.o,_ZN1A3fooEv, \ @@ -65,9 +63,7 @@ ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR ; New PM -; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39436. ; RUN: llvm-lto2 run %t.o -save-temps -use-new-pm -pass-remarks=. \ -; RUN: -verify-machineinstrs=0 \ ; RUN: -o %t3 \ ; RUN: -r=%t.o,_Z3bazP1A,px \ ; RUN: -r=%t.o,_ZN1A3fooEv, \