From 68720204a7240167ef3853df50b54c8dd3504d79 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Tue, 1 Aug 2017 00:28:40 +0000 Subject: [PATCH] [ScheduleDAG] Don't schedule node with physical register interference https://reviews.llvm.org/D31536 didn't really solve the problem it was trying to solve; it got rid of the assertion failure, but we were still scheduling the DAG incorrectly (mixing together instructions from different calls), leading to a MachineVerifier failure. In order to schedule the DAG correctly, we have to make sure we don't schedule a node which should be blocked by an interference. Fix ScheduleDAGRRList::PickNodeToScheduleBottomUp so it doesn't pick a node like that. The added call to FindAvailableNode() is the key change here; this makes sure we don't try to schedule a call while we're in the middle of scheduling a different call. I'm not sure this is the right approach; in particular, I'm not sure how to prove we don't end up with an infinite loop of repeatedly backtracking. This also reverts the code change from D31536. It doesn't do anything useful: we should never schedule an ADJCALLSTACKDOWN unless we've already scheduled the corresponding ADJCALLSTACKUP. Differential Revision: https://reviews.llvm.org/D33818 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@309642 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../SelectionDAG/ScheduleDAGRRList.cpp | 62 +++++++++++-------- test/CodeGen/ARM/unschedule-first-call.ll | 2 +- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp b/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp index 70b1fa77a09..83324f869fb 100644 --- a/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp +++ b/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp @@ -550,6 +550,7 @@ void ScheduleDAGRRList::ReleasePredecessors(SUnit *SU) { unsigned NestLevel = 0; unsigned MaxNest = 0; SDNode *N = FindCallSeqStart(Node, NestLevel, MaxNest, TII); + assert(N && "Must find call sequence start"); SUnit *Def = &SUnits[N->getNodeId()]; CallSeqEndForStart[Def] = SU; @@ -821,9 +822,13 @@ void ScheduleDAGRRList::UnscheduleNodeBottomUp(SUnit *SU) { SUNode = SUNode->getGluedNode()) { if (SUNode->isMachineOpcode() && SUNode->getMachineOpcode() == TII->getCallFrameSetupOpcode()) { + SUnit *SeqEnd = CallSeqEndForStart[SU]; + assert(SeqEnd && "Call sequence start/end must be known"); + assert(!LiveRegDefs[CallResource]); + assert(!LiveRegGens[CallResource]); ++NumLiveRegs; LiveRegDefs[CallResource] = SU; - LiveRegGens[CallResource] = CallSeqEndForStart[SU]; + LiveRegGens[CallResource] = SeqEnd; } } @@ -835,6 +840,8 @@ void ScheduleDAGRRList::UnscheduleNodeBottomUp(SUnit *SU) { if (SUNode->isMachineOpcode() && SUNode->getMachineOpcode() == TII->getCallFrameDestroyOpcode()) { assert(NumLiveRegs > 0 && "NumLiveRegs is already zero!"); + assert(LiveRegDefs[CallResource]); + assert(LiveRegGens[CallResource]); --NumLiveRegs; LiveRegDefs[CallResource] = nullptr; LiveRegGens[CallResource] = nullptr; @@ -1319,8 +1326,7 @@ DelayForLiveRegsBottomUp(SUnit *SU, SmallVectorImpl &LRegs) { // If we're in the middle of scheduling a call, don't begin scheduling // another call. Also, don't allow any physical registers to be live across // the call. - if ((Node->getMachineOpcode() == TII->getCallFrameDestroyOpcode()) || - (Node->getMachineOpcode() == TII->getCallFrameSetupOpcode())) { + if (Node->getMachineOpcode() == TII->getCallFrameDestroyOpcode()) { // Check the special calling-sequence resource. unsigned CallResource = TRI->getNumRegs(); if (LiveRegDefs[CallResource]) { @@ -1390,27 +1396,30 @@ void ScheduleDAGRRList::releaseInterferences(unsigned Reg) { /// (3) No Interferences: may unschedule to break register interferences. SUnit *ScheduleDAGRRList::PickNodeToScheduleBottomUp() { SUnit *CurSU = AvailableQueue->empty() ? nullptr : AvailableQueue->pop(); - while (CurSU) { - SmallVector LRegs; - if (!DelayForLiveRegsBottomUp(CurSU, LRegs)) - break; - DEBUG(dbgs() << " Interfering reg " << - (LRegs[0] == TRI->getNumRegs() ? "CallResource" - : TRI->getName(LRegs[0])) - << " SU #" << CurSU->NodeNum << '\n'); - std::pair LRegsPair = - LRegsMap.insert(std::make_pair(CurSU, LRegs)); - if (LRegsPair.second) { - CurSU->isPending = true; // This SU is not in AvailableQueue right now. - Interferences.push_back(CurSU); - } - else { - assert(CurSU->isPending && "Interferences are pending"); - // Update the interference with current live regs. - LRegsPair.first->second = LRegs; + auto FindAvailableNode = [&]() { + while (CurSU) { + SmallVector LRegs; + if (!DelayForLiveRegsBottomUp(CurSU, LRegs)) + break; + DEBUG(dbgs() << " Interfering reg " << + (LRegs[0] == TRI->getNumRegs() ? "CallResource" + : TRI->getName(LRegs[0])) + << " SU #" << CurSU->NodeNum << '\n'); + std::pair LRegsPair = + LRegsMap.insert(std::make_pair(CurSU, LRegs)); + if (LRegsPair.second) { + CurSU->isPending = true; // This SU is not in AvailableQueue right now. + Interferences.push_back(CurSU); + } + else { + assert(CurSU->isPending && "Interferences are pending"); + // Update the interference with current live regs. + LRegsPair.first->second = LRegs; + } + CurSU = AvailableQueue->pop(); } - CurSU = AvailableQueue->pop(); - } + }; + FindAvailableNode(); if (CurSU) return CurSU; @@ -1447,13 +1456,16 @@ SUnit *ScheduleDAGRRList::PickNodeToScheduleBottomUp() { // If one or more successors has been unscheduled, then the current // node is no longer available. - if (!TrySU->isAvailable || !TrySU->NodeQueueId) + if (!TrySU->isAvailable || !TrySU->NodeQueueId) { + DEBUG(dbgs() << "TrySU not available; choosing node from queue\n"); CurSU = AvailableQueue->pop(); - else { + } else { + DEBUG(dbgs() << "TrySU available\n"); // Available and in AvailableQueue AvailableQueue->remove(TrySU); CurSU = TrySU; } + FindAvailableNode(); // Interferences has been mutated. We must break. break; } diff --git a/test/CodeGen/ARM/unschedule-first-call.ll b/test/CodeGen/ARM/unschedule-first-call.ll index 4a218afcc5e..6fb0b2281ab 100644 --- a/test/CodeGen/ARM/unschedule-first-call.ll +++ b/test/CodeGen/ARM/unschedule-first-call.ll @@ -1,4 +1,4 @@ -; RUN: llc < %s +; RUN: llc -verify-machineinstrs < %s ; PR30911 target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64" -- 2.40.0