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.

Sign up for GitHub

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

Jump to bottom

JIT: Fix checked/release diff #70144

Merged
merged 1 commit into from
Jun 2, 2022
Merged

JIT: Fix checked/release diff #70144

merged 1 commit into from
Jun 2, 2022

Conversation

jakobbotsch
Copy link
Member

Fix #70143 introduced by #68869

@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 Jun 2, 2022
@ghost ghost assigned jakobbotsch Jun 2, 2022
@ghost
Copy link

ghost commented Jun 2, 2022

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

Issue Details

Fix #70143 introduced by #68869

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr
Milestone: -

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @kunalspathak

What would the effect of this be in release builds?

@kunalspathak
Copy link
Member

Thanks for figuring it out. This would have almost might have made every possible loop's size calculated wrong (it will go all the way to end of the method) and leading to most of the loops being not aligned because of heuristic not met. This would also explain the regression in #69444.

Does this fix completely remove the checked vs. release diffs?

kunalspathak
kunalspathak approved these changes Jun 2, 2022
View reviewed changes
Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM

@jakobbotsch
Copy link
Member Author

Does this fix completely remove the checked vs. release diffs?

It does for win-x64 at least.

@kunalspathak
Copy link
Member

/azp run runtime-coreclr superpmi-asmdiffs-checked-release

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kunalspathak
Copy link
Member

It does for win-x64 at least.

This was the culprit for other platforms as well.

BruceForstall
BruceForstall approved these changes Jun 2, 2022
View reviewed changes
Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

@kunalspathak kunalspathak mentioned this pull request Jun 2, 2022
Closed
@jakobbotsch jakobbotsch merged commit 218a4b1 into dotnet:main Jun 2, 2022
@jakobbotsch jakobbotsch deleted the fix-70143 branch June 2, 2022 19:50
This was referenced Jun 5, 2022
Closed
Closed
Closed
Closed
@JulieLeeMSFT JulieLeeMSFT mentioned this pull request Jun 7, 2022
Closed
This was referenced Jun 7, 2022
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@dakersnar dakersnar mentioned this pull request Jun 10, 2022
Closed
14 tasks
This was referenced Jun 10, 2022
Closed
Closed
@AndyAyersMS
Copy link
Member

Possible regression: dotnet/perf-autofiling-issues#5825

This was referenced Jun 12, 2022
Closed
Closed
Closed
This was referenced Jun 27, 2022
Closed
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jul 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BruceForstall BruceForstall BruceForstall approved these changes

@kunalspathak kunalspathak kunalspathak approved these changes

Assignees

@jakobbotsch jakobbotsch

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

JIT: checked/release diffs
4 participants
@jakobbotsch @kunalspathak @AndyAyersMS @BruceForstall