JIT: replace R2R EH-inlining workaround for PInvoke stubs#128790
Draft
AndyAyersMS wants to merge 1 commit into
Draft
JIT: replace R2R EH-inlining workaround for PInvoke stubs#128790AndyAyersMS wants to merge 1 commit into
AndyAyersMS wants to merge 1 commit into
Conversation
PR dotnet#112998 added a workaround blocking inline of PInvoke callees with EH, standing in for a missing crossgen2 version-resilience check. Jan fixed that in PR dotnet#115277, so the workaround is no longer needed. Replace it with: * Early bail in impMarkInlineCandidateHelper when call->IsUnmanaged(). Rename CALLEE_HAS_UNMANAGED_CALLCONV to CALLEE_IS_UNMANAGED. * New GTF_CALL_M_PINVOKE_STUB_NO_INLINE, set by impCheckForPInvokeCall at the two cost-based bails (rarely-run, !impCanPInvokeInline) and honored by the inliner. Closes the cold-pinvoke case where the inliner would still inline the (larger) stub IL. No SPMI asmdiffs. R2R version-resilience tests pass. Fixes dotnet#113742. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
There was a problem hiding this comment.
Pull request overview
This PR narrows the JIT’s P/Invoke inlining behavior after the crossgen2 version-resilience fix, removing the broader EH-based workaround and adding a targeted callsite marker to prevent inlining P/Invoke IL stubs when P/Invoke inlining is disabled or not profitable.
Changes:
- Renames the unmanaged-callee inline observation to reflect unmanaged code rather than call convention.
- Adds a
GTF_CALL_M_PINVOKE_STUB_NO_INLINEcall flag and accessor. - Marks P/Invoke calls that should avoid IL stub inlining for ambient-disablement and cold-callsite cases, then honors that flag in inline candidate selection.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/coreclr/jit/inline.def |
Updates inline observation names/messages for unmanaged calls and P/Invoke stub no-inline decisions. |
src/coreclr/jit/gentree.h |
Adds the new P/Invoke stub no-inline call flag and query helper. |
src/coreclr/jit/importercalls.cpp |
Sets the new flag during P/Invoke import and uses it to reject affected inline candidates while removing the old EH workaround. |
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.
PR #112998 added a workaround blocking inline of PInvoke callees with EH, standing in for a missing crossgen2 version-resilience check. Jan fixed that in PR #115277, so the workaround is no longer needed.
Replace it with:
No SPMI asmdiffs. R2R version-resilience tests pass.
Fixes #113742.