aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Maitland <michaeltmaitland@gmail.com>2024-05-22 08:27:35 -0400
committerGitHub <noreply@github.com>2024-05-22 08:27:35 -0400
commit71b1fbdff6cf567ad278c51f0acdcdf23de0ac28 (patch)
tree7be2864fccf1d17b7e09a4e14325771cf87e9f5a
parent2aa218c247eef03f4ea922d635b7a9f46d061119 (diff)
downloadllvm-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.cpp15
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())