JIT: Attribute GDV inline failures to candidate - #133993
Open
AndyAyersMS wants to merge 1 commit into
Open
AndyAyersMS wants to merge 1 commit into
AndyAyersMS wants to merge 1 commit into
Conversation
GDV inline checks reported failures against the original virtual method, permanently marking the wrong method as not inlineable. Record the guarded candidate as the callee and add regression coverage. Fixes dotnet#133974 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cc9ff2b1-3240-4db8-85b3-8e42b2a0f2ec
github-actions
Bot
added
the
area-CodeGen-coreclr
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Member
Author
|
@EgorBo PTAL Can't really assess this via SPMI or PMI. It's probably not going to change much in practice, so I don't plan to do any bespoke SPMI. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Centralize or pass through the guarded-target selection to avoid duplicated logic and future misattribution.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes GDV inline-failure attribution so permanent no-inline marking targets the guarded method.
Changes:
- Adds explicit callee support to
InlineResult. - Uses the GDV candidate for reporting and propagation.
- Adds tiered-PGO regression coverage.
File summaries
| File | Summary |
|---|---|
src/tests/JIT/opt/GuardedDevirtualization/badinlinee.csproj |
Configures isolated tiered-PGO testing. |
src/tests/JIT/opt/GuardedDevirtualization/badinlinee.cs |
Adds allocation-elimination regression coverage. |
src/coreclr/jit/inline.h |
Extends InlineResult with an explicit callee. |
src/coreclr/jit/inline.cpp |
Uses the supplied callee for reporting. |
src/coreclr/jit/importercalls.cpp |
Passes the GDV target during candidate evaluation. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+9228
to
+9232
| CORINFO_METHOD_HANDLE callee = gdvCandidate->guardedMethodUnboxedResolvedToken.hMethod; | ||
| if (callee == nullptr) | ||
| { | ||
| callee = gdvCandidate->guardedMethodHandle; | ||
| } |
This was referenced Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GDV inline checks evaluated the guarded target but reported failures against
the original virtual method. For permanent failures this could mark the wrong
method as not inlineable for the lifetime of the process.
Pass the actual GDV candidate to
InlineResultso reporting andCORINFO_FLG_BAD_INLINEEpropagation use the guarded target. Add a regressiontest that tiers the poisoning call before checking allocation elimination in
an unrelated exact call.
Fixes #133974
Validation:
Testcodegen improved from 34 bytes to 6 bytesNote
This pull request description was generated with GitHub Copilot.