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: start checking sum of successor likelihoods #99024

Merged
merged 6 commits into from
Feb 29, 2024

Conversation

AndyAyersMS
Copy link
Member

@AndyAyersMS AndyAyersMS commented Feb 27, 2024

Add the ability to check that successor edge likelihoods sum to 1.0 Verify that successor edge likelihoods sum to 1.0 for the phases up through and including fgAddInternal.

Fixed a few problems

  • importer branch folding needs to update likelihoods
  • gdv likelihood needed some adjustments, especially for the multi-guess case
  • profile incorporation for OSR methods also needed some fixing. Here we may have inconsistencies so the right answer is not always clear. With this change we now trust the successor edge counts even if they do not sum to the block count.

Contributes to #93020.

Diffs

Add the ability to check that uccessor edge likelihoods sum to 1.0
Verify that successor edge likelihoods sum to 1.0 for the phases up through
and including `fgAddInternal`.

Fixed a few problems
* importer branch folding needs to update likelihoods
* gdv likelihood needed some adjustments, especially for the multi-guess case
* profile incorporation for OSR methods also needed some fixing. Here
we may have inconsistencies so the right answer is not always clear.
With this change we now trust the successor edge counts even if they
do not sum to the block count.

Contributes to dotnet#93020.
@ghost ghost assigned AndyAyersMS Feb 27, 2024
@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 Feb 27, 2024
@ghost
Copy link

ghost commented Feb 27, 2024

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

Issue Details

Add the ability to check that uccessor edge likelihoods sum to 1.0 Verify that successor edge likelihoods sum to 1.0 for the phases up through and including fgAddInternal.

Fixed a few problems

  • importer branch folding needs to update likelihoods
  • gdv likelihood needed some adjustments, especially for the multi-guess case
  • profile incorporation for OSR methods also needed some fixing. Here we may have inconsistencies so the right answer is not always clear. With this change we now trust the successor edge counts even if they do not sum to the block count.

Contributes to #93020.

Author: AndyAyersMS
Assignees: AndyAyersMS
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

@amanasifkhalid PTAL
cc @dotnet/jit-contrib

A few small diffs expected.

Copy link
Member

@amanasifkhalid amanasifkhalid left a comment

Choose a reason for hiding this comment

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

LGTM, I'll take another look once you've merged from main. Thanks!

src/coreclr/jit/fginline.cpp Show resolved Hide resolved
src/coreclr/jit/indirectcalltransformer.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/indirectcalltransformer.cpp Outdated Show resolved Hide resolved
Copy link
Member

@amanasifkhalid amanasifkhalid left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@AndyAyersMS
Copy link
Member Author

One runtime failure is #99074, another is (perhaps) #98817
Replays are hitting some sort of infra issue trying to download results from helix?

  Waiting for completion of job 0a0cdc60-4209-4427-8d83-4668d827952c on Windows.10.Amd64.Open (Details: https://helix.dot.net/api/jobs/0a0cdc60-4209-4427-8d83-4668d827952c/details?api-version=2019-06-17 )
  Job 0a0cdc60-4209-4427-8d83-4668d827952c on Windows.10.Amd64.Open is completed with 7 finished work items.
  Downloading result files for job 0a0cdc60-4209-4427-8d83-4668d827952c
D:\a\_work\1\s\.packages\microsoft.dotnet.helix.sdk\9.0.0-beta.24112.1\tools\download-results\DownloadFromResultsContainer.targets(26,5): 
   error : RestApiException`1: The response contained an invalid status code 500 Internal Server Error
   [D:\a\_work\1\s\src\coreclr\scripts\superpmi-replay.proj]

``

@AndyAyersMS AndyAyersMS merged commit 54b4a60 into dotnet:main Feb 29, 2024
124 of 129 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2024
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.

2 participants