JIT: improve profile update for loop inversion #85265
Conversation
If the loop test block has multiple predecessors we will not do proper profile updates. This can lead to downstream problems with block layout (say leaving a cold block in a loop). Fix by changing how we compute the amount of profile that should remain in the test block. Fixes dotnet#84319.
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsIf the loop test block has multiple predecessors we will not do proper profile updates. This can lead to downstream problems with block layout (say leaving a cold block in a loop). Fix by changing how we compute the amount of profile that should remain in the test block. Fixes #84319.
|
|
@BruceForstall PTAL I've had these changes sitting around for awhile but didn't have a case where it mattered. Now I do: #84264 (comment) Small number of diffs in SPMI PGO runs (though oddly not in the benchmark above -- likely because of how we do that collection). |
| weightNext); | ||
| bTest->inheritWeight(block->bbNext); | ||
| weightTop); | ||
| bTest->inheritWeight(bTop); |
There was a problem hiding this comment.
This is the actual fix -- the rest is just renaming and protecting against inconsistent profile data.
|
please don't try to clear the |
|
investigation is done |
|
Looking like this may have had mixed impact on benchmarks (just have arm64 autofiled so far) Improvements:
Regressions: |
ghost
locked as resolved and limited conversation to collaborators
If the loop test block has multiple predecessors we will not do proper profile updates. This can lead to downstream problems with block layout (say leaving a cold block in a loop).
Fix by changing how we compute the amount of profile that should remain in the test block.
Fixes #84319.
Diffs