-
Notifications
You must be signed in to change notification settings - Fork 5.3k
JIT: revise checking for computed preds #81582
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
Conversation
Since pred lists now exist pervasively, change most methods that had conditional pred list updates to assert preds exist and always update. To make sure I got all uses, I renamed `fgComputePredsDone` to `fgPredsComputed`. Remove the ability to drop EH, as it doesn't update pred lists properly and so has been broken for quite a while now. Remove some of the logic for fixing up finally targets, since we now always have pred lists around and so don't need the blanket invaliation. Closes dotnet#80193.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsSince pred lists now exist pervasively, change most methods that had conditional pred list updates to assert preds exist and always update. To make sure I got all uses, I renamed Remove the ability to drop EH, as it doesn't update pred lists properly and so has been broken for quite a while now. Remove some of the logic for fixing up finally targets, since we now always have pred lists around and so don't need the blanket invaliation. Closes #80193.
|
|
@EgorBo PTAL This is the final bit of revision for pred lists. I may still try and encapsulate the remove/add ref patterns that are now fairly pervasive but will likely instead start building upon the new capabilities that persistent pred lists offer. |
|
|
||
| bool fgModified; // True if the flow graph has been modified recently | ||
| bool fgComputePredsDone; // Have we computed the bbPreds list | ||
| bool fgPredsComputed; // Have we computed the bbPreds list |
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.
Can you educate me why we need this at all? I assume it's false only during importing?
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.
We actually build pred lists before importation, so fgPredsComputed is true for all phases. So it seems like something we don't need. But I kept this concept around for two reasons:
- we sometimes do a bit of flow graph modification very early before we have added preds lists, and call methods that are also called after we have pred edges; those methods need to behave differently before and after (mainly
fgEnsureFirstBBisScratchand friends - we want to make sure no other pred-list sensitive method is called early.
|
Failure is known issue. |
Since pred lists now exist pervasively, change most methods that had conditional pred list updates to assert preds exist and always update.
To make sure I got all uses, I renamed
fgComputePredsDonetofgPredsComputed.Remove the ability to drop EH, as it doesn't update pred lists properly and so has been broken for quite a while now.
Remove some of the logic for fixing up finally targets, since we now always have pred lists around and so don't need the blanket invaliation.
Closes #80193.