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: Use edge weights in LinearScan::findPredBlockForLiveIn #108493

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

amanasifkhalid
Copy link
Member

Unrelated to the recent LSRA block order changes, but I noticed that for blocks with multiple predecessors, LinearScan::findPredBlockForLiveIn picks the predecessor with the largest block weight to get the current block's live-in variable register locations from. It would be more precise to factor in the predecessor edges' likelihoods, too -- for example, if one predecessor block is marginally heavier than the other, but is far less likely to flow into their shared successor, the latter seems like a better choice.

Diffs were a mixed bag locally, though now that we always have edge likelihoods available, this seems like something we should be doing on principle.

@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 Oct 2, 2024
Copy link
Contributor

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

@amanasifkhalid
Copy link
Member Author

cc @dotnet/jit-contrib, @kunalspathak PTAL. Diffs differ quite a bit by platform, though this looks like a PerfScore win most of the time. The last superpmi-collect run seems to have dropped quite a few collections, so we're missing some platforms -- if you'd like, I can post diffs for this change against an older GUID on win-x64, or kick off a new collect run?

@kunalspathak
Copy link
Member

This sounds reasonable to me. Do we know why we do not see asmdiffs for win/x64, etc.?

@amanasifkhalid
Copy link
Member Author

Do we know why we do not see asmdiffs for win/x64, etc.?

I'm guessing I kicked this run off too early, before all of the collections had propagated. Newer runs seem back to normal, so I'll kick another one off now.

@AndyAyersMS
Copy link
Member

FWIW I merged a PR that updated the GUID yesterday (#108153) so maybe some of the new collection runs failed? I should have a new asp.net collection soonish.

@amanasifkhalid
Copy link
Member Author

FWIW I merged a PR that updated the GUID yesterday (#108153) so maybe some of the new collection runs failed?

I was thinking that might be the case, but the SPMI diffs run from one of my other PRs with the updated GUID had collections for every platform, and the latest run on this PR is still missing collections...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

3 participants