Skip to content

Commit 71b1fbd

Browse files
[MISched][NFC] Add documentation comment in pickNode for ReadyQueue maintenence (#92976)
I had some trouble understanding why `removeReady` removed nodes from the Pending queue, since my intuition told me that the Pending queue did not represent a node that was ready. I took a deeper look and found that pickOnlyNode and pickNodeFromQueue only picked nodes from the Available queue too. I found that need to nodes from the Available and Pending queues that correspond to the opposite direction that we ended up choosing from (IsTopNode vs !IsTopNode). It took me a little longer than I would have liked to understand this fact, so I figured that I would add a comment in the code that makes it clear for future readers.
1 parent 2aa218c commit 71b1fbd

File tree

1 file changed

+15
-0
lines changed

1 file changed

+15
-0
lines changed

llvm/lib/CodeGen/MachineScheduler.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3777,6 +3777,21 @@ SUnit *GenericScheduler::pickNode(bool &IsTopNode) {
37773777
}
37783778
} while (SU->isScheduled);
37793779

3780+
// If IsTopNode, then SU is in Top.Available and must be removed. Otherwise,
3781+
// if isTopReady(), then SU is in either Top.Available or Top.Pending.
3782+
// If !IsTopNode, then SU is in Bot.Available and must be removed. Otherwise,
3783+
// if isBottomReady(), then SU is in either Bot.Available or Bot.Pending.
3784+
//
3785+
// It is coincidental when !IsTopNode && isTopReady or when IsTopNode &&
3786+
// isBottomReady. That is, it didn't factor into the decision to choose SU
3787+
// because it isTopReady or isBottomReady, respectively. In fact, if the
3788+
// RegionPolicy is OnlyTopDown or OnlyBottomUp, then the Bot queues and Top
3789+
// queues respectivley contain the original roots and don't get updated when
3790+
// picking a node. So if SU isTopReady on a OnlyBottomUp pick, then it was
3791+
// because we schduled everything but the top roots. Conversley, if SU
3792+
// isBottomReady on OnlyTopDown, then it was because we scheduled everything
3793+
// but the bottom roots. If its in a queue even coincidentally, it should be
3794+
// removed so it does not get re-picked in a subsequent pickNode call.
37803795
if (SU->isTopReady())
37813796
Top.removeReady(SU);
37823797
if (SU->isBottomReady())

0 commit comments

Comments
 (0)