Skip to content

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

Merged
kouvel merged 1 commit into
dotnet:masterfrom
kouvel:NoQuickJitFix
Mar 13, 2020
Merged

When QuickJit is disabled, fix assertion failures regarding R2R code#33512
kouvel merged 1 commit into
dotnet:masterfrom
kouvel:NoQuickJitFix

Conversation

@kouvel

@kouvelkouvel commented Mar 12, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor
  • 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.

@kouvelkouvel added this to the 5.0 milestone Mar 12, 2020
@kouvel
kouvel requested review from davmason and noahfalkMarch 12, 2020 13:05
@kouvelkouvel self-assigned this Mar 12, 2020
@kouvelkouvel mentioned this pull request Mar 12, 2020
Merged
@kouvelkouvel changed the title When QuickJit is disabled, fix assertion failuresWhen QuickJit is disabled, fix assertion failures with R2R codeMar 12, 2020
@kouvelkouvel changed the title When QuickJit is disabled, fix assertion failures with R2R codeWhen QuickJit is disabled, fix assertion failures regarding R2R codeMar 12, 2020
Koundinya Veluri
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 #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

@noahfalknoahfalk left a comment

Copy link
Copy Markdown
Member

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:masterMar 13, 2020
@kouvel
kouvel deleted the NoQuickJitFix branch March 13, 2020 20:14
@ghostghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@noahfalknoahfalknoahfalk approved these changes
@davmasondavmasonAwaiting requested review from davmason

Assignees

@kouvelkouvel

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