JIT: build pred lists first #81448
JIT: build pred lists first #81448
Conversation
Move pred list building before importation. It now runs as the first phase in the phase list. * Split up some unions in block.h as some things can't share storage anymore (may revisit this later). * Modify importer not to rely on cheap preds. Most of the work here is in `impImportLeave`. * Adjust OSR protection strategy for the method entry. In particular, watch for the degenerate case where the OSR entry is the method entry. * Ensure we don't double-decrement some ref counts when blocks with degenerate or folded flow are re-imported. * Update spill clique discovery to use the actual pred lists. * Add new method `impFixPredLists` for the importer to use at the end of importation. This adds pred list entries finally returns, which can't be done until all `BBJ_LEAVE` blocks are processed. Contributes to dotnet#80193.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsMove pred list building before importation. It now runs as the first phase in the phase list.
Contributes to #80193.
|
|
@jakobbotsch PTAL (and/or @BruceForstall if you're only quasi-vacationing) This gets pred list building to the front of the phase list and removes one more user of cheap pred lists. Some small diffs in methods with patchpoints; the adaptive placement strategy is sensitive to Next step will be to build pred lists when we initially build the flow graph and get rid of the pred list building phase and the last user of cheap preds (eh normalization). Then I'm envisioning a bunch more cleanup as it seems like we can just do "automatic" maintenance of all these references. |
|
Failing on some bad IL tests. Note even with main these tests will fail if dumps are enabled, as we hit an assert trying to display the successors of an endfinally in the BB table. Looks like we will now need to BADCODE when building pred lists; odd that we don't catch this sooner. |
|
TP impact looks high (with minopts close to 1%), but drilling in, the only collection actually slower is minopts coreclr-tests; the other collections are all faster, so possibly a scaling issue with some of the crazier tests. We may gain some of this back in the next stage of work. |
Maybe related to #78267? |
Sure.
Yeah, probably cases like that one. Add/Remove ref can also be quadratic in worst case since they have to search a list, but it only matters when there are huge numbers of predecessors. |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Looks like some widespread jitstress issue. Will investigate. |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Stress is green. |
ghost
locked as resolved and limited conversation to collaborators
Move pred list building before importation. It now runs as the first phase in the phase list.
impImportLeave.impFixPredListsfor the importer to use at the end of importation. This adds pred list entries finally returns, which can't be done until allBBJ_LEAVEblocks are processed.Contributes to #80193.