JIT: revise checking for computed preds #81582
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.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
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. |
| @@ -4512,7 +4508,7 @@ class Compiler | |||
| #endif // FEATURE_JIT_METHOD_PERF | |||
|
|
|||
| 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.
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.
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. |
ghost
locked as resolved and limited conversation to collaborators
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.