Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MISched][NFC] Add documentation comment in pickNode for ReadyQueue maintenence #92976

Merged
merged 1 commit into from
May 22, 2024

Conversation

michaelmaitland
Copy link
Contributor

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.

@michaelmaitland michaelmaitland added the mi-sched machine instruction scheduler label May 22, 2024
@michaelmaitland michaelmaitland requested a review from wangpc-pp May 22, 2024 00:31
…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.
Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't take a deep look at this part, but I think it makes sense.

@michaelmaitland michaelmaitland merged commit 71b1fbd into llvm:main May 22, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mi-sched machine instruction scheduler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants