Enable R2R precompilation of blittable ObjC P/Invokes#124770
Enable R2R precompilation of blittable ObjC P/Invokes#124770davidnguyen-tech wants to merge 14 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Enables ReadyToRun (R2R) precompilation of Objective-C objc_msgSend-family P/Invoke stubs by emitting a managed pending-exception check after the native call, avoiding a runtime-stub/interpreter fallback on JIT-less Apple platforms.
Changes:
- Add
ObjectiveCMarshal.ThrowPendingExceptionObject()in CoreCLR and expose it to the runtime binder. - Update the R2R
PInvokeILEmitterto emit a post-call pending-exception check instead of rejecting ObjC P/Invokes. - Adjust R2R compilation policy and IL caching behavior to allow ObjC P/Invokes to be attempted and to gracefully fall back when unsupported.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/corelib.h | Adds CoreLib binder entry for ObjectiveCMarshal.ThrowPendingExceptionObject. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs | Simplifies P/Invoke stub IL retrieval logic now that unsupported cases are handled/cached in ILCache. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/Stubs/PInvokeILEmitter.cs | Emits ThrowPendingExceptionObject after ObjC native calls; removes ObjC-specific NotSupportedException. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs | Allows GeneratesPInvoke for ObjC msgSend P/Invokes even when marshalling is otherwise considered required. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs | Caches “requires runtime JIT” decisions for unsupported P/Invoke stub emission via RequiresRuntimeJitException catch. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.CoreCLR.cs | Implements ThrowPendingExceptionObject() using pending-exception storage and ExceptionDispatchInfo.Throw. |
|
Tagging subscribers to 'os-ios': @vitek-karas, @kotlarmilos, @steveisok, @akoeplinger |
|
Were there also any interpreted pinvokes with non-blittable types ? In case we might need additional fixes, related to the original plan of using |
|
Do we need to re-enable or create a new sets of tests for this? We have tests for these APIs, /cc @jkoritzinsky |
The tests there are falling back to JIT/Interpreter for these stubs (which is correct behavior). We could add testing to validate that the methods were actually generated ( |
Analyzes reviewer feedback from BrzVlad, AaronRobinsonMSFT, and jkoritzinsky. Includes recommended code changes and test coverage plan. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ning findings - Add LibraryImport vs DllImport analysis explaining why switching macios wouldn't solve the R2R precompilation issue - Add P/Invoke inlining call chain trace showing CorInfoImpl.ReadyToRun.cs path (not VM dllimport.cpp) for R2R marshalling decisions - Document graceful fallback behavior for unsupported marshalling types - Confirm BrzVlad's approach is both cleaner and more efficient Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
You can also share your feedback on Copilot code review. Take the survey.
- Document new 3-commit structure and PInvokeILStubMethodIL mechanism - Mark BrzVlad's recommendations as implemented (GeneratesPInvoke reverted, ShouldCheckForPendingException removed from IsMarshallingRequired) - Add open concerns: code size, P/Invoke frame correctness, non-void IL stack risk, JIT helper alternative under investigation - Test coverage still flagged as missing (merge blocker) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a ReadyToRun test that validates objc_msgSend P/Invoke stubs are precompiled into the R2R image using crossgen2's --map output. The test: - Declares blittable objc_msgSend DllImport methods - Compiles them with crossgen2 --map - At runtime, reads the .map file and asserts P/Invoke stub entries exist - Only runs on macOS (CLRTestTargetUnsupported for non-Apple platforms) Addresses reviewer feedback from jkoritzinsky and AaronRobinsonMSFT requesting test coverage for the R2R ObjC P/Invoke precompilation feature. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validates that blittable objc_msgSend P/Invoke stubs are precompiled into the ReadyToRun image by checking the crossgen2 --map output for MethodWithGCInfo entries. Key details: - Uses /usr/lib/libobjc.dylib (exact path matched by ShouldCheckForPendingException) - Checks specifically for MethodWithGCInfo entries (not just any map reference) - Requires --inputbubble so crossgen2 can resolve ThrowPendingExceptionObject tokens - macOS-only (CLRTestTargetUnsupported for non-OSX) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Promote regression test verification to a mandatory section in copilot-instructions.md with explicit guidance on using baseline build artifacts for negative verification - Add ReadyToRun/crossgen2 test gotchas section covering --inputbubble, map entry semantics, and MarshalHelpers.cs string matching - Add cross-component build warning (crossgen2 + CoreLib changes) - Add design doc discoverability instruction - Strengthen CoreLib rebuild guidance to explain libs.pretest necessity - Add Step 5 (mandatory negative verification) to jit-regression-test skill - Create src/tests/readytorun/README.md documenting test patterns, map validation rules, version bubble constraints, and platform gating Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
You can also share your feedback on Copilot code review. Take the survey.
Add P/Invoke marshalling and R2R-specific review rules: - inputbubble requirement for cross-module CoreLib references - IsMarshallingRequired prevents JIT from inlining past R2R stubs - Separation of marshalling vs platform-specific stub requirements - DllImport detection string matching against MarshalHelpers.cs - R2R map validation (MethodWithGCInfo vs MethodFixupSignature) - R2R+NativeAOT P/Invoke emitter alignment checks - R2R tests need both map validation and runtime verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
- Spill native return value across the ThrowPendingExceptionObject helper
call so the IL evaluation stack is empty at the call site.
- Add runtime pending-exception validation to the ObjCPInvokeR2R test
via SetMessageSendCallback + SetMessageSendPendingException.
- Add $(TestLibraryProjectPath) ProjectReference to ObjCPInvokeR2R.csproj
to match other R2R test projects.
- Remove pr-124770-{plan,analysis}.md scratch notes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mirrors the existing AddNecessaryAsyncReferences pattern: when an ObjC P/Invoke is selected for R2R compilation, the new emitter calls ObjectiveCMarshal.ThrowPendingExceptionObject() in CoreLib. In non-composite mode (the default on iOS), CoreLib is an external reference, so without manifest pre-registration the version-bubble check in ManifestMetadataTableNode rejects the helper and crossgen2 fails. Uses GetType(throwIfNotFound: false) so non-Apple workloads (no ObjC interop) skip the lookup harmlessly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Enables ReadyToRun precompilation of blittable
objc_msgSend-family P/Invokes on Apple platforms. The R2R-emitted stub now calls a managedObjectiveCMarshal.ThrowPendingExceptionObject()helper right after the native call, so ObjC P/Invokes no longer have to fall back to JIT/interpreter for the pending-exception check.Changes
ObjectiveCMarshal.ThrowPendingExceptionObject()in CoreLib and exposes it to the runtime binder viacorelib.h.PInvokeILEmitteremits a call to that helper for ObjC P/Invokes, spilling the native return value across the call so the IL evaluation stack is empty at the helper site.Marshaller.ReadyToRunno longer rejects ObjC P/Invokes, andPInvokeILStubMethodIL.IsMarshallingRequiredis the single place that gates the ObjC pending-exception check.ReadyToRunCodegenCompilationpre-registersObjectiveCMarshal/ThrowPendingExceptionObjectreferences with the manifest before fixup emission, mirroring the existingAddNecessaryAsyncReferencespattern. Required in non-composite mode (the default on iOS), where CoreLib is an external reference and would otherwise get rejected as out-of-bubble.src/tests/readytorun/ObjCPInvokeR2R/asserts both that the stubs land in the crossgen2.map(MethodWithGCInfo) and that the pending-exception path actually executes by hijackingMessageSendFunction.MsgSendwith a managed callback.Impact
Synthetic ObjC P/Invoke benchmark on osx-arm64 Release: a console host invokes 100 distinct blittable
objc_msgSendP/Invokes per iteration.MAUI template app on device doesn't provide measurable improvements because ObjC P/Invokes on the MAUI startup path are not the dominant cost.