From 5cbe6637fd7ecba311dc5a3d0b5f85bc89e5ac39 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 28 Oct 2025 01:08:16 +0100 Subject: [PATCH 1/5] Implement CEE_JMP for the interpreter This makes around 20 more coreclr tests pass with the interpreter. --- src/coreclr/interpreter/compiler.cpp | 11 +++++++++++ src/coreclr/interpreter/inc/intops.def | 2 ++ src/coreclr/vm/interpexec.cpp | 14 ++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 45d931af5529aa..ec2facf7da7c70 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -6929,6 +6929,17 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) EmitUnaryArithmeticOp(INTOP_NOT_I4); m_ip++; break; + case CEE_JMP: + { + CHECK_STACK(0); + uint32_t token = getU4LittleEndian(m_ip + 1); + CORINFO_RESOLVED_TOKEN resolvedToken; + ResolveToken(token, CORINFO_TOKENKIND_Method, &resolvedToken); + AddIns(INTOP_JMP); + m_pLastNewIns->data[0] = GetDataItemIndex(resolvedToken.hMethod); + m_ip += 5; + break; + } case CEE_CALLVIRT: case CEE_CALL: EmitCall(pConstrainedToken, readonly, tailcall, false /*newObj*/, false /*isCalli*/); diff --git a/src/coreclr/interpreter/inc/intops.def b/src/coreclr/interpreter/inc/intops.def index 1ba3e9cee29ddf..6de251516595c7 100644 --- a/src/coreclr/interpreter/inc/intops.def +++ b/src/coreclr/interpreter/inc/intops.def @@ -378,6 +378,8 @@ OPDEF(INTOP_CALL_TAIL, "call.tail", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALLI_TAIL, "calli.tail", 6, 1, 2, InterpOpLdPtr) OPDEF(INTOP_CALLVIRT_TAIL, "callvirt.tail", 4, 1, 1, InterpOpMethodHandle) +OPDEF(INTOP_JMP, "jmp", 4, 1, 1, InterpOpMethodHandle) + // The following helper call instructions exist in 2 variants, one for normal methods, and one for cases where a shared generic lookup is needed. // In the case where a shared generic lookup is needed an extra argument is passed as an svar, which is a pointer to the generic context. // If there is a generic context argument it is always the first SVar to the instruction. diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 914f16a995d11d..fb09f61ae2fc97 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -2682,6 +2682,8 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { // If we didn't get the interpreter code pointer setup, then this is a method we need to invoke as a compiled method. // Interpreter-FIXME: Implement tailcall via helpers, see https://github.com/dotnet/runtime/blob/main/docs/design/features/tailcalls-with-helpers.md + // INTOP_JMP handles only managed targets + _ASSERTE(*ip != INTOP_JMP); InvokeManagedMethod(targetMethod, callArgsAddress, returnValueAddress, (PCODE)NULL); break; } @@ -2700,6 +2702,10 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // required to be followed by a ret, so we know nothing is going to read from stack[returnOffset] after the call. pFrame->ReInit(pFrame->pParent, targetIp, pFrame->pRetVal, pFrame->pStack); } + else if (*ip == INTOP_JMP) + { + pFrame->ReInit(pFrame->pParent, targetIp, pFrame->pRetVal, pFrame->pStack); + } else { // Save current execution state for when we return from called method @@ -2730,6 +2736,14 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr pThreadContext->pStackPointer = stack + pMethod->allocaSize; break; } + case INTOP_JMP: + methodSlot = ip[1]; + targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; + // Set the offsets so that the code at CALL_INTERP_METHOD can compute the callArgsAddress and returnValueAddress + // to be the same as the current frame's ones. + callArgsOffset = 0; + returnOffset = (int32_t)(pFrame->pRetVal - stack); + goto CALL_INTERP_METHOD; case INTOP_NEWOBJ_GENERIC: { isTailcall = false; From 218b7b213a069e7d1d6c24a81f02a68464753a0c Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Tue, 28 Oct 2025 02:38:54 +0100 Subject: [PATCH 2/5] Fix the opcode definition and few other details --- src/coreclr/interpreter/compiler.cpp | 5 +++-- src/coreclr/interpreter/inc/intops.def | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index ec2facf7da7c70..8ad9b4784d2cdb 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -975,7 +975,8 @@ void ValidateEmittedSequenceTermination(InterpInst *lastIns) (lastIns->opcode == INTOP_CALLVIRT_TAIL) || (lastIns->opcode == INTOP_RETHROW) || (lastIns->opcode == INTOP_LEAVE_FILTER) || - (lastIns->opcode == INTOP_LEAVE_CATCH)) + (lastIns->opcode == INTOP_LEAVE_CATCH) || + (lastIns->opcode == INTOP_JMP)) { // Valid terminating instruction return; @@ -6936,7 +6937,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) CORINFO_RESOLVED_TOKEN resolvedToken; ResolveToken(token, CORINFO_TOKENKIND_Method, &resolvedToken); AddIns(INTOP_JMP); - m_pLastNewIns->data[0] = GetDataItemIndex(resolvedToken.hMethod); + m_pLastNewIns->data[0] = GetMethodDataItemIndex(resolvedToken.hMethod); m_ip += 5; break; } diff --git a/src/coreclr/interpreter/inc/intops.def b/src/coreclr/interpreter/inc/intops.def index 6de251516595c7..70c692029ee5ab 100644 --- a/src/coreclr/interpreter/inc/intops.def +++ b/src/coreclr/interpreter/inc/intops.def @@ -378,7 +378,7 @@ OPDEF(INTOP_CALL_TAIL, "call.tail", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALLI_TAIL, "calli.tail", 6, 1, 2, InterpOpLdPtr) OPDEF(INTOP_CALLVIRT_TAIL, "callvirt.tail", 4, 1, 1, InterpOpMethodHandle) -OPDEF(INTOP_JMP, "jmp", 4, 1, 1, InterpOpMethodHandle) +OPDEF(INTOP_JMP, "jmp", 4, 0, 0, InterpOpMethodHandle) // The following helper call instructions exist in 2 variants, one for normal methods, and one for cases where a shared generic lookup is needed. // In the case where a shared generic lookup is needed an extra argument is passed as an svar, which is a pointer to the generic context. From a900ae554705e516e5a5769bfac46fd3e0942a35 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 30 Oct 2025 01:53:42 +0100 Subject: [PATCH 3/5] Change the implementation to emulate CEE_JMP via tailcall --- src/coreclr/interpreter/compiler.cpp | 22 +++++++++++++++------- src/coreclr/interpreter/inc/intops.def | 2 -- src/coreclr/vm/interpexec.cpp | 14 -------------- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 8ad9b4784d2cdb..e64933c295e667 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2234,7 +2234,7 @@ void InterpCompiler::CreateBasicBlocks(CORINFO_METHOD_INFO* methodInfo) default: assert(0); } - if (opcode == CEE_THROW || opcode == CEE_ENDFINALLY || opcode == CEE_RETHROW) + if (opcode == CEE_THROW || opcode == CEE_ENDFINALLY || opcode == CEE_RETHROW || opcode == CEE_JMP) GetBB((int32_t)(ip - codeStart)); } } @@ -3901,6 +3901,7 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re bool isVirtual = (*m_ip == CEE_CALLVIRT); bool isDelegateInvoke = false; + bool isJmp = (*m_ip == CEE_JMP); CORINFO_RESOLVED_TOKEN resolvedCallToken; CORINFO_CALL_INFO callInfo; @@ -4032,6 +4033,16 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re CORINFO_ARG_LIST_HANDLE args; args = callInfo.sig.args; + if (isJmp) + { + assert(tailcall); + // CEE_JMP is simulated as a tail call, so we need to load the current method's args + for (int i = 0; i < numArgsFromStack; i++) + { + EmitLoadVar(i); + } + } + for (int iActualArg = 0, iLogicalArg = 0; iActualArg < numArgs; iActualArg++) { if (iActualArg == extraParamArgLocation) @@ -4047,6 +4058,7 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re else { int iCurrentStackArg = iLogicalArg - numArgsFromStack; + if (iLogicalArg != 0 || !callInfo.sig.hasThis() || newObj) { CORINFO_CLASS_HANDLE classHandle; @@ -6933,12 +6945,8 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) case CEE_JMP: { CHECK_STACK(0); - uint32_t token = getU4LittleEndian(m_ip + 1); - CORINFO_RESOLVED_TOKEN resolvedToken; - ResolveToken(token, CORINFO_TOKENKIND_Method, &resolvedToken); - AddIns(INTOP_JMP); - m_pLastNewIns->data[0] = GetMethodDataItemIndex(resolvedToken.hMethod); - m_ip += 5; + EmitCall(pConstrainedToken, readonly, true /* tailcall */, false /*newObj*/, false /*isCalli*/); + linkBBlocks = false; break; } case CEE_CALLVIRT: diff --git a/src/coreclr/interpreter/inc/intops.def b/src/coreclr/interpreter/inc/intops.def index 70c692029ee5ab..1ba3e9cee29ddf 100644 --- a/src/coreclr/interpreter/inc/intops.def +++ b/src/coreclr/interpreter/inc/intops.def @@ -378,8 +378,6 @@ OPDEF(INTOP_CALL_TAIL, "call.tail", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALLI_TAIL, "calli.tail", 6, 1, 2, InterpOpLdPtr) OPDEF(INTOP_CALLVIRT_TAIL, "callvirt.tail", 4, 1, 1, InterpOpMethodHandle) -OPDEF(INTOP_JMP, "jmp", 4, 0, 0, InterpOpMethodHandle) - // The following helper call instructions exist in 2 variants, one for normal methods, and one for cases where a shared generic lookup is needed. // In the case where a shared generic lookup is needed an extra argument is passed as an svar, which is a pointer to the generic context. // If there is a generic context argument it is always the first SVar to the instruction. diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index fb09f61ae2fc97..914f16a995d11d 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -2682,8 +2682,6 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { // If we didn't get the interpreter code pointer setup, then this is a method we need to invoke as a compiled method. // Interpreter-FIXME: Implement tailcall via helpers, see https://github.com/dotnet/runtime/blob/main/docs/design/features/tailcalls-with-helpers.md - // INTOP_JMP handles only managed targets - _ASSERTE(*ip != INTOP_JMP); InvokeManagedMethod(targetMethod, callArgsAddress, returnValueAddress, (PCODE)NULL); break; } @@ -2702,10 +2700,6 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // required to be followed by a ret, so we know nothing is going to read from stack[returnOffset] after the call. pFrame->ReInit(pFrame->pParent, targetIp, pFrame->pRetVal, pFrame->pStack); } - else if (*ip == INTOP_JMP) - { - pFrame->ReInit(pFrame->pParent, targetIp, pFrame->pRetVal, pFrame->pStack); - } else { // Save current execution state for when we return from called method @@ -2736,14 +2730,6 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr pThreadContext->pStackPointer = stack + pMethod->allocaSize; break; } - case INTOP_JMP: - methodSlot = ip[1]; - targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; - // Set the offsets so that the code at CALL_INTERP_METHOD can compute the callArgsAddress and returnValueAddress - // to be the same as the current frame's ones. - callArgsOffset = 0; - returnOffset = (int32_t)(pFrame->pRetVal - stack); - goto CALL_INTERP_METHOD; case INTOP_NEWOBJ_GENERIC: { isTailcall = false; From 71d08eba41c37e864525bf3ccbe88bbda828fe37 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 30 Oct 2025 17:13:06 +0100 Subject: [PATCH 4/5] Remove forgotten usage of INTOP_JMP --- src/coreclr/interpreter/compiler.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index e64933c295e667..b8de96fa22ef5d 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -975,8 +975,7 @@ void ValidateEmittedSequenceTermination(InterpInst *lastIns) (lastIns->opcode == INTOP_CALLVIRT_TAIL) || (lastIns->opcode == INTOP_RETHROW) || (lastIns->opcode == INTOP_LEAVE_FILTER) || - (lastIns->opcode == INTOP_LEAVE_CATCH) || - (lastIns->opcode == INTOP_JMP)) + (lastIns->opcode == INTOP_LEAVE_CATCH)) { // Valid terminating instruction return; From 94ff2e247188f1113f7e33491d3faf7ef6f904e9 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 31 Oct 2025 01:07:20 +0100 Subject: [PATCH 5/5] Reflect feedback * Make the handling of generic methods with the CEE_JMP equivalent to JIT * Add tests verifying that --- src/coreclr/interpreter/compiler.cpp | 16 ++- src/tests/JIT/Directed/Directed_1.csproj | 1 + src/tests/JIT/Directed/Directed_3.csproj | 1 + src/tests/JIT/Directed/jmp/genericjmp.il | 112 +++++++++++++++++++ src/tests/JIT/Directed/jmp/genericjmp.ilproj | 9 ++ 5 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 src/tests/JIT/Directed/jmp/genericjmp.il create mode 100644 src/tests/JIT/Directed/jmp/genericjmp.ilproj diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index b8de96fa22ef5d..dea39ad1a35c3b 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -3944,6 +3944,16 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re BADCODE("Vararg methods are not supported in interpreted code"); } + if (isJmp) + { + if (callInfo.sig.numArgs != m_methodInfo->args.numArgs || + callInfo.sig.retType != m_methodInfo->args.retType || + callInfo.sig.callConv != m_methodInfo->args.callConv) + { + BADCODE("Incompatible target for CEE_JMP"); + } + } + // Inject call to callsite callout helper EmitCallsiteCallout(callInfo.accessAllowed, &callInfo.callsiteCalloutHelper); @@ -4057,7 +4067,6 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re else { int iCurrentStackArg = iLogicalArg - numArgsFromStack; - if (iLogicalArg != 0 || !callInfo.sig.hasThis() || newObj) { CORINFO_CLASS_HANDLE classHandle; @@ -6944,6 +6953,11 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) case CEE_JMP: { CHECK_STACK(0); + if (m_pCBB->clauseType != BBClauseNone) + { + // CEE_JMP inside a funclet is not allowed + BADCODE("CEE_JMP inside funclet"); + } EmitCall(pConstrainedToken, readonly, true /* tailcall */, false /*newObj*/, false /*isCalli*/); linkBBlocks = false; break; diff --git a/src/tests/JIT/Directed/Directed_1.csproj b/src/tests/JIT/Directed/Directed_1.csproj index 66ec17b4004a62..f67d9fc445dacd 100644 --- a/src/tests/JIT/Directed/Directed_1.csproj +++ b/src/tests/JIT/Directed/Directed_1.csproj @@ -2,6 +2,7 @@ + diff --git a/src/tests/JIT/Directed/Directed_3.csproj b/src/tests/JIT/Directed/Directed_3.csproj index 22f7e7688b5fd7..52a10d33386556 100644 --- a/src/tests/JIT/Directed/Directed_3.csproj +++ b/src/tests/JIT/Directed/Directed_3.csproj @@ -5,6 +5,7 @@ + diff --git a/src/tests/JIT/Directed/jmp/genericjmp.il b/src/tests/JIT/Directed/jmp/genericjmp.il new file mode 100644 index 00000000000000..e0ed32fd94a1d5 --- /dev/null +++ b/src/tests/JIT/Directed/jmp/genericjmp.il @@ -0,0 +1,112 @@ +.assembly JmpGenericTest {} +.assembly extern System.Console +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) + .ver 0:0:0:0 +} +.assembly extern System.Runtime +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) + .ver 0:0:0:0 +} +.assembly extern xunit.core {} +.assembly extern xunit.assert {} + +.assembly genericjmp { } + +.class public auto ansi beforefieldinit JmpGenericToGeneric extends [System.Runtime]System.Object +{ + .method public static void Target() cil managed + { + .maxstack 8 + ldstr "Target called" + call void [System.Console]System.Console::WriteLine(string) + ret + } + + .method public static void Source() cil managed + { + jmp void JmpGenericToGeneric::Target() + } + + .method public static void TestEntryPoint() cil managed + { + .custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = ( + 01 00 00 00 + ) + call void JmpGenericToGeneric::Source() + ret + } +} + +.class public auto ansi beforefieldinit JmpGenericToRegular extends [System.Runtime]System.Object +{ + .method public static void Target() cil managed + { + .maxstack 8 + ldstr "Regular Target called" + call void [System.Console]System.Console::WriteLine(string) + ret + } + + .method public static void Source() cil managed + { + jmp void JmpGenericToRegular::Target() + } + + .method public static void TestEntryPoint() cil managed + { + .custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = ( + 01 00 00 00 + ) + .try + { + call void JmpGenericToRegular::Source() + ldstr "JMP from generic to regular method is not allowed" + call void [xunit.assert]Xunit.Assert::Fail(string) + leave END + } + catch [System.Runtime]System.InvalidProgramException + { + leave END + } + END: + ret + } +} + +.class public auto ansi beforefieldinit JmpRegularToGeneric extends [System.Runtime]System.Object +{ + .method public static void Target() cil managed + { + .maxstack 8 + ldstr "Generic Target called" + call void [System.Console]System.Console::WriteLine(string) + ret + } + + .method public static void Source() cil managed + { + jmp void JmpRegularToGeneric::Target() + } + + .method public static void TestEntryPoint() cil managed + { + .custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = ( + 01 00 00 00 + ) + .try + { + call void JmpRegularToGeneric::Source() + ldstr "JMP from regular to generic method is not allowed" + call void [xunit.assert]Xunit.Assert::Fail(string) + leave END + } + catch [System.Runtime]System.InvalidProgramException + { + leave END + } + END: + ret + } +} \ No newline at end of file diff --git a/src/tests/JIT/Directed/jmp/genericjmp.ilproj b/src/tests/JIT/Directed/jmp/genericjmp.ilproj new file mode 100644 index 00000000000000..e30982fc15569e --- /dev/null +++ b/src/tests/JIT/Directed/jmp/genericjmp.ilproj @@ -0,0 +1,9 @@ + + + Full + 1 + + + + +