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

When QuickJit is disabled, fix assertion failures regarding R2R code #33512

Merged
merged 1 commit into from
Mar 13, 2020
Merged

When QuickJit is disabled, fix assertion failures regarding R2R code #33512

merged 1 commit into from
Mar 13, 2020

Conversation

kouvel
Copy link
Member

@kouvel kouvel commented Mar 12, 2020 •
edited
Loading

  • When QuickJit is disabled, for precompiled R2R code the initial tier is Optimized instead of the correct Tier0. This causes assertion failures as tiering tries to count calls and promote the method to Tier1.
    • Does not appear to be an issue in release builds, as the methods are still call-counted and promoted despite the incorrect tier
  • Add some basic tiering tests for config modes that are exposed and supported through <app>.runtimeconfig.json, QuickJit and QuickJitForLoops, when on and off
  • Removed an invalid and redundant assertion that was causing a profiler rejit test to fail, see Remove asserts that were firing during rejit tests #33492 (comment). What the assertion was intending to verify is already verified by an assertion above it that checks the tier, which also covers the default native code version case.

@kouvel kouvel added this to the 5.0 milestone Mar 12, 2020
@kouvel kouvel self-assigned this Mar 12, 2020
@kouvel kouvel mentioned this pull request Mar 12, 2020
Merged
@kouvel kouvel changed the title When QuickJit is disabled, fix assertion failures When QuickJit is disabled, fix assertion failures with R2R code Mar 12, 2020
@kouvel kouvel changed the title When QuickJit is disabled, fix assertion failures with R2R code When QuickJit is disabled, fix assertion failures regarding R2R code Mar 12, 2020
@kouvel
When QuickJit is disabled, fix assertion failures
72f909e 
- When QuickJit is disabled, the initial tier is Optimized instead of the correct Tier0. This causes assertion failures as tiering tries to count calls and promote the method to Tier1.
  - Does not appear to be an issue in release builds, as the methods are still call-counted and promoted despite the incorrect tier
- Add some basic tiering tests for config modes that are exposed and supported through <app>.runtimeconfig.json, QuickJit and QuickJitForLoops, when on and off
- Removed an invalid and redundant assertion that was causing a profiler rejit test to fail, see dotnet#33492 (comment). What the assertion was intending to verify is already verified by an assertion above it that checks the tier, which also covers the default native code version case.
noahfalk
noahfalk approved these changes Mar 13, 2020
View reviewed changes
Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

LGTM

@kouvel kouvel merged commit 490f09e into dotnet:master Mar 13, 2020
@kouvel kouvel deleted the NoQuickJitFix branch March 13, 2020 20:14
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@noahfalk noahfalk noahfalk approved these changes

@davmason davmason Awaiting requested review from davmason

Assignees

@kouvel kouvel

Labels
area-TieredCompilation-coreclr
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@kouvel @noahfalk