diff options
author | Michael Maitland <michaeltmaitland@gmail.com> | 2024-05-22 08:27:35 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-22 08:27:35 -0400 |
commit | 71b1fbdff6cf567ad278c51f0acdcdf23de0ac28 (patch) | |
tree | 7be2864fccf1d17b7e09a4e14325771cf87e9f5a | |
parent | 2aa218c247eef03f4ea922d635b7a9f46d061119 (diff) | |
download | llvm-71b1fbdff6cf567ad278c51f0acdcdf23de0ac28.zip llvm-71b1fbdff6cf567ad278c51f0acdcdf23de0ac28.tar.gz llvm-71b1fbdff6cf567ad278c51f0acdcdf23de0ac28.tar.bz2 |
[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.
-rw-r--r-- | llvm/lib/CodeGen/MachineScheduler.cpp | 15 |
1 files changed, 15 insertions, 0 deletions
diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp index 0858be6..03e892a5 100644 --- a/llvm/lib/CodeGen/MachineScheduler.cpp +++ b/llvm/lib/CodeGen/MachineScheduler.cpp @@ -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()) |