Skip to content
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

JIT: fix exit edge likelihood computation for capped loops #84817

Merged
merged 2 commits into from
Apr 14, 2023

Conversation

AndyAyersMS
Copy link
Member

@AndyAyersMS AndyAyersMS commented Apr 14, 2023

The initial version of this only really worked for very simple cases. Generalize to handle loops with multiple exits better.

The rough idea is to compute how much additional profile flow needs to come out of the loop to match the capped probability, and then try and find one exit block where we can adjust likelhoods to cause exactly this amount of extra flow to leave the loop.

Closes #84798.

The initial version of this only really worked for very simple cases.
Generalize to handle loops with multiple exits better.

The rough idea is to compute how much additional profile flow needs
to come out of the loop to match the capped probability, and then
try and find one exit block where we can adjust likelhoods to cause
exactly this amount of extra flow to leave the loop.

Closes dotnet#84789.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 14, 2023
@ghost ghost assigned AndyAyersMS Apr 14, 2023
@ghost
Copy link

ghost commented Apr 14, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak
See info in area-owners.md if you want to be subscribed.

Issue Details

The initial version of this only really worked for very simple cases. Generalize to handle loops with multiple exits better.

The rough idea is to compute how much additional profile flow needs to come out of the loop to match the capped probability, and then try and find one exit block where we can adjust likelhoods to cause exactly this amount of extra flow to leave the loop.

Closes #84789.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

@EgorBo PTAL
cc @dotnet/jit-contrib

File this one under "I really should have known better" -- I recall thinking about this problem long ago and deciding it wasn't simple to fix, so my simplistic initial fix in #84741 should have set off some personal alarm bells.

No SPMI diffs. Have verified some of the pgo pipeline cases in #84798 are fixed, will run them here to check the rest.

@AndyAyersMS AndyAyersMS requested a review from EgorBo April 14, 2023 02:12
@AndyAyersMS
Copy link
Member Author

/azp run runtime-coreclr pgo, runtime-coreclr libraries-pgo

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@EgorBo
Copy link
Member

EgorBo commented Apr 14, 2023

Failures are related? e.g.

Assertion failed 'cappedExitWeight < 1.0' in 'System.Xml.XmlUtf8RawTextWriter:WriteCommentOrPi(System.String,int):this' during 'Profile incorporation'

@AndyAyersMS
Copy link
Member Author

Failures are related? e.g.

Assertion failed 'cappedExitWeight < 1.0' in 'System.Xml.XmlUtf8RawTextWriter:WriteCommentOrPi(System.String,int):this' during 'Profile incorporation'

Yes, this is a newly added assert.

@AndyAyersMS
Copy link
Member Author

/azp run runtime-coreclr pgo, runtime-coreclr libraries-pgo

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@EgorBo EgorBo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming CI passes

@AndyAyersMS
Copy link
Member Author

Neither PGO pipeline is clean, but so far the failures are known

@AndyAyersMS AndyAyersMS merged commit 3182dfb into dotnet:main Apr 14, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion failed 'likelihood >= 0.0' during 'Profile incorporation'
2 participants