[llvm] r309642 - [ScheduleDAG] Don't schedule node with physical register interference

Eli Friedman via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 31 17:28:40 PDT 2017


Author: efriedma
Date: Mon Jul 31 17:28:40 2017
New Revision: 309642

URL: http://llvm.org/viewvc/llvm-project?rev=309642&view=rev
Log:
[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


Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
    llvm/trunk/test/CodeGen/ARM/unschedule-first-call.ll

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp?rev=309642&r1=309641&r2=309642&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp Mon Jul 31 17:28:40 2017
@@ -550,6 +550,7 @@ void ScheduleDAGRRList::ReleasePredecess
         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::UnscheduleNodeBo
        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::UnscheduleNodeBo
       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, Smal
     // 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::releaseInterfere
 /// (3) No Interferences: may unschedule to break register interferences.
 SUnit *ScheduleDAGRRList::PickNodeToScheduleBottomUp() {
   SUnit *CurSU = AvailableQueue->empty() ? nullptr : AvailableQueue->pop();
-  while (CurSU) {
-    SmallVector<unsigned, 4> 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<LRegsMapT::iterator, bool> 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<unsigned, 4> 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<LRegsMapT::iterator, bool> 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::PickNodeToSche
 
       // 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;
     }

Modified: llvm/trunk/test/CodeGen/ARM/unschedule-first-call.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/unschedule-first-call.ll?rev=309642&r1=309641&r2=309642&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/unschedule-first-call.ll (original)
+++ llvm/trunk/test/CodeGen/ARM/unschedule-first-call.ll Mon Jul 31 17:28:40 2017
@@ -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"




More information about the llvm-commits mailing list