From f828a0ccc7ecd54e71240e404f6c47c822ea6619 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Thu, 1 Oct 2015 18:44:59 +0000 Subject: [WinEH] Make FuncletLayout more robust against catchret Catchret transfers control from a catch funclet to an earlier funclet. However, it is not completely clear which funclet the catchret target is part of. Make this clear by stapling the catchret target's funclet membership onto the CATCHRET SDAG node. llvm-svn: 249052 --- llvm/lib/CodeGen/FuncletLayout.cpp | 37 ++++++++- .../CodeGen/SelectionDAG/FunctionLoweringInfo.cpp | 4 +- .../CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 14 +++- llvm/lib/CodeGen/WinEHPrepare.cpp | 97 ++++++++++++++++++---- llvm/lib/Target/X86/X86FrameLowering.cpp | 12 --- llvm/lib/Target/X86/X86InstrCompiler.td | 8 +- llvm/lib/Target/X86/X86InstrInfo.cpp | 8 +- llvm/lib/Target/X86/X86MCInstLower.cpp | 29 +++++++ 8 files changed, 169 insertions(+), 40 deletions(-) (limited to 'llvm/lib') diff --git a/llvm/lib/CodeGen/FuncletLayout.cpp b/llvm/lib/CodeGen/FuncletLayout.cpp index 0cda11f53db..e1e185b4752 100644 --- a/llvm/lib/CodeGen/FuncletLayout.cpp +++ b/llvm/lib/CodeGen/FuncletLayout.cpp @@ -16,6 +16,8 @@ #include "llvm/CodeGen/MachineFunction.h" #include "llvm/CodeGen/MachineFunctionPass.h" #include "llvm/CodeGen/MachineModuleInfo.h" +#include "llvm/Target/TargetInstrInfo.h" +#include "llvm/Target/TargetSubtargetInfo.h" using namespace llvm; #define DEBUG_TYPE "funclet-layout" @@ -36,8 +38,21 @@ static void collectFuncletMembers(DenseMap &FuncletMembership, int Funclet, MachineBasicBlock *MBB) { // Don't revisit blocks. - if (FuncletMembership.count(MBB) > 0) + if (FuncletMembership.count(MBB) > 0) { + // FIXME: This is a hack, we need to assert this unconditionally. + bool IsProbablyUnreachableBlock = + MBB->empty() || + (MBB->succ_empty() && !MBB->getFirstTerminator()->isReturn() && + MBB->size() == 1); + + if (!IsProbablyUnreachableBlock) { + if (FuncletMembership[MBB] != Funclet) { + assert(false && "MBB is part of two funclets!"); + report_fatal_error("MBB is part of two funclets!"); + } + } return; + } // Add this MBB to our funclet. FuncletMembership[MBB] = Funclet; @@ -71,18 +86,36 @@ bool FuncletLayout::runOnMachineFunction(MachineFunction &F) { if (!F.getMMI().hasEHFunclets()) return false; + const TargetInstrInfo *TII = F.getSubtarget().getInstrInfo(); SmallVector FuncletBlocks; - for (MachineBasicBlock &MBB : F) + SmallVector, 16> CatchRetSuccessors; + for (MachineBasicBlock &MBB : F) { if (MBB.isEHFuncletEntry()) FuncletBlocks.push_back(&MBB); + MachineBasicBlock::iterator MBBI = MBB.getFirstTerminator(); + if (MBBI->getOpcode() != TII->getCatchReturnOpcode()) + continue; + + MachineBasicBlock *Successor = MBBI->getOperand(0).getMBB(); + MachineBasicBlock *SuccessorColor = MBBI->getOperand(1).getMBB(); + CatchRetSuccessors.push_back({Successor, SuccessorColor->getNumber()}); + } + // We don't have anything to do if there aren't any EH pads. if (FuncletBlocks.empty()) return false; DenseMap FuncletMembership; + // Identify all the basic blocks reachable from the function entry. + collectFuncletMembers(FuncletMembership, F.front().getNumber(), F.begin()); + // Next, identify all the blocks inside the funclets. for (MachineBasicBlock *MBB : FuncletBlocks) collectFuncletMembers(FuncletMembership, MBB->getNumber(), MBB); + // Finally, identify all the targets of a catchret. + for (std::pair CatchRetPair : CatchRetSuccessors) + collectFuncletMembers(FuncletMembership, CatchRetPair.second, + CatchRetPair.first); F.sort([&](MachineBasicBlock &x, MachineBasicBlock &y) { return FuncletMembership[&x] < FuncletMembership[&y]; diff --git a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp index baf9ee990ea..7da68deaaa3 100644 --- a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp @@ -290,9 +290,11 @@ void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf, const Function *WinEHParentFn = MMI.getWinEHParent(&fn); if (Personality == EHPersonality::MSVC_CXX) calculateWinCXXEHStateNumbers(WinEHParentFn, EHInfo); - else + else if (isAsynchronousEHPersonality(Personality)) calculateSEHStateNumbers(WinEHParentFn, EHInfo); + calculateCatchReturnSuccessorColors(WinEHParentFn, EHInfo); + // Map all BB references in the WinEH data to MBBs. for (WinEHTryBlockMapEntry &TBME : EHInfo.TryBlockMap) { for (WinEHHandlerType &H : TBME.HandlerArray) { diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 4f9656532c2..f779fd1b03a 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -1168,9 +1168,21 @@ void SelectionDAGBuilder::visitCatchRet(const CatchReturnInst &I) { MachineBasicBlock *TargetMBB = FuncInfo.MBBMap[I.getSuccessor()]; FuncInfo.MBB->addSuccessor(TargetMBB); + // Figure out the funclet membership for the catchret's successor. + // This will be used by the FuncletLayout pass to determine how to order the + // BB's. + MachineModuleInfo &MMI = DAG.getMachineFunction().getMMI(); + WinEHFuncInfo &EHInfo = + MMI.getWinEHFuncInfo(DAG.getMachineFunction().getFunction()); + const BasicBlock *SuccessorColor = EHInfo.CatchRetSuccessorColorMap[&I]; + assert(SuccessorColor && "No parent funclet for catchret!"); + MachineBasicBlock *SuccessorColorMBB = FuncInfo.MBBMap[SuccessorColor]; + assert(SuccessorColorMBB && "No MBB for SuccessorColor!"); + // Create the terminator node. SDValue Ret = DAG.getNode(ISD::CATCHRET, getCurSDLoc(), MVT::Other, - getControlRoot(), DAG.getBasicBlock(TargetMBB)); + getControlRoot(), DAG.getBasicBlock(TargetMBB), + DAG.getBasicBlock(SuccessorColorMBB)); DAG.setRoot(Ret); } diff --git a/llvm/lib/CodeGen/WinEHPrepare.cpp b/llvm/lib/CodeGen/WinEHPrepare.cpp index 7240159b083..07734c352c0 100644 --- a/llvm/lib/CodeGen/WinEHPrepare.cpp +++ b/llvm/lib/CodeGen/WinEHPrepare.cpp @@ -399,6 +399,29 @@ FunctionPass *llvm::createWinEHPass(const TargetMachine *TM) { return new WinEHPrepare(TM); } +static bool +findExceptionalConstructs(Function &Fn, + SmallVectorImpl &LPads, + SmallVectorImpl &Resumes, + SmallVectorImpl &EntryBlocks) { + bool ForExplicitEH = false; + for (BasicBlock &BB : Fn) { + Instruction *First = BB.getFirstNonPHI(); + if (auto *LP = dyn_cast(First)) { + LPads.push_back(LP); + } else if (First->isEHPad()) { + if (!ForExplicitEH) + EntryBlocks.push_back(&Fn.getEntryBlock()); + if (!isa(First) && !isa(First)) + EntryBlocks.push_back(&BB); + ForExplicitEH = true; + } + if (auto *Resume = dyn_cast(BB.getTerminator())) + Resumes.push_back(Resume); + } + return ForExplicitEH; +} + bool WinEHPrepare::runOnFunction(Function &Fn) { if (!Fn.hasPersonalityFn()) return false; @@ -417,21 +440,8 @@ bool WinEHPrepare::runOnFunction(Function &Fn) { SmallVector LPads; SmallVector Resumes; SmallVector EntryBlocks; - bool ForExplicitEH = false; - for (BasicBlock &BB : Fn) { - Instruction *First = BB.getFirstNonPHI(); - if (auto *LP = dyn_cast(First)) { - LPads.push_back(LP); - } else if (First->isEHPad()) { - if (!ForExplicitEH) - EntryBlocks.push_back(&Fn.getEntryBlock()); - if (!isa(First) && !isa(First)) - EntryBlocks.push_back(&BB); - ForExplicitEH = true; - } - if (auto *Resume = dyn_cast(BB.getTerminator())) - Resumes.push_back(Resume); - } + bool ForExplicitEH = + findExceptionalConstructs(Fn, LPads, Resumes, EntryBlocks); if (ForExplicitEH) return prepareExplicitEH(Fn, EntryBlocks); @@ -2878,8 +2888,11 @@ void WinEHPrepare::replaceTerminatePadWithCleanup(Function &F) { } } -void WinEHPrepare::colorFunclets(Function &F, - SmallVectorImpl &EntryBlocks) { +static void +colorFunclets(Function &F, SmallVectorImpl &EntryBlocks, + std::map> &BlockColors, + std::map> &FuncletBlocks, + std::map> &FuncletChildren) { SmallVector, 16> Worklist; BasicBlock *EntryBlock = &F.getEntryBlock(); @@ -2976,6 +2989,56 @@ void WinEHPrepare::colorFunclets(Function &F, } } +void WinEHPrepare::colorFunclets(Function &F, + SmallVectorImpl &EntryBlocks) { + ::colorFunclets(F, EntryBlocks, BlockColors, FuncletBlocks, FuncletChildren); +} + +void llvm::calculateCatchReturnSuccessorColors(const Function *Fn, + WinEHFuncInfo &FuncInfo) { + SmallVector LPads; + SmallVector Resumes; + SmallVector EntryBlocks; + // colorFunclets needs the set of EntryBlocks, get them using + // findExceptionalConstructs. + bool ForExplicitEH = findExceptionalConstructs(const_cast(*Fn), + LPads, Resumes, EntryBlocks); + if (!ForExplicitEH) + return; + + std::map> BlockColors; + std::map> FuncletBlocks; + std::map> FuncletChildren; + // Figure out which basic blocks belong to which funclets. + colorFunclets(const_cast(*Fn), EntryBlocks, BlockColors, + FuncletBlocks, FuncletChildren); + + // We need to find the catchret successors. To do this, we must first find + // all the catchpad funclets. + for (auto &Funclet : FuncletBlocks) { + // Figure out what kind of funclet we are looking at; We only care about + // catchpads. + BasicBlock *FuncletPadBB = Funclet.first; + Instruction *FirstNonPHI = FuncletPadBB->getFirstNonPHI(); + auto *CatchPad = dyn_cast(FirstNonPHI); + if (!CatchPad) + continue; + + // The users of a catchpad are always catchrets. + for (User *Exit : CatchPad->users()) { + auto *CatchReturn = cast(Exit); + BasicBlock *CatchRetSuccessor = CatchReturn->getSuccessor(); + std::set &SuccessorColors = BlockColors[CatchRetSuccessor]; + assert(SuccessorColors.size() == 1 && "Expected BB to be monochrome!"); + BasicBlock *Color = *SuccessorColors.begin(); + if (auto *CPI = dyn_cast(Color->getFirstNonPHI())) + Color = CPI->getNormalDest(); + // Record the catchret successor's funclet membership. + FuncInfo.CatchRetSuccessorColorMap[CatchReturn] = Color; + } + } +} + void WinEHPrepare::demotePHIsOnFunclets(Function &F) { // Strip PHI nodes off of EH pads. SmallVector PHINodes; diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp index ca5d5702813..fc192cd6bfd 100644 --- a/llvm/lib/Target/X86/X86FrameLowering.cpp +++ b/llvm/lib/Target/X86/X86FrameLowering.cpp @@ -1120,24 +1120,12 @@ void X86FrameLowering::emitEpilogue(MachineFunction &MF, BuildMI(*RestoreMBB, RestoreMBBI, DL, TII.get(X86::JMP_4)) .addMBB(TargetMBB); } - // Replace CATCHRET with the appropriate RET. - unsigned RetOp = STI.is64Bit() ? X86::RETQ : X86::RETL; - MachineBasicBlock::iterator NewExit = - BuildMI(MBB, MBBI, DL, TII.get(RetOp)).addReg(ReturnReg); - MBBI->eraseFromParent(); - MBBI = NewExit; } else if (MBBI->getOpcode() == X86::CLEANUPRET) { NumBytes = MFI->getMaxCallFrameSize(); assert(hasFP(MF) && "EH funclets without FP not yet implemented"); BuildMI(MBB, MBBI, DL, TII.get(Is64Bit ? X86::POP64r : X86::POP32r), MachineFramePtr) .setMIFlag(MachineInstr::FrameDestroy); - // Replace CLEANUPRET with the appropriate RET. - unsigned RetOp = STI.is64Bit() ? X86::RETQ : X86::RETL; - MachineBasicBlock::iterator NewExit = - BuildMI(MBB, MBBI, DL, TII.get(RetOp)); - MBBI->eraseFromParent(); - MBBI = NewExit; } else if (hasFP(MF)) { // Calculate required stack adjustment. uint64_t FrameSize = StackSize - SlotSize; diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td index 8d94aaa0eac..654d8cc98fa 100644 --- a/llvm/lib/Target/X86/X86InstrCompiler.td +++ b/llvm/lib/Target/X86/X86InstrCompiler.td @@ -152,10 +152,10 @@ def EH_RETURN64 : I<0xC3, RawFrm, (outs), (ins GR64:$addr), } -let isTerminator = 1, isBarrier = 1, hasCtrlDep = 1, isCodeGenOnly = 1, isReturn = 1 in { -def CATCHRET : I<0, Pseudo, (outs), (ins brtarget32:$dst), - "# CATCHRET", - [(catchret bb:$dst)]>; +let isTerminator = 1, hasSideEffects = 1, isBarrier = 1, hasCtrlDep = 1, isCodeGenOnly = 1, isReturn = 1 in { +def CATCHRET : I<0, Pseudo, (outs), (ins brtarget32:$dst, brtarget32:$from), + "# CATCHRET", + [(catchret bb:$dst, bb:$from)]>; def CLEANUPRET : I<0, Pseudo, (outs), (ins), "# CLEANUPRET", [(cleanupret)]>; } diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index 71b5ab4770a..b06eefb0727 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -101,9 +101,11 @@ struct X86MemoryFoldTableEntry { void X86InstrInfo::anchor() {} X86InstrInfo::X86InstrInfo(X86Subtarget &STI) - : X86GenInstrInfo( - (STI.isTarget64BitLP64() ? X86::ADJCALLSTACKDOWN64 : X86::ADJCALLSTACKDOWN32), - (STI.isTarget64BitLP64() ? X86::ADJCALLSTACKUP64 : X86::ADJCALLSTACKUP32)), + : X86GenInstrInfo((STI.isTarget64BitLP64() ? X86::ADJCALLSTACKDOWN64 + : X86::ADJCALLSTACKDOWN32), + (STI.isTarget64BitLP64() ? X86::ADJCALLSTACKUP64 + : X86::ADJCALLSTACKUP32), + X86::CATCHRET), Subtarget(STI), RI(STI.getTargetTriple()) { static const X86MemoryFoldTableEntry MemoryFoldTable2Addr[] = { diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp index c61dc125ebc..e3a107ba224 100644 --- a/llvm/lib/Target/X86/X86MCInstLower.cpp +++ b/llvm/lib/Target/X86/X86MCInstLower.cpp @@ -531,6 +531,23 @@ ReSimplify: break; } + case X86::CLEANUPRET: { + // Replace CATCHRET with the appropriate RET. + OutMI = MCInst(); + OutMI.setOpcode(getRetOpcode(AsmPrinter.getSubtarget())); + break; + } + + case X86::CATCHRET: { + // Replace CATCHRET with the appropriate RET. + const X86Subtarget &Subtarget = AsmPrinter.getSubtarget(); + unsigned ReturnReg = Subtarget.is64Bit() ? X86::RAX : X86::EAX; + OutMI = MCInst(); + OutMI.setOpcode(getRetOpcode(Subtarget)); + OutMI.addOperand(MCOperand::createReg(ReturnReg)); + break; + } + // TAILJMPd, TAILJMPd64 - Lower to the correct jump instructions. case X86::TAILJMPr: case X86::TAILJMPd: @@ -1076,6 +1093,18 @@ void X86AsmPrinter::EmitInstruction(const MachineInstr *MI) { X86ATTInstPrinter::getRegisterName(Reg)); break; } + case X86::CLEANUPRET: { + // Lower these as normal, but add some comments. + OutStreamer->AddComment("CLEANUPRET"); + break; + } + + case X86::CATCHRET: { + // Lower these as normal, but add some comments. + OutStreamer->AddComment("CATCHRET"); + break; + } + case X86::TAILJMPr: case X86::TAILJMPm: case X86::TAILJMPd: -- cgit v1.2.3