Skip to content

Commit 808823c

Browse files
committed
Fix the MachineScheduler's logic for updating ready times for in-order.
Now the scheduler updates a node's ready time as soon as it is scheduled, before releasing dependent nodes. There was a reason I didn't do this initially but it no longer applies. A53 is in-order and was running into an issue where nodes where added to the readyQ too early. That's now fixed. This also makes it easier for custom scheduling strategies to build heuristics based on the actual cycles that the node was scheduled at. The only impact on OOO (sandybridge/cyclone) is that ready times will be slightly more accurate. I didn't measure any significant regressions. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@210390 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 7fe8882 commit 808823c

File tree

2 files changed

+27
-34
lines changed

2 files changed

+27
-34
lines changed

include/llvm/CodeGen/MachineScheduler.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -624,9 +624,9 @@ class SchedBoundary {
624624
SmallVector<unsigned, 16> ReservedCycles;
625625

626626
#ifndef NDEBUG
627-
// Remember the greatest operand latency as an upper bound on the number of
627+
// Remember the greatest possible stall as an upper bound on the number of
628628
// times we should retry the pending queue because of a hazard.
629-
unsigned MaxObservedLatency;
629+
unsigned MaxObservedStall;
630630
#endif
631631

632632
public:

lib/CodeGen/MachineScheduler.cpp

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,11 @@ void ScheduleDAGMI::releaseSucc(SUnit *SU, SDep *SuccEdge) {
535535
llvm_unreachable(nullptr);
536536
}
537537
#endif
538+
// SU->TopReadyCycle was set to CurrCycle when it was scheduled. However,
539+
// CurrCycle may have advanced since then.
540+
if (SuccSU->TopReadyCycle < SU->TopReadyCycle + SuccEdge->getLatency())
541+
SuccSU->TopReadyCycle = SU->TopReadyCycle + SuccEdge->getLatency();
542+
538543
--SuccSU->NumPredsLeft;
539544
if (SuccSU->NumPredsLeft == 0 && SuccSU != &ExitSU)
540545
SchedImpl->releaseTopNode(SuccSU);
@@ -569,6 +574,11 @@ void ScheduleDAGMI::releasePred(SUnit *SU, SDep *PredEdge) {
569574
llvm_unreachable(nullptr);
570575
}
571576
#endif
577+
// SU->BotReadyCycle was set to CurrCycle when it was scheduled. However,
578+
// CurrCycle may have advanced since then.
579+
if (PredSU->BotReadyCycle < SU->BotReadyCycle + PredEdge->getLatency())
580+
PredSU->BotReadyCycle = SU->BotReadyCycle + PredEdge->getLatency();
581+
572582
--PredSU->NumSuccsLeft;
573583
if (PredSU->NumSuccsLeft == 0 && PredSU != &EntrySU)
574584
SchedImpl->releaseBottomNode(PredSU);
@@ -680,10 +690,13 @@ void ScheduleDAGMI::schedule() {
680690
CurrentBottom = MI;
681691
}
682692
}
683-
updateQueues(SU, IsTopNode);
684-
685-
// Notify the scheduling strategy after updating the DAG.
693+
// Notify the scheduling strategy before updating the DAG.
694+
// This sets the scheduled nodes ReadyCycle to CurrCycle. When updateQueues
695+
// runs, it can then use the accurate ReadyCycle time to determine whether
696+
// newly released nodes can move to the readyQ.
686697
SchedImpl->schedNode(SU, IsTopNode);
698+
699+
updateQueues(SU, IsTopNode);
687700
}
688701
assert(CurrentTop == CurrentBottom && "Nonempty unscheduled zone.");
689702

@@ -1574,7 +1587,7 @@ void SchedBoundary::reset() {
15741587
// Track the maximum number of stall cycles that could arise either from the
15751588
// latency of a DAG edge or the number of cycles that a processor resource is
15761589
// reserved (SchedBoundary::ReservedCycles).
1577-
MaxObservedLatency = 0;
1590+
MaxObservedStall = 0;
15781591
#endif
15791592
// Reserve a zero-count for invalid CritResIdx.
15801593
ExecutedResCounts.resize(1);
@@ -1731,6 +1744,12 @@ getOtherResourceCount(unsigned &OtherCritIdx) {
17311744
}
17321745

17331746
void SchedBoundary::releaseNode(SUnit *SU, unsigned ReadyCycle) {
1747+
assert(SU->getInstr() && "Scheduled SUnit must have instr");
1748+
1749+
#ifndef NDEBUG
1750+
MaxObservedStall = std::max(ReadyCycle - CurrCycle, MaxObservedStall);
1751+
#endif
1752+
17341753
if (ReadyCycle < MinReadyCycle)
17351754
MinReadyCycle = ReadyCycle;
17361755

@@ -1750,39 +1769,13 @@ void SchedBoundary::releaseTopNode(SUnit *SU) {
17501769
if (SU->isScheduled)
17511770
return;
17521771

1753-
for (SUnit::pred_iterator I = SU->Preds.begin(), E = SU->Preds.end();
1754-
I != E; ++I) {
1755-
if (I->isWeak())
1756-
continue;
1757-
unsigned PredReadyCycle = I->getSUnit()->TopReadyCycle;
1758-
unsigned Latency = I->getLatency();
1759-
#ifndef NDEBUG
1760-
MaxObservedLatency = std::max(Latency, MaxObservedLatency);
1761-
#endif
1762-
if (SU->TopReadyCycle < PredReadyCycle + Latency)
1763-
SU->TopReadyCycle = PredReadyCycle + Latency;
1764-
}
17651772
releaseNode(SU, SU->TopReadyCycle);
17661773
}
17671774

17681775
void SchedBoundary::releaseBottomNode(SUnit *SU) {
17691776
if (SU->isScheduled)
17701777
return;
17711778

1772-
assert(SU->getInstr() && "Scheduled SUnit must have instr");
1773-
1774-
for (SUnit::succ_iterator I = SU->Succs.begin(), E = SU->Succs.end();
1775-
I != E; ++I) {
1776-
if (I->isWeak())
1777-
continue;
1778-
unsigned SuccReadyCycle = I->getSUnit()->BotReadyCycle;
1779-
unsigned Latency = I->getLatency();
1780-
#ifndef NDEBUG
1781-
MaxObservedLatency = std::max(Latency, MaxObservedLatency);
1782-
#endif
1783-
if (SU->BotReadyCycle < SuccReadyCycle + Latency)
1784-
SU->BotReadyCycle = SuccReadyCycle + Latency;
1785-
}
17861779
releaseNode(SU, SU->BotReadyCycle);
17871780
}
17881781

@@ -1951,7 +1944,7 @@ void SchedBoundary::bumpNode(SUnit *SU) {
19511944
if (SchedModel->getProcResource(PIdx)->BufferSize == 0) {
19521945
ReservedCycles[PIdx] = isTop() ? NextCycle + PI->Cycles : NextCycle;
19531946
#ifndef NDEBUG
1954-
MaxObservedLatency = std::max(PI->Cycles, MaxObservedLatency);
1947+
MaxObservedStall = std::max(PI->Cycles, MaxObservedStall);
19551948
#endif
19561949
}
19571950
}
@@ -2055,7 +2048,7 @@ SUnit *SchedBoundary::pickOnlyChoice() {
20552048
}
20562049
}
20572050
for (unsigned i = 0; Available.empty(); ++i) {
2058-
assert(i <= (HazardRec->getMaxLookAhead() + MaxObservedLatency) &&
2051+
assert(i <= (HazardRec->getMaxLookAhead() + MaxObservedStall) &&
20592052
"permanent hazard"); (void)i;
20602053
bumpCycle(CurrCycle + 1);
20612054
releasePending();

0 commit comments

Comments
 (0)