Skip to content

Commit

Permalink
[MISched][NFC] Add documentation comment in pickNode for ReadyQueue m…
Browse files Browse the repository at this point in the history
…aintenence

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.
  • Loading branch information
michaelmaitland committed May 22, 2024
1 parent 9f23138 commit b39173e
Showing 1 changed file with 15 additions and 0 deletions.
15 changes: 15 additions & 0 deletions llvm/lib/CodeGen/MachineScheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3777,6 +3777,21 @@ SUnit *GenericScheduler::pickNode(bool &IsTopNode) {
}
} while (SU->isScheduled);

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

0 comments on commit b39173e

Please sign in to comment.