-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Reduce differences between successor iterators #93445
Reduce differences between successor iterators #93445
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThe NumSucc/GetSucc/Succs iterators have two variants. It is hard to understand the distinction. The goal of this change is to reduce the differences, so the only difference is the SWITCH case (iterating either all successors or only unique successors). So:
|
@AndyAyersMS PTAL |
src/coreclr/jit/importer.cpp
Outdated
@@ -11528,22 +11528,30 @@ void Compiler::impImportBlock(BasicBlock* block) | |||
impReimportSpillClique(block); | |||
|
|||
// For blocks that haven't been imported yet, we still need to mark them as pending import. | |||
for (BasicBlock* const succ : block->Succs()) | |||
// Filter successor from BBJ_EHFILTERRET have already been handled, above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't figure out where the "above" was. Would be nice to avoid this special case.
Seems like we should never get into the spill clique case for filters since the exit stack will be emtpy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I can remove this from the spill clique case if you think it's unnecessary. (Just assert block is not a filterret?) I was just being defensive, since I hit an assert without this in the "normal" case.
The NumSucc/GetSucc/Succs iterators have two variants. It is hard to understand the distinction. The goal of this change is to reduce the differences, so the only difference is the SWITCH case (iterating either all successors or only unique successors). So: 1. BBJ_EHFILTERRET: always return the single successor, which is the first block of the filter handler. 2. BBJ_EHFINALLYRET: Move to standard successor iterator
2ae9a3a
to
cf4e02b
Compare
@AndyAyersMS updated |
The NumSucc/GetSucc/Succs iterators have two variants. It is hard to understand the distinction. The goal of this change is to reduce the differences, so the only difference is the SWITCH case (iterating either all successors or only unique successors).
So: