From 395850648ec4d5f7d2f9e172bf19f861181d1a77 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 5 Aug 2025 11:46:36 -0700 Subject: [PATCH 1/4] [Interpreter] Intrinic handling for Delegate Invoke methods - Add a new opcode - Add a new invocation method. Note that this needs to have the same copy the CallStubHeader logic as calli, as the target is not consistent from use to use --- src/coreclr/interpreter/compiler.cpp | 16 ++++++- src/coreclr/interpreter/intops.def | 1 + src/coreclr/vm/interpexec.cpp | 67 ++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 2363738a3f725f..5818bd6f9f1134 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2707,6 +2707,7 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re { uint32_t token = getU4LittleEndian(m_ip + 1); bool isVirtual = (*m_ip == CEE_CALLVIRT); + bool isDelegateInvoke = false; CORINFO_RESOLVED_TOKEN resolvedCallToken; CORINFO_CALL_INFO callInfo; @@ -2758,6 +2759,11 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re } } + if (callInfo.methodFlags & CORINFO_FLG_DELEGATE_INVOKE) + { + isDelegateInvoke = true; + } + if (callInfo.thisTransform != CORINFO_NO_THIS_TRANSFORM) { assert(pConstrainedToken != NULL); @@ -3031,7 +3037,15 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re // before the call. // TODO: Add null checking behavior somewhere here! } - AddIns((isPInvoke && !isMarshaledPInvoke) ? INTOP_CALL_PINVOKE : INTOP_CALL); + if (isDelegateInvoke) + { + assert(!isPInvoke && !isMarshaledPInvoke); + AddIns(INTOP_CALLDELEGATE); + } + else + { + AddIns((isPInvoke && !isMarshaledPInvoke) ? INTOP_CALL_PINVOKE : INTOP_CALL); + } m_pLastNewIns->data[0] = GetMethodDataItemIndex(callInfo.hMethod); if (isPInvoke && !isMarshaledPInvoke) { diff --git a/src/coreclr/interpreter/intops.def b/src/coreclr/interpreter/intops.def index 0e1d73769a44fa..90083caa3cc7b8 100644 --- a/src/coreclr/interpreter/intops.def +++ b/src/coreclr/interpreter/intops.def @@ -356,6 +356,7 @@ OPDEF(INTOP_LDFLDA, "ldflda", 4, 1, 1, InterpOpInt) // Calls OPDEF(INTOP_CALL, "call", 4, 1, 1, InterpOpMethodHandle) +OPDEF(INTOP_CALLDELEGATE, "call.delegate", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALLI, "calli", 5, 1, 2, InterpOpLdPtr) OPDEF(INTOP_CALLVIRT, "callvirt", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALL_PINVOKE, "call.pinvoke", 6, 1, 1, InterpOpMethodHandle) // inlined (no marshaling wrapper) pinvokes only diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 2fb4f73e4f2a69..aa1524c3ce1a32 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -56,6 +56,49 @@ void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE ta pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); } +void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer(pMDDelegateInvoke)); + PRECONDITION(CheckPointer(pArgs)); + PRECONDITION(CheckPointer(pRet)); + } + CONTRACTL_END + + CallStubHeader *stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); + if (stubHeaderTemplate == NULL) + { + CallStubGenerator callStubGenerator; + GCX_PREEMP(); + + AllocMemTracker amTracker; + stubHeaderTemplate = callStubGenerator.GenerateCallStub(pMDDelegateInvoke, &amTracker, true /* interpreterToNative */); + + if (pMDDelegateInvoke->SetCallStub(stubHeaderTemplate)) + { + amTracker.SuppressRelease(); + } + else + { + // We have lost the race for generating the header, use the one that was generated by another thread + // and let the amTracker release the memory of the one we generated. + stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); + } + } + + // CallStubHeaders encode their destination addresses in the Routines array, so they need to be + // copied to a local buffer before we can actually set their target address. + size_t templateSize = stubHeaderTemplate->GetSize(); + uint8_t* actualCallStub = (uint8_t*)alloca(templateSize); + memcpy(actualCallStub, stubHeaderTemplate, templateSize); + CallStubHeader *pHeader = (CallStubHeader*)actualCallStub; + pHeader->SetTarget(target); // The method to call + pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); +} + void InvokeCalliStub(PCODE ftn, CallStubHeader *stubHeaderTemplate, int8_t *pArgs, int8_t *pRet) { CONTRACTL @@ -1881,6 +1924,30 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr break; } + case INTOP_CALLDELEGATE: + { + returnOffset = ip[1]; + callArgsOffset = ip[2]; + methodSlot = ip[3]; + + // targetMethod holds a pointer to the Invoke method of the delegate, not the final actual target. + targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; + + ip += 4; + + DELEGATEREF delegateObj = LOCAL_VAR(callArgsOffset, DELEGATEREF); + NULL_CHECK(delegateObj); + PCODE targetAddress = delegateObj->GetMethodPtr(); + OBJECTREF targetMethodObj = delegateObj->GetTarget(); + LOCAL_VAR(callArgsOffset, OBJECTREF) = targetMethodObj; + + // TODO! Once we are investigatin performance here, we may way to optimize this so that + // delegate calls to interpeted methods don't have to go through the native invoke here, but for + // now this should work well. + InvokeDelegateInvokeMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, targetAddress); + break; + } + case INTOP_CALL: { returnOffset = ip[1]; From 0b110cffc1f4dea1408a0aa7c37c253274b1a7f7 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 5 Aug 2025 11:47:37 -0700 Subject: [PATCH 2/4] Update src/coreclr/vm/interpexec.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/vm/interpexec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index aa1524c3ce1a32..644c593c61b124 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -1941,7 +1941,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr OBJECTREF targetMethodObj = delegateObj->GetTarget(); LOCAL_VAR(callArgsOffset, OBJECTREF) = targetMethodObj; - // TODO! Once we are investigatin performance here, we may way to optimize this so that + // TODO! Once we are investigating performance here, we may want to optimize this so that // delegate calls to interpeted methods don't have to go through the native invoke here, but for // now this should work well. InvokeDelegateInvokeMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, targetAddress); From 232dee5bdc20278b5cbf8c708b6b3f0e0432cf13 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 5 Aug 2025 13:25:51 -0700 Subject: [PATCH 3/4] Refactor slow path out of function, and make wasm build --- src/coreclr/vm/interpexec.cpp | 66 ++++++++++++++++----------------- src/coreclr/vm/wasm/helpers.cpp | 5 +++ 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index aa1524c3ce1a32..1770b7e5221065 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -14,9 +14,40 @@ #ifdef TARGET_WASM void InvokeCalliStub(PCODE ftn, CallStubHeader *stubHeaderTemplate, int8_t *pArgs, int8_t *pRet); void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target); +void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target); #else #include "callstubgenerator.h" +CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD) +{ + CONTRACTL + { + THROWS; + MODE_ANY; + PRECONDITION(CheckPointer(pMD)); + } + CONTRACTL_END + + CallStubGenerator callStubGenerator; + GCX_PREEMP(); + + AllocMemTracker amTracker; + CallStubHeader *header = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */); + + if (pMD->SetCallStub(header)) + { + amTracker.SuppressRelease(); + } + else + { + // We have lost the race for generating the header, use the one that was generated by another thread + // and let the amTracker release the memory of the one we generated. + header = pMD->GetCallStub(); + } + + return header; +} + void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target) { CONTRACTL @@ -32,22 +63,7 @@ void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE ta CallStubHeader *pHeader = pMD->GetCallStub(); if (pHeader == NULL) { - CallStubGenerator callStubGenerator; - GCX_PREEMP(); - - AllocMemTracker amTracker; - pHeader = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */); - - if (pMD->SetCallStub(pHeader)) - { - amTracker.SuppressRelease(); - } - else - { - // We have lost the race for generating the header, use the one that was generated by another thread - // and let the amTracker release the memory of the one we generated. - pHeader = pMD->GetCallStub(); - } + pHeader = UpdateCallStubForMethod(pMD); } // Interpreter-FIXME: Potential race condition if a single CallStubHeader is reused for multiple targets. @@ -71,22 +87,7 @@ void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, in CallStubHeader *stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); if (stubHeaderTemplate == NULL) { - CallStubGenerator callStubGenerator; - GCX_PREEMP(); - - AllocMemTracker amTracker; - stubHeaderTemplate = callStubGenerator.GenerateCallStub(pMDDelegateInvoke, &amTracker, true /* interpreterToNative */); - - if (pMDDelegateInvoke->SetCallStub(stubHeaderTemplate)) - { - amTracker.SuppressRelease(); - } - else - { - // We have lost the race for generating the header, use the one that was generated by another thread - // and let the amTracker release the memory of the one we generated. - stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); - } + stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke); } // CallStubHeaders encode their destination addresses in the Routines array, so they need to be @@ -127,7 +128,6 @@ CallStubHeader *CreateNativeToInterpreterCallStub(InterpMethod* pInterpMethod) CallStubHeader *pHeader = VolatileLoadWithoutBarrier(&pInterpMethod->pCallStub); GCX_PREEMP(); - AllocMemTracker amTracker; if (pHeader == NULL) { // Ensure that there is an interpreter thread context instance and thus an interpreter stack diff --git a/src/coreclr/vm/wasm/helpers.cpp b/src/coreclr/vm/wasm/helpers.cpp index 7f1fde32c918fc..ac2298f34837a1 100644 --- a/src/coreclr/vm/wasm/helpers.cpp +++ b/src/coreclr/vm/wasm/helpers.cpp @@ -487,3 +487,8 @@ void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE ta { PORTABILITY_ASSERT("Attempted to execute non-interpreter code from interpreter on wasm, this is not yet implemented"); } + +void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, int8_t *pRet, PCODE target) +{ + PORTABILITY_ASSERT("Attempted to execute non-interpreter code from interpreter on wasm, this is not yet implemented"); +} \ No newline at end of file From e04e9788e34d3304cf402a8c93fc2567ebc480cf Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 5 Aug 2025 15:12:57 -0700 Subject: [PATCH 4/4] Update src/coreclr/vm/interpexec.cpp Co-authored-by: Jan Kotas --- src/coreclr/vm/interpexec.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 3f707e3b617160..3a40f605e08c8f 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -28,9 +28,10 @@ CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD) } CONTRACTL_END - CallStubGenerator callStubGenerator; GCX_PREEMP(); + CallStubGenerator callStubGenerator; + AllocMemTracker amTracker; CallStubHeader *header = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */);