JIT: enable inlining of IL stubs with EH #118140
Conversation
Also, bail out of inlining analysis early if the pinvoke call was marked as unmanaged during initial pinvoke analysis. Closes dotnet#113742
github-actions
bot
added
the
area-CodeGen-coreclr
There was a problem hiding this comment.
Pull Request Overview
This PR enables the JIT to inline P/Invoke IL stubs that contain exception handling (EH) by removing a previous restriction. The change also adds early bailout logic to avoid unnecessary inlining analysis for calls already marked as unmanaged during P/Invoke analysis.
Key changes:
- Adds new inline observation type for unmanaged code detection
- Introduces early bailout for unmanaged calls in inlining analysis
- Removes restriction preventing inlining of P/Invoke stubs with exception handling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/jit/inline.def | Adds IS_UNMANAGED observation type for fatal inline rejection |
| src/coreclr/jit/importercalls.cpp | Adds early unmanaged call detection and removes P/Invoke EH inlining restriction |
|
|
||
| if (methAttr & CORINFO_FLG_PINVOKE) | ||
| { | ||
| // We should have already ruled out cases where we can directly call the unamanged method. |
There was a problem hiding this comment.
There is a spelling error in the comment: 'unamanged' should be 'unmanaged'.
| // We should have already ruled out cases where we can directly call the unamanged method. | |
| // We should have already ruled out cases where we can directly call the unmanaged method. |
Copilot uses AI. Check for mistakes.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
Still assessing this one, not sure we want to enable the extra inlining. |
|
NAOT size impact is a small net decrease, per MichalStrehovsky/rt-sz#153 |
|
@jkotas any opinion on whether this is worth doing on its own? Based on the above I would say no, there's not much benefit. Seems like we should sort out the schizophrenia around stub inlining in the JIT first, but I'm somewhat reluctant to churn those code paths at this point in the release. |
Yes, I agree. It is fine to postpone it to .NET 11. This is not fixing any regressions. |
|
Opened #118220 to track sorting out the inlining logic. |
2 participants
Also, bail out of inlining analysis early if the pinvoke call was marked as unmanaged during initial pinvoke analysis.
Closes #113742