Add support for inline pinvokes to Wasm Ryujit#130384
Draft
davidwrighton wants to merge 1 commit into
Draft
Conversation
- They should always be READYTORUN_FIXUP_PInvokeTarget instead of READYTORUN_FIXUP_IndirectPInvokeTarget - There is a debug version of JIT_PinvokeEnd which validates that sp == the __stack_pointer global. I believe our codegen will maintain this invariant, so we should be ok to skip resetting the __stack_pointer global before calls to the C++ implementation of JIT_PInvokeEnd - One detail which is no longer true after this work is merged is that the __stack_pointer global will be set to increasingly unpredictable values during execution. This is not a new phenomena, as it was happening during EH flow, but now it will happen in normal execution flow. Since the interpreter to R2R thunks will reset the stack before we return to any emscripten compiled code, we should be ok with this.
Contributor
|
Tagging subscribers to this area: @agocke |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes the Wasm/CoreCLR and ReadyToRun toolchain to enable (or move toward enabling) inlined P/Invoke sequences in Wasm RyuJIT / R2R scenarios, primarily by adjusting how P/Invoke targets are represented in R2R fixups and how the runtime establishes stackwalk state for inlined P/Invokes on Wasm.
Changes:
- Updates Wasm
InlinedCallFrameregister display logic to derive IP/SP/FP from an R2R stack pointer in the “inlined P/Invoke” case. - Introduces Wasm implementations/wrappers for
JIT_PInvokeBegin/JIT_PInvokeEndand adjusts fixup handling so Wasm usesREADYTORUN_FIXUP_PInvokeTarget(not indirect). - Gates
READYTORUN_FIXUP_IndirectPInvokeTargethandling in the runtime behindHAS_PINVOKE_IMPORT_PRECODE.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| src/coreclr/vm/wasm/helpers.cpp | Adds Wasm-specific inlined P/Invoke frame setup and stackwalk register-display logic; introduces Wasm JIT_PInvokeBegin/End implementations/wrappers. |
| src/coreclr/vm/jitinterface.cpp | Wraps READYTORUN_FIXUP_IndirectPInvokeTarget runtime fixup support with HAS_PINVOKE_IMPORT_PRECODE. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs | Forces Wasm ReadyToRun compilations to use direct P/Invoke target fixups (and adds explanatory comments). |
|
|
||
| #define WASM_STRINGIFY_HELPER(value) #value | ||
| #define WASM_STRINGIFY(value) WASM_STRINGIFY_HELPER(value) | ||
| #define INLINED_PINVOKE_FROM_R2R 0 |
|
|
||
| pRD->pCurrentContext->InterpreterSP = *(DWORD *)&m_pCallSiteSP; | ||
| pRD->pCurrentContext->InterpreterFP = *(DWORD *)&m_pCalleeSavedFP; | ||
| if (m_CallerReturnAddress == INLINED_PINVOKE_FROM_R2R) |
Comment on lines
+499
to
+503
| pRD->pCurrentContext->InterpreterSP = m_pCallSiteSP; | ||
| pRD->pCurrentContext->InterpreterIP = GetWasmVirtualIPFromStackPointer(m_pCallSiteSP); | ||
| _ASSERTE(pRD->pCurrentContext->InterpreterIP != 0); // We should be in RyuJit compiled code here | ||
| pRD->pCurrentContext->InterpreterFP = GetWasmFramePointerFromStackPointer(m_pCallSiteSP, pRD->pCurrentContext->InterpreterIP); | ||
| } |
Comment on lines
+708
to
+712
| ::new ((void*)pFrame) InlinedCallFrame(); | ||
| pFrame->m_pCallSiteSP = sp; | ||
| pFrame->m_pCallerReturnAddress = INLINED_PINVOKE_FROM_R2R; // When this is tru the UpdateRegisters function will do all work based on m_pCallerReturnAddress | ||
| pFrame->m_pCalleeSavedFP = 0; | ||
| pFrame->m_pThread = pThread; |
Comment on lines
+734
to
+742
| EXTERN_C void* JIT_PInvokeEndImpl(TADDR sp, TADDR stack_pointer_global_value, InlinedCallFrame* pFrame) | ||
| { | ||
| _ASSERTE(sp == stack_pointer_global_value); | ||
| Thread* pThread = (Thread*)pFrame->m_pThread; | ||
|
|
||
| // Transition back to cooperative GC mode and unlink the frame. | ||
| pThread->DisablePreemptiveGC(); | ||
| pFrame->Pop(); | ||
| } |
Comment on lines
+746
to
+750
| asm("local.get 0\n" /* sp */ | ||
| "global.set __stack_pointer\n" /* __stack_pointer = sp before any native code runs */ | ||
| "local.get 1\n" /* pFrame */ | ||
| "call %0\n" | ||
| "return" ::"i"(JIT_PInvokeEndImpl)); |
Comment on lines
+753
to
+755
| extern "C" void JIT_PInvokeEnd(void* sp, InlinedCallFrame* pFrame, PCODE pep) | ||
| { | ||
| Thread* pThread = (Thread*)pFrame->m_pThread; |
Comment on lines
+3226
to
+3228
| // Suppress GC transition pinvokes are called directly, since we can't do a gc transition at this point. | ||
| // On Wasm, we also call directly, as the runtime doesn't generate stubs for the pinvoke calls which can produce errors. Instead | ||
| // the error is produced as we fixup the method in the first place. |
Comment on lines
+722
to
723
| extern "C" __attribute__((naked)) void JIT_PInvokeBegin(void* sp, InlinedCallFrame* pFrame, PCODE pep) | ||
| { |
Open
3 tasks
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.
DRAFT: since this is mostly untested at the moment.