-
Notifications
You must be signed in to change notification settings - Fork 5k
[crossgen2] Prevent inlining of PInvoke marshalling accross version bubbles #115277
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.
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
Conversation
…ubles Contributes to dotnet#113742
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR aims to prevent inlining of PInvoke marshalling across the version bubble. The changes remove a version bubble check from the JIT interface implementation and add a corresponding check in the compilation module group to enforce the desired behavior.
- Removed version bubble check in pInvokeMarshalingRequired in CorInfoImpl.ReadyToRun.cs.
- Added a version bubble check in GeneratesPInvoke in ReadyToRunCompilationModuleGroupBase.cs.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs | Removed legacy check for version bubble compliance. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs | Added explicit version bubble check to prevent PInvoke inlining. |
Comments suppressed due to low confidence (1)
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs:3079
- The removal of this version bubble check in pInvokeMarshalingRequired should be clearly coordinated with the new logic in GeneratesPInvoke to ensure consistent behavior. Please verify that the change does not unintentionally affect related code paths.
// Marshalling behavior isn't modeled as protected by R2R rules, so disable pinvoke inlining for code outside of the version bubble
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs
Show resolved
Hide resolved
cc @AndyAyersMS This is fixing the crossgen2 logic bug. I am not deleting the JIT workaround in this change. I think we want to evaluate code quality and JIT throughput impact of deleting the workaround separately. |
/ba-g networking failure, infrastructure timeout |
Inlining of PInvoke stubs should be blocked explicitly in crossgen2. It was blocked as side-effect of JIT never inlining methods with EH, but that is not a safe assumption to make now that the JIT is able to inline methods with EH.
Contributes to #113742