From 2afdb8f198c016984971b5d0457e339b0a050721 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 3 Jun 2025 15:27:01 -0700 Subject: [PATCH 1/7] Step 1 in shared generics 1. Handle calling convention for CEE_CALLing 2. Use getCallInfo for handling the call target resolution 3. Add handling for the param type arg as well as an ability to use the generic dictionary 4. Add a new ldtoken interpreter instruction which can take the ldtoken input as a var instead of as a data item - Not done - Anything to do with newobj - Anything to do with calli or the CODEPOINTER path - Anything to do with the LDVIRTFTN path --- src/coreclr/inc/corinfo.h | 2 +- src/coreclr/interpreter/compiler.cpp | 364 +++++++++++++++++++++++---- src/coreclr/interpreter/compiler.h | 7 +- src/coreclr/interpreter/intops.def | 7 +- src/coreclr/interpreter/intops.h | 2 + src/coreclr/vm/callstubgenerator.cpp | 22 +- src/coreclr/vm/callstubgenerator.h | 2 +- src/coreclr/vm/interpexec.cpp | 100 +++++++- src/coreclr/vm/jithelpers.cpp | 19 +- src/coreclr/vm/jitinterface.cpp | 5 + 10 files changed, 463 insertions(+), 67 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index ec2d1b54d56b17..e0148949903765 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1358,7 +1358,7 @@ enum CORINFO_CALLINFO_FLAGS CORINFO_CALLINFO_ALLOWINSTPARAM = 0x0001, // Can the compiler generate code to pass an instantiation parameters? Simple compilers should not use this flag CORINFO_CALLINFO_CALLVIRT = 0x0002, // Is it a virtual call? // UNUSED = 0x0004, - // UNUSED = 0x0008, + CORINFO_CALLINFO_DISALLOW_STUB = 0x0008, // Do not use a stub for this call, even if it is a virtual call. CORINFO_CALLINFO_SECURITYCHECKS = 0x0010, // Perform security checks. CORINFO_CALLINFO_LDFTN = 0x0020, // Resolving target of LDFTN // UNUSED = 0x0040, diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 466193eb228682..b589ac02a6e3a6 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1359,6 +1359,8 @@ int32_t InterpCompiler::GetInterpTypeStackSize(CORINFO_CLASS_HANDLE clsHnd, Inte void InterpCompiler::CreateILVars() { bool hasThis = m_methodInfo->args.hasThis(); + bool hasParamArg = m_methodInfo->args.hasTypeArg(); + int paramArgIndex = hasParamArg ? hasThis ? 1 : 0 : INT_MAX; int32_t offset, size, align; int numArgs = hasThis + m_methodInfo->args.numArgs; int numILLocals = m_methodInfo->locals.numArgs; @@ -1367,16 +1369,21 @@ void InterpCompiler::CreateILVars() // add some starting extra space for new vars m_varsCapacity = m_numILVars + m_methodInfo->EHcount + 64; m_pVars = (InterpVar*)AllocTemporary0(m_varsCapacity * sizeof (InterpVar)); - m_varsSize = m_numILVars; + m_varsSize = m_numILVars + hasParamArg; offset = 0; INTERP_DUMP("\nCreate IL Vars:\n"); CORINFO_ARG_LIST_HANDLE sigArg = m_methodInfo->args.args; - for (int i = 0; i < numArgs; i++) { + for (int i = 0; i < (numArgs + hasParamArg); i++) { InterpType interpType; CORINFO_CLASS_HANDLE argClass; + int iArgToSet = i; + if (iArgToSet > paramArgIndex) + { + iArgToSet--; // The param arg is stored after the IL locals in the m_pVars array + } if (hasThis && i == 0) { argClass = m_compHnd->getMethodClass(m_methodInfo->ftn); @@ -1385,6 +1392,14 @@ void InterpCompiler::CreateILVars() else interpType = InterpTypeO; } + else if (i == paramArgIndex) + { + iArgToSet = m_varsSize - 1; // The param arg is stored after the IL locals in the m_pVars array + m_paramArgIndex = iArgToSet; + INTERP_DUMP("Param arg at index m_pVars[%d]\n", iArgToSet); + interpType = InterpTypeI; + argClass = NULL; // No class for the param arg + } else { CorInfoType argCorType; @@ -1394,14 +1409,14 @@ void InterpCompiler::CreateILVars() } size = GetInterpTypeStackSize(argClass, interpType, &align); - new (&m_pVars[i]) InterpVar(interpType, argClass, size); + new (&m_pVars[iArgToSet]) InterpVar(interpType, argClass, size); - m_pVars[i].global = true; - m_pVars[i].ILGlobal = true; - m_pVars[i].size = size; + m_pVars[iArgToSet].global = true; + m_pVars[iArgToSet].ILGlobal = true; + m_pVars[iArgToSet].size = size; offset = ALIGN_UP_TO(offset, align); - m_pVars[i].offset = offset; - INTERP_DUMP("alloc arg var %d to offset %d\n", i, offset); + m_pVars[iArgToSet].offset = offset; + INTERP_DUMP("alloc arg var %d to offset %d in m_pVars[%d]\n", i, offset, iArgToSet); offset += size; } @@ -1431,6 +1446,12 @@ void InterpCompiler::CreateILVars() index++; } + if (hasParamArg) + { + // The param arg is stored after the IL locals in the m_pVars array + index++; + } + offset = ALIGN_UP_TO(offset, INTERP_STACK_ALIGNMENT); m_ILLocalsSize = offset - m_ILLocalsOffset; @@ -2091,6 +2112,16 @@ int32_t InterpCompiler::GetDataItemIndex(void *data) return m_dataItems.Add(data); } +void* InterpCompiler::GetDataItemAtIndex(int32_t index) +{ + if (index < 0 || index >= m_dataItems.GetSize()) + { + assert(!"Invalid data item index"); + return NULL; + } + return m_dataItems.Get(index); +} + int32_t InterpCompiler::GetMethodDataItemIndex(CORINFO_METHOD_HANDLE mHandle) { size_t data = (size_t)mHandle | INTERP_METHOD_HANDLE_TAG; @@ -2178,46 +2209,188 @@ CORINFO_CLASS_HANDLE InterpCompiler::ResolveClassToken(uint32_t token) return resolvedToken.hClass; } -void InterpCompiler::EmitCall(CORINFO_CLASS_HANDLE constrainedClass, bool readonly, bool tailcall) +CORINFO_CLASS_HANDLE InterpCompiler::getClassFromContext(CORINFO_CONTEXT_HANDLE context) +{ + if (context == METHOD_BEING_COMPILED_CONTEXT()) + { + return m_compHnd->getMethodClass(m_methodHnd); // This really should be just a field access, but we don't have that field in the InterpCompiler now + } + + if (((SIZE_T)context & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS) + { + return CORINFO_CLASS_HANDLE((SIZE_T)context & ~CORINFO_CONTEXTFLAGS_MASK); + } + else + { + return m_compHnd->getMethodClass(CORINFO_METHOD_HANDLE((SIZE_T)context & ~CORINFO_CONTEXTFLAGS_MASK)); + } +} + +int InterpCompiler::getParamArgIndex() +{ + return m_paramArgIndex; +} + +int InterpCompiler::FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, bool embedParent, int existingTemp) +{ + CORINFO_GENERICHANDLE_RESULT embedInfo; + m_compHnd->embedGenericHandle(resolvedToken, false, m_methodInfo->ftn, &embedInfo); + + int resultVar = existingTemp; + if (resultVar == -1) + { + PushStackType(StackTypeI, NULL); + resultVar = m_pStackPointer[-1].var; + m_pStackPointer--; + } + + if (embedInfo.lookup.lookupKind.needsRuntimeLookup) + { + CORINFO_RUNTIME_LOOKUP_KIND runtimeLookupKind = embedInfo.lookup.lookupKind.runtimeLookupKind; + if (runtimeLookupKind == CORINFO_LOOKUP_METHODPARAM) + { + AddIns(INTOP_GENERICLOOKUP_METHOD); + } + else if (runtimeLookupKind == CORINFO_LOOKUP_THISOBJ) + { + AddIns(INTOP_GENERICLOOKUP_THIS); + } + else + { + AddIns(INTOP_GENERICLOOKUP_CLASS); + } + CORINFO_RUNTIME_LOOKUP *pRuntimeLookup = (CORINFO_RUNTIME_LOOKUP*)AllocMethodData(sizeof(CORINFO_RUNTIME_LOOKUP)); + *pRuntimeLookup = embedInfo.lookup.runtimeLookup; + m_pLastNewIns->data[0] = GetDataItemIndex(pRuntimeLookup); + + m_pLastNewIns->SetSVar(getParamArgIndex()); + m_pLastNewIns->SetDVar(resultVar); + } + else + { + AddIns(INTOP_LDPTR); + m_pLastNewIns->SetDVar(resultVar); + + assert(embedInfo.lookup.constLookup.accessType == IAT_VALUE); + m_pLastNewIns->data[0] = GetDataItemIndex(embedInfo.lookup.constLookup.handle); + } + return resultVar; +} + +void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool readonly, bool tailcall) { uint32_t token = getU4LittleEndian(m_ip + 1); bool isVirtual = (*m_ip == CEE_CALLVIRT); - CORINFO_METHOD_HANDLE targetMethod = ResolveMethodToken(token); + CORINFO_RESOLVED_TOKEN resolvedCallToken; + ResolveToken(token, CORINFO_TOKENKIND_Method, &resolvedCallToken); - CORINFO_SIG_INFO targetSignature; - m_compHnd->getMethodSig(targetMethod, &targetSignature); - - uint32_t mflags = m_compHnd->getMethodAttribs(targetMethod); + CORINFO_CALL_INFO callInfo; + CORINFO_CALLINFO_FLAGS flags = (CORINFO_CALLINFO_FLAGS)(CORINFO_CALLINFO_ALLOWINSTPARAM | CORINFO_CALLINFO_SECURITYCHECKS | CORINFO_CALLINFO_DISALLOW_STUB); + if (isVirtual) + flags = (CORINFO_CALLINFO_FLAGS)(flags | CORINFO_CALLINFO_CALLVIRT); - if (isVirtual && (!(mflags & CORINFO_FLG_VIRTUAL) || (mflags & CORINFO_FLG_FINAL))) - isVirtual = false; + m_compHnd->getCallInfo(&resolvedCallToken, constrainedClass, m_methodInfo->ftn, flags, &callInfo); - if (EmitCallIntrinsics(targetMethod, targetSignature)) + if (EmitCallIntrinsics(callInfo.hMethod, callInfo.sig)) { m_ip += 5; return; } // Process sVars - int numArgs = targetSignature.numArgs + targetSignature.hasThis(); - m_pStackPointer -= numArgs; + int numArgsFromStack = callInfo.sig.numArgs + callInfo.sig.hasThis(); + int numArgs = numArgsFromStack; + m_pStackPointer -= numArgsFromStack; + + int extraParamArgLocation = INT_MAX; + if (callInfo.sig.hasTypeArg()) + { + extraParamArgLocation = callInfo.sig.hasThis() ? 1 : 0; + numArgs++; + } int *callArgs = (int*) AllocMemPool((numArgs + 1) * sizeof(int)); - for (int i = 0; i < numArgs; i++) - callArgs[i] = m_pStackPointer [i].var; + for (int iActualArg = 0, iLogicalArg = 0; iActualArg < numArgs; iActualArg++) + { + if (iActualArg == extraParamArgLocation) + { + // This is the extra type argument, which is not on the logical IL stack + // Skip it for now. We will fill it in later. + } + else + { + callArgs[iActualArg] = m_pStackPointer [iLogicalArg].var; + } + } callArgs[numArgs] = -1; + if (extraParamArgLocation != INT_MAX) + { + PushStackType(StackTypeI, NULL); + m_pStackPointer--; + int contextParamVar = m_pStackPointer[0].var; + callArgs[extraParamArgLocation] = contextParamVar; + + // Instantiated generic method + CORINFO_CONTEXT_HANDLE exactContextHnd = callInfo.contextHandle; + if (((SIZE_T)exactContextHnd & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD) + { + assert(exactContextHnd != METHOD_BEING_COMPILED_CONTEXT()); + + CORINFO_METHOD_HANDLE exactMethodHandle = + (CORINFO_METHOD_HANDLE)((SIZE_T)exactContextHnd & ~CORINFO_CONTEXTFLAGS_MASK); + + if (!callInfo.exactContextNeedsRuntimeLookup) + { + AddIns(INTOP_LDPTR); + m_pLastNewIns->SetDVar(contextParamVar); + m_pLastNewIns->data[0] = GetDataItemIndex((void*)exactMethodHandle); + } + else + { + FillTempVarWithToken(&resolvedCallToken, false, contextParamVar); + } + } + + // otherwise must be an instance method in a generic struct, + // a static method in a generic type, or a runtime-generated array method + else + { + assert(((SIZE_T)exactContextHnd & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS); + CORINFO_CLASS_HANDLE exactClassHandle = getClassFromContext(exactContextHnd); + + if ((callInfo.classFlags & CORINFO_FLG_ARRAY) && readonly) + { + // We indicate "readonly" to the Address operation by using a null + // instParam. + AddIns(INTOP_LDPTR); + m_pLastNewIns->SetDVar(contextParamVar); + m_pLastNewIns->data[0] = GetDataItemIndex(NULL); + } + else if (!callInfo.exactContextNeedsRuntimeLookup) + { + AddIns(INTOP_LDPTR); + m_pLastNewIns->SetDVar(contextParamVar); + m_pLastNewIns->data[0] = GetDataItemIndex((void*)exactClassHandle); + } + else + { + FillTempVarWithToken(&resolvedCallToken, true, contextParamVar); + } + } + } + // Process dVar int32_t dVar; - if (targetSignature.retType != CORINFO_TYPE_VOID) + if (callInfo.sig.retType != CORINFO_TYPE_VOID) { - InterpType interpType = GetInterpType(targetSignature.retType); + InterpType interpType = GetInterpType(callInfo.sig.retType); if (interpType == InterpTypeVT) { - int32_t size = m_compHnd->getClassSize(targetSignature.retTypeClass); - PushTypeVT(targetSignature.retTypeClass, size); + int32_t size = m_compHnd->getClassSize(callInfo.sig.retTypeClass); + PushTypeVT(callInfo.sig.retTypeClass, size); } else { @@ -2236,16 +2409,47 @@ void InterpCompiler::EmitCall(CORINFO_CLASS_HANDLE constrainedClass, bool readon } // Emit call instruction - if (isVirtual) - { - AddIns(INTOP_CALLVIRT); - m_pLastNewIns->data[0] = GetDataItemIndex(targetMethod); - } - else + switch (callInfo.kind) { - AddIns(INTOP_CALL); - m_pLastNewIns->data[0] = GetMethodDataItemIndex(targetMethod); + case CORINFO_CALL: + // Normal call + if (callInfo.nullInstanceCheck) + { + // If the call is a normal call, we need to check for null instance + // before the call. + // TODO: Add null checking behavior somewhere here! + } + AddIns(INTOP_CALL); + m_pLastNewIns->data[0] = GetMethodDataItemIndex(callInfo.hMethod); + break; + + case CORINFO_CALL_CODE_POINTER: + if (callInfo.nullInstanceCheck) + { + // If the call is a normal call, we need to check for null instance + // before the call. + // TODO: Add null checking behavior somewhere here! + } + assert(!"Need to support calling a code pointer"); + break; + + case CORINFO_VIRTUALCALL_VTABLE: + // Traditional virtual call. + AddIns(INTOP_CALLVIRT); + m_pLastNewIns->data[0] = GetDataItemIndex(callInfo.hMethod); + break; + + case CORINFO_VIRTUALCALL_LDVIRTFTN: + // Resolve a virtual call using the helper function to a function pointer, and then call through that + assert("!Need to support ldvirtftn"); + break; + + case CORINFO_VIRTUALCALL_STUB: + // This case should never happen + assert(!"Unexpected call kind"); + break; } + m_pLastNewIns->SetDVar(dVar); m_pLastNewIns->SetSVar(CALL_ARGS_SVAR); @@ -2505,7 +2709,8 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) bool readonly = false; bool tailcall = false; bool volatile_ = false; - CORINFO_CLASS_HANDLE constrainedClass = NULL; + CORINFO_RESOLVED_TOKEN* constrainedClass = NULL; + CORINFO_RESOLVED_TOKEN constrainedToken; uint8_t *codeEnd; int numArgs = m_methodInfo->args.hasThis() + m_methodInfo->args.numArgs; bool emittedBBlocks, linkBBlocks, needsRetryEmit; @@ -3853,14 +4058,13 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) case CEE_CONSTRAINED: { uint32_t token = getU4LittleEndian(m_ip + 1); - CORINFO_RESOLVED_TOKEN resolvedToken; - - resolvedToken.tokenScope = m_compScopeHnd; - resolvedToken.tokenContext = METHOD_BEING_COMPILED_CONTEXT(); - resolvedToken.token = token; - resolvedToken.tokenType = CORINFO_TOKENKIND_Constrained; - m_compHnd->resolveToken(&resolvedToken); - constrainedClass = resolvedToken.hClass; + + constrainedToken.tokenScope = m_compScopeHnd; + constrainedToken.tokenContext = METHOD_BEING_COMPILED_CONTEXT(); + constrainedToken.token = token; + constrainedToken.tokenType = CORINFO_TOKENKIND_Constrained; + m_compHnd->resolveToken(&constrainedToken); + constrainedClass = &constrainedToken; m_ip += 5; break; } @@ -4204,39 +4408,54 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) case CEE_LDTOKEN: { - AddIns(INTOP_LDTOKEN); - + CORINFO_RESOLVED_TOKEN resolvedToken; ResolveToken(getU4LittleEndian(m_ip + 1), CORINFO_TOKENKIND_Ldtoken, &resolvedToken); + + CORINFO_GENERICHANDLE_RESULT embedInfo; + m_compHnd->embedGenericHandle(&resolvedToken, false, m_methodInfo->ftn, &embedInfo); + + if (embedInfo.lookup.lookupKind.needsRuntimeLookup) + { + int runtimeLookupVar = FillTempVarWithToken(&resolvedToken, false); + m_pLastNewIns->SetSVar(getParamArgIndex()); + m_pLastNewIns->SetDVar(runtimeLookupVar); + AddIns(INTOP_LDTOKEN_VAR); + m_pLastNewIns->SetSVar(runtimeLookupVar); + } + else + { + AddIns(INTOP_LDTOKEN); + assert(embedInfo.lookup.constLookup.accessType == IAT_VALUE); + m_pLastNewIns->data[1] = GetDataItemIndex(embedInfo.lookup.constLookup.handle); + } + CORINFO_CLASS_HANDLE clsHnd = m_compHnd->getTokenTypeAsHandle(&resolvedToken); PushStackType(StackTypeVT, clsHnd); m_pLastNewIns->SetDVar(m_pStackPointer[-1].var); - + // see jit/importer.cpp CEE_LDTOKEN CorInfoHelpFunc helper; if (resolvedToken.hClass) { helper = CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE; - m_pLastNewIns->data[0] = GetDataItemIndex(resolvedToken.hClass); } else if (resolvedToken.hMethod) { helper = CORINFO_HELP_METHODDESC_TO_STUBRUNTIMEMETHOD; - m_pLastNewIns->data[0] = GetDataItemIndex(resolvedToken.hMethod); } else if (resolvedToken.hField) { helper = CORINFO_HELP_FIELDDESC_TO_STUBRUNTIMEFIELD; - m_pLastNewIns->data[0] = GetDataItemIndex(resolvedToken.hField); } else { helper = CORINFO_HELP_FAIL_FAST; assert(!"Token not resolved or resolved to unexpected type"); } - - m_pLastNewIns->data[1] = GetDataItemIndexForHelperFtn(helper); + m_pLastNewIns->data[0] = GetDataItemIndexForHelperFtn(helper); + m_ip += 5; break; } @@ -4428,6 +4647,20 @@ void InterpCompiler::PrintIns(InterpInst *ins) PrintInsData(ins, ins->ilOffset, &ins->data[0], ins->opcode); } +static const char* s_jitHelperNames[CORINFO_HELP_COUNT] = { +#define JITHELPER(code, pfnHelper, binderId) #code, +#define DYNAMICJITHELPER(code, pfnHelper, binderId) #code, +#include "jithelpers.h" +}; + +const char* CorInfoHelperToName(CorInfoHelpFunc helper) +{ + if (helper < 0 || helper >= CORINFO_HELP_COUNT) + return "UnknownHelper"; + + return s_jitHelperNames[helper]; +} + void InterpCompiler::PrintInsData(InterpInst *ins, int32_t insOffset, const int32_t *pData, int32_t opcode) { switch (g_interpOpArgType[opcode]) { @@ -4465,6 +4698,37 @@ void InterpCompiler::PrintInsData(InterpInst *ins, int32_t insOffset, const int3 else printf(" IR_%04x", insOffset + *pData); break; + case InterpOpLdPtr: + { + printf("%p", (void*)GetDataItemAtIndex(pData[0])); + break; + } + case InterpOpGenericLookup: + { + CORINFO_RUNTIME_LOOKUP *pGenericLookup = (CORINFO_RUNTIME_LOOKUP*)GetDataItemAtIndex(pData[0]); + printf("%s,%p[", CorInfoHelperToName(pGenericLookup->helper), pGenericLookup->signature); + for (int i = 0; i < pGenericLookup->indirections; i++) + { + if (i > 0) + printf(","); + + if (i == 0 && pGenericLookup->indirectFirstOffset) + printf("*"); + if (i == 1 && pGenericLookup->indirectSecondOffset) + printf("*"); + printf("%d", (int)pGenericLookup->offsets[i]); + } + printf("]"); + if (pGenericLookup->sizeOffset != CORINFO_NO_SIZE_CHECK) + { + printf(" sizeOffset=%d", (int)pGenericLookup->sizeOffset); + } + if (pGenericLookup->testForNull) + { + printf(" testForNull"); + } + } + break; case InterpOpSwitch: { int32_t n = *pData; diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 9ee206edd34098..358f6b0cf49b21 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -343,6 +343,7 @@ class InterpCompiler // FIXME during compilation this should be a hashtable for fast lookup of duplicates TArray m_dataItems; int32_t GetDataItemIndex(void* data); + void* GetDataItemAtIndex(int32_t index); int32_t GetMethodDataItemIndex(CORINFO_METHOD_HANDLE mHandle); int32_t GetDataItemIndexForHelperFtn(CorInfoHelpFunc ftn); @@ -353,6 +354,9 @@ class InterpCompiler void ResolveToken(uint32_t token, CorInfoTokenKind tokenKind, CORINFO_RESOLVED_TOKEN *pResolvedToken); CORINFO_METHOD_HANDLE ResolveMethodToken(uint32_t token); CORINFO_CLASS_HANDLE ResolveClassToken(uint32_t token); + CORINFO_CLASS_HANDLE getClassFromContext(CORINFO_CONTEXT_HANDLE context); + int getParamArgIndex(); // Get the index into the m_pVars array of the Parameter argument. This is either the this pointer, a methoddesc or a class handle + int FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, bool embedParent, int existingVar = -1); void* AllocMethodData(size_t numBytes); public: @@ -411,6 +415,7 @@ class InterpCompiler int32_t m_varsSize = 0; int32_t m_varsCapacity = 0; int32_t m_numILVars = 0; + int32_t m_paramArgIndex = 0; // Index of the type parameter argument in the m_pVars array. // For each catch or filter clause, we create a variable that holds the exception object. // This is the index of the first such variable. int32_t m_clauseVarsIndex = 0; @@ -448,7 +453,7 @@ class InterpCompiler void EmitUnaryArithmeticOp(int32_t opBase); void EmitShiftOp(int32_t opBase); void EmitCompareOp(int32_t opBase); - void EmitCall(CORINFO_CLASS_HANDLE constrainedClass, bool readonly, bool tailcall); + void EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool readonly, bool tailcall); bool EmitCallIntrinsics(CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO sig); void EmitLdind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset); void EmitStind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset, bool reverseSVarOrder); diff --git a/src/coreclr/interpreter/intops.def b/src/coreclr/interpreter/intops.def index 3fe446900541cd..80e5121c7bc0c0 100644 --- a/src/coreclr/interpreter/intops.def +++ b/src/coreclr/interpreter/intops.def @@ -21,7 +21,7 @@ OPDEF(INTOP_LDC_I8, "ldc.i8", 4, 1, 0, InterpOpLongInt) OPDEF(INTOP_LDC_R4, "ldc.r4", 3, 1, 0, InterpOpFloat) OPDEF(INTOP_LDC_R8, "ldc.r8", 4, 1, 0, InterpOpDouble) -OPDEF(INTOP_LDPTR, "ldptr", 3, 1, 0, InterpOpInt) +OPDEF(INTOP_LDPTR, "ldptr", 3, 1, 0, InterpOpLdPtr) OPDEF(INTOP_NEWARR, "newarr", 5, 1, 1, InterpOpInt) OPDEF(INTOP_LDELEM_I1, "ldelem.i1", 4, 1, 2, InterpOpNoArgs) @@ -43,6 +43,7 @@ OPDEF(INTOP_STELEM_R4, "stelem.r4", 4, 0, 3, InterpOpNoArgs) OPDEF(INTOP_STELEM_R8, "stelem.r8", 4, 0, 3, InterpOpNoArgs) OPDEF(INTOP_LDTOKEN, "ldtoken", 4, 1, 0, InterpOpTwoInts) // [token data item] [conversion helper func] +OPDEF(INTOP_LDTOKEN_VAR, "ldtoken.var", 4, 1, 1, InterpOpInt) // [var index] [conversion helper func] OPDEF(INTOP_MOV_I4_I1, "mov.i4.i1", 3, 1, 1, InterpOpNoArgs) OPDEF(INTOP_MOV_I4_U1, "mov.i4.u1", 3, 1, 1, InterpOpNoArgs) @@ -285,6 +286,10 @@ OPDEF(INTOP_NEWOBJ_VT, "newobj.vt", 5, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALL_HELPER_PP, "call.helper.pp", 4, 1, 0, InterpOpTwoInts) OPDEF(INTOP_CALL_HELPER_PP_2, "call.helper.pp.2", 5, 1, 1, InterpOpTwoInts) +OPDEF(INTOP_GENERICLOOKUP_METHOD, "generic.method", 4, 1, 1, InterpOpGenericLookup) +OPDEF(INTOP_GENERICLOOKUP_CLASS, "generic.class", 4, 1, 1, InterpOpGenericLookup) +OPDEF(INTOP_GENERICLOOKUP_THIS, "generic.this", 4, 1, 1, InterpOpGenericLookup) + OPDEF(INTOP_CALL_FINALLY, "call.finally", 2, 0, 0, InterpOpBranch) OPDEF(INTOP_ZEROBLK_IMM, "zeroblk.imm", 3, 0, 1, InterpOpInt) diff --git a/src/coreclr/interpreter/intops.h b/src/coreclr/interpreter/intops.h index 6886400ac35fe0..5ff775b2b0d896 100644 --- a/src/coreclr/interpreter/intops.h +++ b/src/coreclr/interpreter/intops.h @@ -22,6 +22,8 @@ typedef enum InterpOpSwitch, InterpOpMethodHandle, InterpOpClassHandle, + InterpOpGenericLookup, + InterpOpLdPtr, } InterpOpArgType; extern const uint8_t g_interpOpLen[]; diff --git a/src/coreclr/vm/callstubgenerator.cpp b/src/coreclr/vm/callstubgenerator.cpp index 30a70af7ace201..a8e65fab3ee5b0 100644 --- a/src/coreclr/vm/callstubgenerator.cpp +++ b/src/coreclr/vm/callstubgenerator.cpp @@ -582,7 +582,15 @@ CallStubHeader *CallStubGenerator::GenerateCallStub(MethodDesc *pMD, AllocMemTra // Allocate space for the routines. The size of the array is conservatively set to twice the number of arguments // plus one slot for the target pointer and reallocated to the real size at the end. - PCODE *pRoutines = (PCODE*)alloca(sizeof(CallStubHeader) + (numArgs * 2 + 1) * sizeof(PCODE)); + PCODE *pRoutines = (PCODE*)alloca(sizeof(CallStubHeader) + ((numArgs + 1) * 2 + 1) * sizeof(PCODE)); + + if (argIt.HasParamType()) + { + // In the Interpreter calling convention the argument after the "this" pointer is the parameter type + ArgLocDesc paramArgLocDesc; + argIt.GetParamTypeLoc(¶mArgLocDesc); + ProcessArgument(NULL, paramArgLocDesc, pRoutines); + } int ofs; while ((ofs = argIt.GetNextOffset()) != TransitionBlock::InvalidOffset) @@ -642,13 +650,13 @@ CallStubHeader *CallStubGenerator::GenerateCallStub(MethodDesc *pMD, AllocMemTra assert(!"Unhandled systemv classification for argument in GenerateCallStub"); break; } - ProcessArgument(argIt, argLocDescEightByte, pRoutines); + ProcessArgument(&argIt, argLocDescEightByte, pRoutines); } } else #endif // UNIX_AMD64_ABI { - ProcessArgument(argIt, argLocDesc, pRoutines); + ProcessArgument(&argIt, argLocDesc, pRoutines); } } @@ -857,7 +865,7 @@ CallStubHeader *CallStubGenerator::GenerateCallStub(MethodDesc *pMD, AllocMemTra // Process the argument described by argLocDesc. This function is called for each argument in the method signature. // It updates the ranges of registers and emits entries into the routines array at discontinuities. -void CallStubGenerator::ProcessArgument(ArgIterator& argIt, ArgLocDesc& argLocDesc, PCODE *pRoutines) +void CallStubGenerator::ProcessArgument(ArgIterator *pArgIt, ArgLocDesc& argLocDesc, PCODE *pRoutines) { LIMITED_METHOD_CONTRACT; @@ -895,7 +903,7 @@ void CallStubGenerator::ProcessArgument(ArgIterator& argIt, ArgLocDesc& argLocDe m_r1 = argLocDesc.m_idxGenReg; m_r2 = m_r1 + argLocDesc.m_cGenReg - 1; } - else if (argLocDesc.m_idxGenReg == m_r2 + 1 && !argIt.IsArgPassedByRef()) + else if (argLocDesc.m_idxGenReg == m_r2 + 1 && (!pArgIt || !pArgIt->IsArgPassedByRef())) { // Extend an existing range, but only if the argument is not passed by reference. // Arguments passed by reference are handled separately, because the interpreter stores the value types on its stack by value. @@ -987,11 +995,11 @@ void CallStubGenerator::ProcessArgument(ArgIterator& argIt, ArgLocDesc& argLocDe // Arguments passed by reference are handled separately, because the interpreter stores the value types on its stack by value. // So the argument loading routine needs to load the address of the argument. To avoid explosion of number of the routines, // we always process single argument passed by reference using single routine. - if (argIt.IsArgPassedByRef()) + if (pArgIt != NULL && pArgIt->IsArgPassedByRef()) { _ASSERTE(argLocDesc.m_cGenReg == 1); pRoutines[m_routineIndex++] = GetGPRegRefLoadRoutine(argLocDesc.m_idxGenReg); - pRoutines[m_routineIndex++] = argIt.GetArgSize(); + pRoutines[m_routineIndex++] = pArgIt->GetArgSize(); m_r1 = NoRange; } #endif // UNIX_AMD64_ABI diff --git a/src/coreclr/vm/callstubgenerator.h b/src/coreclr/vm/callstubgenerator.h index 476bb2f4c9da91..bce448b5e00ce1 100644 --- a/src/coreclr/vm/callstubgenerator.h +++ b/src/coreclr/vm/callstubgenerator.h @@ -68,7 +68,7 @@ class CallStubGenerator int m_totalStackSize; // Process the argument described by argLocDesc. This function is called for each argument in the method signature. - void ProcessArgument(ArgIterator& argIt, ArgLocDesc& argLocDesc, PCODE *pRoutines); + void ProcessArgument(ArgIterator *pArgIt, ArgLocDesc& argLocDesc, PCODE *pRoutines); public: // Generate the call stub for the given method. CallStubHeader *GenerateCallStub(MethodDesc *pMD, AllocMemTracker *pamTracker); diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index a196dc871a625f..3bfc227a9d1879 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -63,6 +63,20 @@ InterpThreadContext::~InterpThreadContext() free(pStackStart); } +CORINFO_GENERIC_HANDLE GenericHandleWorkerCore(MethodDesc * pMD, MethodTable * pMT, LPVOID signature, DWORD dictionaryIndexAndSlot, Module* pModule); + +CORINFO_GENERIC_HANDLE GenericHandleCommon(MethodDesc * pMD, MethodTable * pMT, LPVOID signature) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + } CONTRACTL_END; + GCX_PREEMP(); + return GenericHandleWorkerCore(pMD, pMT, signature, 0xFFFFFFFF, NULL); +} + #ifdef DEBUG static void InterpBreakpoint() { @@ -1479,10 +1493,77 @@ do { \ STELEM(double, double); break; } - case INTOP_LDTOKEN: +#define DO_GENERIC_LOOKUP(mdParam, mtParam) \ + CORINFO_RUNTIME_LOOKUP *pLookup = (CORINFO_RUNTIME_LOOKUP*)pMethod->pDataItems[ip[3]]; \ + CORINFO_GENERIC_HANDLE result = 0; \ + \ + assert(!pLookup->indirectFirstOffset); \ + assert(!pLookup->indirectSecondOffset); \ + lookup = *(uint8_t**)(lookup + pLookup->offsets[0]); \ + if (pLookup->indirections >= 3) \ + lookup = *(uint8_t**)(lookup + pLookup->offsets[1]); \ + if (pLookup->indirections >= 4) \ + lookup = *(uint8_t**)(lookup + pLookup->offsets[2]); \ + do { \ + size_t lastOffset = pLookup->offsets[pLookup->indirections - 1]; \ + if (pLookup->sizeOffset != CORINFO_NO_SIZE_CHECK) \ + { \ + /* Last indirection is the size*/ \ + size_t size = *(size_t*)(lookup + pLookup->sizeOffset); \ + if (size <= lastOffset) \ + { \ + result = GenericHandleCommon(mdParam, mtParam, pLookup->signature); \ + break; \ + } \ + } \ + lookup = *(uint8_t**)(lookup + lastOffset); \ + \ + if (lookup == NULL) \ + { \ + result = GenericHandleCommon(mdParam, mtParam, pLookup->signature); \ + break; \ + } \ + result = (CORINFO_GENERIC_HANDLE)lookup; \ + } while (0); \ + LOCAL_VAR(dreg, CORINFO_GENERIC_HANDLE) = result; \ + ip += 4; + + case INTOP_GENERICLOOKUP_THIS: + { + int dreg = ip[1]; + int sreg = ip[2]; + OBJECTREF thisPtr = LOCAL_VAR(sreg, OBJECTREF); + if (thisPtr == NULL) + assert(0); // TODO: throw NullReferenceException + MethodTable *pMT = thisPtr->GetMethodTable(); + uint8_t *lookup = (uint8_t*)pMT; + + DO_GENERIC_LOOKUP(NULL, pMT); + + break; + } + case INTOP_GENERICLOOKUP_METHOD: + { + int dreg = ip[1]; + int sreg = ip[2]; + MethodDesc *paramReg = LOCAL_VAR(sreg, MethodDesc*); + uint8_t *lookup = (uint8_t*)paramReg; + DO_GENERIC_LOOKUP(paramReg, NULL); + break; + } + case INTOP_GENERICLOOKUP_CLASS: { int dreg = ip[1]; - void *nativeHandle = pMethod->pDataItems[ip[2]]; + int sreg = ip[2]; + MethodTable *paramReg = LOCAL_VAR(sreg, MethodTable*); + uint8_t *lookup = (uint8_t*)paramReg; + DO_GENERIC_LOOKUP(NULL, paramReg); + break; + } + case INTOP_LDTOKEN_VAR: + { + int dreg = ip[1]; + void *nativeHandle = LOCAL_VAR(ip[2], void*); size_t helperDirectOrIndirect = (size_t)pMethod->pDataItems[ip[3]]; HELPER_FTN_PP helper = nullptr; if (helperDirectOrIndirect & INTERP_INDIRECT_HELPER_TAG) @@ -1494,6 +1575,21 @@ do { \ ip += 4; break; } + case INTOP_LDTOKEN: + { + int dreg = ip[1]; + void *nativeHandle = pMethod->pDataItems[ip[3]]; + size_t helperDirectOrIndirect = (size_t)pMethod->pDataItems[ip[2]]; + HELPER_FTN_PP helper = nullptr; + if (helperDirectOrIndirect & INTERP_INDIRECT_HELPER_TAG) + helper = *(HELPER_FTN_PP *)(helperDirectOrIndirect & ~INTERP_INDIRECT_HELPER_TAG); + else + helper = (HELPER_FTN_PP)helperDirectOrIndirect; + void *managedHandle = helper(nativeHandle); + LOCAL_VAR(dreg, void*) = managedHandle; + ip += 4; + break; + } case INTOP_CALL_FINALLY: { const int32_t* targetIp = ip + ip[1]; diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 74d550ee0ae3bc..5b1cf3cf2086cb 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -989,14 +989,12 @@ extern "C" void QCALLTYPE ThrowInvalidCastException(CORINFO_CLASS_HANDLE pTarget // //======================================================================== -extern "C" CORINFO_GENERIC_HANDLE QCALLTYPE GenericHandleWorker(MethodDesc * pMD, MethodTable * pMT, LPVOID signature, DWORD dictionaryIndexAndSlot, Module* pModule) +CORINFO_GENERIC_HANDLE GenericHandleWorkerCore(MethodDesc * pMD, MethodTable * pMT, LPVOID signature, DWORD dictionaryIndexAndSlot, Module* pModule) { - QCALL_CONTRACT; + STANDARD_VM_CONTRACT; CORINFO_GENERIC_HANDLE result = NULL; - BEGIN_QCALL; - _ASSERTE(pMT != NULL || pMD != NULL); _ASSERTE(pMT == NULL || pMD == NULL); @@ -1059,6 +1057,19 @@ extern "C" CORINFO_GENERIC_HANDLE QCALLTYPE GenericHandleWorker(MethodDesc * pMD } } + return result; +} + +extern "C" CORINFO_GENERIC_HANDLE QCALLTYPE GenericHandleWorker(MethodDesc * pMD, MethodTable * pMT, LPVOID signature, DWORD dictionaryIndexAndSlot, Module* pModule) +{ + QCALL_CONTRACT; + + CORINFO_GENERIC_HANDLE result = NULL; + + BEGIN_QCALL; + + result = GenericHandleWorkerCore(pMD, pMT, signature, dictionaryIndexAndSlot, pModule); + END_QCALL; return result; diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index b8eae789628ff6..46b8dc181c2057 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -5279,6 +5279,11 @@ void CEEInfo::getCallInfo( pResult->kind = CORINFO_VIRTUALCALL_VTABLE; pResult->nullInstanceCheck = TRUE; } + else if (!fIsStaticVirtualMethod && (flags & CORINFO_CALLINFO_DISALLOW_STUB)) + { + pResult->kind = CORINFO_VIRTUALCALL_LDVIRTFTN; // Use the ldvirtftn code path if we are not allowed to use a stub + pResult->nullInstanceCheck = TRUE; + } else { // No need to null check - the dispatch code will deal with null this. From 1b76ff188a48dae435f2e9054b6ed5f522eb182a Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 5 Jun 2025 15:16:52 -0700 Subject: [PATCH 2/7] Implement newobj for normal classes and structs --- src/coreclr/interpreter/compiler.cpp | 134 ++++++++++++++++---- src/coreclr/interpreter/compiler.h | 4 +- src/coreclr/interpreter/interpretershared.h | 14 +- src/coreclr/vm/amd64/asmconstants.h | 2 +- src/coreclr/vm/callstubgenerator.cpp | 10 ++ src/coreclr/vm/interpexec.cpp | 80 ++++-------- src/coreclr/vm/interpexec.h | 4 +- src/coreclr/vm/method.hpp | 18 +++ src/coreclr/vm/prestub.cpp | 3 +- 9 files changed, 188 insertions(+), 81 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index b589ac02a6e3a6..ad1b7ab2be4e03 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2124,8 +2124,7 @@ void* InterpCompiler::GetDataItemAtIndex(int32_t index) int32_t InterpCompiler::GetMethodDataItemIndex(CORINFO_METHOD_HANDLE mHandle) { - size_t data = (size_t)mHandle | INTERP_METHOD_HANDLE_TAG; - return GetDataItemIndex((void*)data); + return GetDataItemIndex((void*)mHandle); } int32_t InterpCompiler::GetDataItemIndexForHelperFtn(CorInfoHelpFunc ftn) @@ -2231,13 +2230,13 @@ int InterpCompiler::getParamArgIndex() return m_paramArgIndex; } -int InterpCompiler::FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, bool embedParent, int existingTemp) +int InterpCompiler::FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, bool embedParent, bool onlyIfNeedsRuntimeLookup, int existingTemp) { CORINFO_GENERICHANDLE_RESULT embedInfo; - m_compHnd->embedGenericHandle(resolvedToken, false, m_methodInfo->ftn, &embedInfo); + m_compHnd->embedGenericHandle(resolvedToken, embedParent, m_methodInfo->ftn, &embedInfo); int resultVar = existingTemp; - if (resultVar == -1) + if ((resultVar == -1) && (!onlyIfNeedsRuntimeLookup || (embedInfo.lookup.lookupKind.needsRuntimeLookup))) { PushStackType(StackTypeI, NULL); resultVar = m_pStackPointer[-1].var; @@ -2266,7 +2265,7 @@ int InterpCompiler::FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, m_pLastNewIns->SetSVar(getParamArgIndex()); m_pLastNewIns->SetDVar(resultVar); } - else + else if (!onlyIfNeedsRuntimeLookup) { AddIns(INTOP_LDPTR); m_pLastNewIns->SetDVar(resultVar); @@ -2277,13 +2276,15 @@ int InterpCompiler::FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, return resultVar; } -void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool readonly, bool tailcall) +void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool readonly, bool tailcall, bool newObj) { uint32_t token = getU4LittleEndian(m_ip + 1); bool isVirtual = (*m_ip == CEE_CALLVIRT); CORINFO_RESOLVED_TOKEN resolvedCallToken; - ResolveToken(token, CORINFO_TOKENKIND_Method, &resolvedCallToken); + bool doCallInsteadOfNew = false; + + ResolveToken(token, newObj ? CORINFO_TOKENKIND_Method : CORINFO_TOKENKIND_NewObj, &resolvedCallToken); CORINFO_CALL_INFO callInfo; CORINFO_CALLINFO_FLAGS flags = (CORINFO_CALLINFO_FLAGS)(CORINFO_CALLINFO_ALLOWINSTPARAM | CORINFO_CALLINFO_SECURITYCHECKS | CORINFO_CALLINFO_DISALLOW_STUB); @@ -2298,9 +2299,17 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea return; } + if (callInfo.classFlags & CORINFO_FLG_VAROBJSIZE) + { + // This is a variable size object which means "System.String". + // For these, we just call the resolved method directly, but don't actually pass a this pointer to it. + doCallInsteadOfNew = true; + } + // Process sVars - int numArgsFromStack = callInfo.sig.numArgs + callInfo.sig.hasThis(); - int numArgs = numArgsFromStack; + int numArgsFromStack = callInfo.sig.numArgs + (newObj ? 0 : callInfo.sig.hasThis()); + int newObjThisArgLocation = newObj && !doCallInsteadOfNew ? 0 : INT_MAX; + int numArgs = numArgsFromStack + (newObjThisArgLocation == 0); m_pStackPointer -= numArgsFromStack; int extraParamArgLocation = INT_MAX; @@ -2318,13 +2327,51 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea // This is the extra type argument, which is not on the logical IL stack // Skip it for now. We will fill it in later. } + else if (iActualArg == newObjThisArgLocation) + { + // This is the newObj arg type argument, which is not on the logical IL stack + // Skip it for now. We will fill it in later. + } else { callArgs[iActualArg] = m_pStackPointer [iLogicalArg].var; + iLogicalArg++; } } callArgs[numArgs] = -1; + int32_t newObjTypeVar = -1; + int32_t newObjThisVar = -1; + int32_t newObjDVar = -1; + InterpType ctorType = InterpTypeO; + int32_t vtsize = 0; + + if (newObjThisArgLocation != INT_MAX) + { + ctorType = GetInterpType(m_compHnd->asCorInfoType(resolvedCallToken.hClass)); + if (ctorType == InterpTypeVT) + { + vtsize = m_compHnd->getClassSize(resolvedCallToken.hClass); + PushTypeVT(resolvedCallToken.hClass, vtsize); + PushInterpType(InterpTypeByRef, NULL); + } + else + { + PushInterpType(ctorType, resolvedCallToken.hClass); + PushInterpType(ctorType, resolvedCallToken.hClass); + + newObjTypeVar = FillTempVarWithToken(&resolvedCallToken, true/*embedParent*/, true /*onlyIfNeedsRuntimeLookup*/); + } + newObjDVar = m_pStackPointer[-2].var; + newObjThisVar = m_pStackPointer[-1].var; + m_pStackPointer--; + // Consider this arg as being defined, although newobj defines it + AddIns(INTOP_DEF); + m_pLastNewIns->SetDVar(newObjThisVar); + + callArgs[newObjThisArgLocation] = newObjThisVar; + } + if (extraParamArgLocation != INT_MAX) { PushStackType(StackTypeI, NULL); @@ -2383,7 +2430,16 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea // Process dVar int32_t dVar; - if (callInfo.sig.retType != CORINFO_TYPE_VOID) + if (newObjDVar != -1) + { + dVar = newObjDVar; + } + else if (doCallInsteadOfNew) + { + PushInterpType(InterpTypeO, NULL); + dVar = m_pStackPointer[-1].var; + } + else if (callInfo.sig.retType != CORINFO_TYPE_VOID) { InterpType interpType = GetInterpType(callInfo.sig.retType); @@ -2412,15 +2468,43 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea switch (callInfo.kind) { case CORINFO_CALL: - // Normal call - if (callInfo.nullInstanceCheck) + if (newObj && !doCallInsteadOfNew) { - // If the call is a normal call, we need to check for null instance - // before the call. - // TODO: Add null checking behavior somewhere here! + if (ctorType == InterpTypeVT) + { + // If this is a newobj for a value type, we need to call the constructor + // and then copy the value type to the stack. + AddIns(INTOP_NEWOBJ_VT); + m_pLastNewIns->data[1] = (int32_t)ALIGN_UP_TO(vtsize, INTERP_STACK_SLOT_SIZE); + } + else + { + if (newObjTypeVar != -1) + { + // newobj of type known only through a generic dictionary lookup. + assert(!"newobj of type known only through a generic dictionary lookup. NYI"); // This isn't implemented yet + } + else + { + // Normal newobj call + AddIns(INTOP_NEWOBJ); + m_pLastNewIns->data[1] = GetDataItemIndex(resolvedCallToken.hClass); + } + } + m_pLastNewIns->data[0] = GetDataItemIndex(callInfo.hMethod); + } + else + { + // Normal call + if (callInfo.nullInstanceCheck) + { + // If the call is a normal call, we need to check for null instance + // before the call. + // TODO: Add null checking behavior somewhere here! + } + AddIns(INTOP_CALL); + m_pLastNewIns->data[0] = GetMethodDataItemIndex(callInfo.hMethod); } - AddIns(INTOP_CALL); - m_pLastNewIns->data[0] = GetMethodDataItemIndex(callInfo.hMethod); break; case CORINFO_CALL_CODE_POINTER: @@ -3617,13 +3701,19 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) break; case CEE_CALLVIRT: case CEE_CALL: - EmitCall(constrainedClass, readonly, tailcall); + EmitCall(constrainedClass, readonly, tailcall, false /*newObj*/); constrainedClass = NULL; readonly = false; tailcall = false; break; case CEE_NEWOBJ: { + EmitCall(NULL /*constrainedClass*/, false /* readonly*/, false /* tailcall*/, true /*newObj*/); + constrainedClass = NULL; + readonly = false; + tailcall = false; + break; + /* CORINFO_METHOD_HANDLE ctorMethod; CORINFO_SIG_INFO ctorSignature; CORINFO_CLASS_HANDLE ctorClass; @@ -3685,7 +3775,7 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) // Pop this, the result of the newobj still remains on the stack m_pStackPointer--; - break; + break;*/ } case CEE_DUP: { @@ -4417,7 +4507,7 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) if (embedInfo.lookup.lookupKind.needsRuntimeLookup) { - int runtimeLookupVar = FillTempVarWithToken(&resolvedToken, false); + int runtimeLookupVar = FillTempVarWithToken(&resolvedToken, false, true); m_pLastNewIns->SetSVar(getParamArgIndex()); m_pLastNewIns->SetDVar(runtimeLookupVar); @@ -4748,7 +4838,7 @@ void InterpCompiler::PrintInsData(InterpInst *ins, int32_t insOffset, const int3 } case InterpOpMethodHandle: { - CORINFO_METHOD_HANDLE mh = (CORINFO_METHOD_HANDLE)((size_t)m_dataItems.Get(*pData) & ~INTERP_METHOD_HANDLE_TAG); + CORINFO_METHOD_HANDLE mh = (CORINFO_METHOD_HANDLE)((size_t)m_dataItems.Get(*pData)); printf(" "); PrintMethodName(mh); break; diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 358f6b0cf49b21..bd28854ed5d1b1 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -356,7 +356,7 @@ class InterpCompiler CORINFO_CLASS_HANDLE ResolveClassToken(uint32_t token); CORINFO_CLASS_HANDLE getClassFromContext(CORINFO_CONTEXT_HANDLE context); int getParamArgIndex(); // Get the index into the m_pVars array of the Parameter argument. This is either the this pointer, a methoddesc or a class handle - int FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, bool embedParent, int existingVar = -1); + int FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, bool embedParent, bool onlyIfNeedsRuntimeLookup, int existingVar = -1); void* AllocMethodData(size_t numBytes); public: @@ -453,7 +453,7 @@ class InterpCompiler void EmitUnaryArithmeticOp(int32_t opBase); void EmitShiftOp(int32_t opBase); void EmitCompareOp(int32_t opBase); - void EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool readonly, bool tailcall); + void EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool readonly, bool tailcall, bool newObj); bool EmitCallIntrinsics(CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO sig); void EmitLdind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset); void EmitStind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset, bool reverseSVarOrder); diff --git a/src/coreclr/interpreter/interpretershared.h b/src/coreclr/interpreter/interpretershared.h index e73f8fbba8aa37..82a51c7221c1ce 100644 --- a/src/coreclr/interpreter/interpretershared.h +++ b/src/coreclr/interpreter/interpretershared.h @@ -17,7 +17,6 @@ #define INTERP_STACK_SLOT_SIZE 8 // Alignment of each var offset on the interpreter stack #define INTERP_STACK_ALIGNMENT 16 // Alignment of interpreter stack at the start of a frame -#define INTERP_METHOD_HANDLE_TAG 4 // Tag of a MethodDesc in the interp method dataItems #define INTERP_INDIRECT_HELPER_TAG 1 // When a helper ftn's address is indirect we tag it with this tag bit struct InterpMethod @@ -51,4 +50,17 @@ struct InterpMethod } }; +struct InterpByteCodeStart +{ +#ifndef DPTR + InterpMethod* const InterpMethod; // Pointer to the InterpMethod structure +#else + DPTR(InterpMethod) const InterpMethod; // Pointer to the InterpMethod structure +#endif + const int* GetByteCodes() const + { + return reinterpret_cast(this + 1); + } +}; + #endif diff --git a/src/coreclr/vm/amd64/asmconstants.h b/src/coreclr/vm/amd64/asmconstants.h index 9f4e3543dfaad6..2b9b588d30af90 100644 --- a/src/coreclr/vm/amd64/asmconstants.h +++ b/src/coreclr/vm/amd64/asmconstants.h @@ -158,7 +158,7 @@ ASMCONSTANTS_C_ASSERT(OFFSETOF__GenericDictionaryDynamicHelperStubData__SlotOffs ASMCONSTANTS_C_ASSERT(OFFSETOF__GenericDictionaryDynamicHelperStubData__HandleArgs == offsetof(GenericDictionaryDynamicHelperStubData, HandleArgs)); -#define OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo DBG_FRE(0x40, 0x18) +#define OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo DBG_FRE(0x48, 0x20) ASMCONSTANTS_C_ASSERT(OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo == offsetof(InstantiatedMethodDesc, m_pPerInstInfo)); diff --git a/src/coreclr/vm/callstubgenerator.cpp b/src/coreclr/vm/callstubgenerator.cpp index a8e65fab3ee5b0..bdb402b8b99fe0 100644 --- a/src/coreclr/vm/callstubgenerator.cpp +++ b/src/coreclr/vm/callstubgenerator.cpp @@ -4,6 +4,7 @@ #ifdef FEATURE_INTERPRETER #include "callstubgenerator.h" +#include "ecall.h" extern "C" void Load_Stack(); @@ -545,6 +546,15 @@ CallStubHeader *CallStubGenerator::GenerateCallStub(MethodDesc *pMD, AllocMemTra { STANDARD_VM_CONTRACT; + // String constructors are special cases, and have a special calling convention that is associated with the actual function executed (which is a static function with no this parameter) + if (pMD->GetMethodTable()->IsString() && pMD->IsCtor()) + { + _ASSERTE(pMD->IsFCall()); + MethodDesc *pMDActualImplementation = NonVirtualEntry2MethodDesc(ECall::GetFCallImpl(pMD)); + _ASSERTE(pMDActualImplementation != pMD); + pMD = pMDActualImplementation; + } + _ASSERTE(pMD != NULL); MetaSig sig(pMD); diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 3bfc227a9d1879..43833a3be03a98 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -41,7 +41,7 @@ void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet) } } - pHeader->SetTarget(pMD->GetNativeCode()); // The method to call + pHeader->SetTarget(pMD->GetMultiCallableAddrOfCode()); // The method to call pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); } @@ -104,7 +104,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr const int32_t *ip; int8_t *stack; - InterpMethod *pMethod = *(InterpMethod**)pFrame->startIp; + InterpMethod *pMethod = pFrame->startIp->InterpMethod; assert(pMethod->CheckIntegrity()); pThreadContext->pStackPointer = pFrame->pStack + pMethod->allocaSize; @@ -113,7 +113,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr if (pExceptionClauseArgs == NULL) { // Start executing at the beginning of the method - ip = pFrame->startIp + sizeof(InterpMethod*) / sizeof(int32_t); + ip = pFrame->startIp->GetByteCodes(); } else { @@ -135,7 +135,8 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr } int32_t returnOffset, callArgsOffset, methodSlot; - const int32_t *targetIp; + PTR_InterpByteCodeStart targetIp; + MethodDesc* targetMethod; MAIN_LOOP: try @@ -1143,20 +1144,9 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // Interpreter-TODO // This needs to be optimized, not operating at MethodDesc level, rather with ftnptr // slots containing the interpreter IR pointer - pMD = pMD->GetMethodDescOfVirtualizedCode(pThisArg, pMD->GetMethodTable()); - - PCODE code = pMD->GetNativeCode(); - if (!code) - { - pInterpreterFrame->SetTopInterpMethodContextFrame(pFrame); - GCX_PREEMP(); - pMD->PrepareInitialCode(CallerGCMode::Coop); - code = pMD->GetNativeCode(); - } - targetIp = (const int32_t*)code; + targetMethod = pMD->GetMethodDescOfVirtualizedCode(pThisArg, pMD->GetMethodTable()); ip += 4; - // Interpreter-TODO unbox if target method class is valuetype - goto CALL_TARGET_IP; + goto CALL_INTERP_METHOD; } case INTOP_CALL: @@ -1167,15 +1157,12 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr ip += 4; CALL_INTERP_SLOT: + targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; +CALL_INTERP_METHOD: + targetIp = targetMethod->GetInterpreterCode(); + if (targetIp == NULL) { - size_t targetMethod = (size_t)pMethod->pDataItems[methodSlot]; - if (targetMethod & INTERP_METHOD_HANDLE_TAG) - { - // First execution of this call. Ensure target method is compiled and - // patch the data item slot with the actual method code. - MethodDesc *pMD = (MethodDesc*)(targetMethod & ~INTERP_METHOD_HANDLE_TAG); - PCODE code = pMD->GetNativeCode(); - if (!code) { + { // This is an optimization to ensure that the stack walk will not have to search // for the topmost frame in the current InterpExecMethod. It is not required // for correctness, as the stack walk will find the topmost frame anyway. But it @@ -1185,30 +1172,19 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr // small subset of frames high. pInterpreterFrame->SetTopInterpMethodContextFrame(pFrame); GCX_PREEMP(); - pMD->PrepareInitialCode(CallerGCMode::Coop); - code = pMD->GetNativeCode(); + // Attempt to setup the interpreter code for the target method. + if (!(targetMethod->IsFCall() || (targetMethod->IsCtor() && targetMethod->GetMethodTable()->IsString()))) + { + targetMethod->PrepareInitialCode(CallerGCMode::Coop); + } + targetIp = targetMethod->GetInterpreterCode(); + } + if (targetIp == NULL) + { + // If we didn't get the interpreter code pointer setup, then this is a method we need to invoke as a compiled method. + InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset); + break; } - pMethod->pDataItems[methodSlot] = (void*)code; - targetIp = (const int32_t*)code; - } - else - { - targetIp = (const int32_t*)targetMethod; - } - } -CALL_TARGET_IP: - // Interpreter-TODO: we need a fast way to check of the targetIp is an interpreter code or not. - // Probably use a tagged pointer for interpreter code and a normal pointer for JIT/R2R code. - EECodeInfo codeInfo((PCODE)targetIp); - if (!codeInfo.IsValid()) - { - EEPOLICY_HANDLE_FATAL_ERROR_WITH_MESSAGE(COR_E_EXECUTIONENGINE, W("Attempted to execute native code from interpreter")); - } - else if (codeInfo.GetCodeManager() != ExecutionManager::GetInterpreterCodeManager()) - { - MethodDesc *pMD = codeInfo.GetMethodDesc(); - InvokeCompiledMethod(pMD, stack + callArgsOffset, stack + returnOffset); - break; } // Save current execution state for when we return from called method @@ -1229,10 +1205,10 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr assert (((size_t)pFrame->pStack % INTERP_STACK_ALIGNMENT) == 0); // Set execution state for the new frame - pMethod = *(InterpMethod**)pFrame->startIp; + pMethod = pFrame->startIp->InterpMethod; assert(pMethod->CheckIntegrity()); stack = pFrame->pStack; - ip = pFrame->startIp + sizeof(InterpMethod*) / sizeof(int32_t); + ip = pFrame->startIp->GetByteCodes(); pThreadContext->pStackPointer = stack + pMethod->allocaSize; break; } @@ -1653,7 +1629,7 @@ do { \ ip = (int32_t*)resumeIP; stack = pFrame->pStack; - pMethod = *(InterpMethod**)pFrame->startIp; + pMethod = pFrame->startIp->InterpMethod; assert(pMethod->CheckIntegrity()); pThreadContext->pStackPointer = pFrame->pStack + pMethod->allocaSize; goto MAIN_LOOP; @@ -1670,7 +1646,7 @@ do { \ pFrame = pFrame->pParent; ip = pFrame->ip; stack = pFrame->pStack; - pMethod = *(InterpMethod**)pFrame->startIp; + pMethod = pFrame->startIp->InterpMethod; assert(pMethod->CheckIntegrity()); pFrame->ip = NULL; diff --git a/src/coreclr/vm/interpexec.h b/src/coreclr/vm/interpexec.h index f79b8cf39c4128..3e43692b2ce890 100644 --- a/src/coreclr/vm/interpexec.h +++ b/src/coreclr/vm/interpexec.h @@ -27,14 +27,14 @@ struct StackVal struct InterpMethodContextFrame { PTR_InterpMethodContextFrame pParent; - const int32_t *startIp; // from startIp we can obtain InterpMethod and MethodDesc + PTR_InterpByteCodeStart startIp; // from startIp we can obtain InterpMethod and MethodDesc int8_t *pStack; int8_t *pRetVal; const int32_t *ip; // This ip is updated only when execution can leave the frame PTR_InterpMethodContextFrame pNext; #ifndef DACCESS_COMPILE - void ReInit(InterpMethodContextFrame *pParent, const int32_t *startIp, int8_t *pRetVal, int8_t *pStack) + void ReInit(InterpMethodContextFrame *pParent, PTR_InterpByteCodeStart startIp, int8_t *pRetVal, int8_t *pStack) { this->pParent = pParent; this->startIp = startIp; diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 93af9e082a60e1..3f1044fb8e1f70 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -37,12 +37,16 @@ class GCCoverageInfo; class DynamicMethodDesc; class ReJitManager; class PrepareCodeConfig; +struct InterpMethod; +struct InterpByteCodeStart; typedef DPTR(FCallMethodDesc) PTR_FCallMethodDesc; typedef DPTR(ArrayMethodDesc) PTR_ArrayMethodDesc; typedef DPTR(DynamicMethodDesc) PTR_DynamicMethodDesc; typedef DPTR(InstantiatedMethodDesc) PTR_InstantiatedMethodDesc; typedef DPTR(GCCoverageInfo) PTR_GCCoverageInfo; // see code:GCCoverageInfo::savedCode +typedef DPTR(InterpMethod) PTR_InterpMethod; +typedef DPTR(InterpByteCodeStart) PTR_InterpByteCodeStart; #ifdef FEATURE_MINIMETADATA_IN_TRIAGEDUMPS GVAL_DECL(DWORD, g_MiniMetaDataBuffMaxSize); @@ -1800,6 +1804,20 @@ class MethodDesc WORD m_wSlotNumber; // The slot number of this MethodDesc in the vtable array. WORD m_wFlags; // See MethodDescFlags PTR_MethodDescCodeData m_codeData; +#ifdef FEATURE_INTERPRETER + PTR_InterpByteCodeStart m_interpreterCode; +public: + const PTR_InterpByteCodeStart GetInterpreterCode() const + { + LIMITED_METHOD_DAC_CONTRACT; + return m_interpreterCode; + } + void SetInterpreterCode(PTR_InterpByteCodeStart interpreterCode) + { + LIMITED_METHOD_CONTRACT; + VolatileStore(&m_interpreterCode, interpreterCode); + } +#endif // FEATURE_INTERPRETER #ifdef _DEBUG public: diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 2f5c506ccb6000..815c150089c1f0 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -437,6 +437,7 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) amt.SuppressRelease(); pCode = PINSTRToPCODE(pPrecode->GetEntryPoint()); SetNativeCodeInterlocked(pCode); + SetInterpreterCode(dac_cast(pPrecode->GetData()->ByteCodeAddr)); } #endif // FEATURE_INTERPRETER } @@ -2018,7 +2019,7 @@ extern "C" void STDCALL ExecuteInterpretedMethod(TransitionBlock* pTransitionBlo StackVal retVal; - frames.interpMethodContextFrame.startIp = (int32_t*)byteCodeAddr; + frames.interpMethodContextFrame.startIp = dac_cast(byteCodeAddr); frames.interpMethodContextFrame.pStack = sp; frames.interpMethodContextFrame.pRetVal = (int8_t*)&retVal; From b5f3cbca7de31ffc09bd675288844181f0cda3c0 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 5 Jun 2025 17:01:40 -0700 Subject: [PATCH 3/7] Pass all the tests, and add some more --- src/coreclr/interpreter/compiler.cpp | 84 +++-------------- src/coreclr/interpreter/intops.def | 1 + src/coreclr/vm/interpexec.cpp | 16 ++++ src/tests/JIT/interpreter/Interpreter.cs | 113 +++++++++++++++++++++++ 4 files changed, 145 insertions(+), 69 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index efc4c8529ff916..b212073fbe29e7 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2403,7 +2403,7 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea } else { - FillTempVarWithToken(&resolvedCallToken, false, contextParamVar); + FillTempVarWithToken(&resolvedCallToken, false, true, contextParamVar); } } @@ -2430,7 +2430,7 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea } else { - FillTempVarWithToken(&resolvedCallToken, true, contextParamVar); + FillTempVarWithToken(&resolvedCallToken, true, true, contextParamVar); } } } @@ -2489,7 +2489,8 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea if (newObjTypeVar != -1) { // newobj of type known only through a generic dictionary lookup. - assert(!"newobj of type known only through a generic dictionary lookup. NYI"); // This isn't implemented yet + AddIns(INTOP_NEWOBJ_VAR); + m_pLastNewIns->SetSVars2(CALL_ARGS_SVAR, newObjTypeVar); } else { @@ -2525,14 +2526,22 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea break; case CORINFO_VIRTUALCALL_VTABLE: - // Traditional virtual call. + // Traditional virtual call. In theory we could optimize this to using the vtable AddIns(INTOP_CALLVIRT); m_pLastNewIns->data[0] = GetDataItemIndex(callInfo.hMethod); break; case CORINFO_VIRTUALCALL_LDVIRTFTN: - // Resolve a virtual call using the helper function to a function pointer, and then call through that - assert("!Need to support ldvirtftn"); + if (callInfo.exactContextNeedsRuntimeLookup) + { + // Resolve a virtual call using the helper function to a function pointer, and then call through that + assert(!"Need to support ldvirtftn path"); + } + else + { + AddIns(INTOP_CALLVIRT); + m_pLastNewIns->data[0] = GetDataItemIndex(callInfo.hMethod); + } break; case CORINFO_VIRTUALCALL_STUB: @@ -3731,69 +3740,6 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) readonly = false; tailcall = false; break; - /* - CORINFO_METHOD_HANDLE ctorMethod; - CORINFO_SIG_INFO ctorSignature; - CORINFO_CLASS_HANDLE ctorClass; - m_ip++; - ctorMethod = ResolveMethodToken(getU4LittleEndian(m_ip)); - m_ip += 4; - - m_compHnd->getMethodSig(ctorMethod, &ctorSignature); - ctorClass = m_compHnd->getMethodClass(ctorMethod); - int32_t numArgs = ctorSignature.numArgs; - - // TODO Special case array ctor / string ctor - m_pStackPointer -= numArgs; - - // Allocate callArgs for the call, this + numArgs + terminator - int32_t *callArgs = (int32_t*) AllocMemPool((numArgs + 2) * sizeof(int32_t)); - for (int i = 0; i < numArgs; i++) - callArgs[i + 1] = m_pStackPointer[i].var; - callArgs[numArgs + 1] = -1; - - // Push the return value and `this` argument to the ctor - InterpType retType = GetInterpType(m_compHnd->asCorInfoType(ctorClass)); - int32_t vtsize = 0; - if (retType == InterpTypeVT) - { - vtsize = m_compHnd->getClassSize(ctorClass); - PushTypeVT(ctorClass, vtsize); - PushInterpType(InterpTypeByRef, NULL); - } - else - { - PushInterpType(retType, ctorClass); - PushInterpType(retType, ctorClass); - } - int32_t dVar = m_pStackPointer[-2].var; - int32_t thisVar = m_pStackPointer[-1].var; - // Consider this arg as being defined, although newobj defines it - AddIns(INTOP_DEF); - m_pLastNewIns->SetDVar(thisVar); - callArgs[0] = thisVar; - - if (retType == InterpTypeVT) - { - AddIns(INTOP_NEWOBJ_VT); - m_pLastNewIns->data[1] = (int32_t)ALIGN_UP_TO(vtsize, INTERP_STACK_SLOT_SIZE); - } - else - { - AddIns(INTOP_NEWOBJ); - m_pLastNewIns->data[1] = GetDataItemIndex(ctorClass); - } - m_pLastNewIns->data[0] = GetMethodDataItemIndex(ctorMethod); - m_pLastNewIns->SetSVar(CALL_ARGS_SVAR); - m_pLastNewIns->SetDVar(dVar); - - m_pLastNewIns->flags |= INTERP_INST_FLAG_CALL; - m_pLastNewIns->info.pCallInfo = (InterpCallInfo*)AllocMemPool0(sizeof(InterpCallInfo)); - m_pLastNewIns->info.pCallInfo->pCallArgs = callArgs; - - // Pop this, the result of the newobj still remains on the stack - m_pStackPointer--; - break;*/ } case CEE_DUP: { diff --git a/src/coreclr/interpreter/intops.def b/src/coreclr/interpreter/intops.def index 80e5121c7bc0c0..160ca3944d22ed 100644 --- a/src/coreclr/interpreter/intops.def +++ b/src/coreclr/interpreter/intops.def @@ -281,6 +281,7 @@ OPDEF(INTOP_LDFLDA, "ldflda", 4, 1, 1, InterpOpInt) OPDEF(INTOP_CALL, "call", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALLVIRT, "callvirt", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_NEWOBJ, "newobj", 5, 1, 1, InterpOpMethodHandle) +OPDEF(INTOP_NEWOBJ_VAR, "newobj.var", 5, 1, 2, InterpOpMethodHandle) OPDEF(INTOP_NEWOBJ_VT, "newobj.vt", 5, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALL_HELPER_PP, "call.helper.pp", 4, 1, 0, InterpOpTwoInts) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index f4f167def6c61e..d5b090176b999f 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -1226,6 +1226,22 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr pThreadContext->pStackPointer = stack + pMethod->allocaSize; break; } + case INTOP_NEWOBJ_VAR: + { + returnOffset = ip[1]; + callArgsOffset = ip[2]; + methodSlot = ip[4]; + + OBJECTREF objRef = AllocateObject(LOCAL_VAR(ip[3], MethodTable*)); + + // This is return value + LOCAL_VAR(returnOffset, OBJECTREF) = objRef; + // Set `this` arg for ctor call + LOCAL_VAR (callArgsOffset, OBJECTREF) = objRef; + ip += 5; + + goto CALL_INTERP_SLOT; + } case INTOP_NEWOBJ: { returnOffset = ip[1]; diff --git a/src/tests/JIT/interpreter/Interpreter.cs b/src/tests/JIT/interpreter/Interpreter.cs index a9461d001f08b4..f1d7baf90a6693 100644 --- a/src/tests/JIT/interpreter/Interpreter.cs +++ b/src/tests/JIT/interpreter/Interpreter.cs @@ -383,54 +383,81 @@ public static void RunInterpreterTests() TestCallingConvention12(1, 2, 3, 4, 5, 6, 7, 8, 9, 'a', 10, 11, 12); // Console.WriteLine("Run interp tests"); + Console.WriteLine("Sum"); if (SumN(50) != 1275) Environment.FailFast(null); + Console.WriteLine("Mul4"); if (Mul4(53, 24, 13, 131) != 2166216) Environment.FailFast(null); + Console.WriteLine("TestSwitch"); TestSwitch(); + Console.WriteLine("PowLoop"); if (!PowLoop(20, 10, 1661992960)) Environment.FailFast(null); + Console.WriteLine("TestJitFields"); if (!TestJitFields()) Environment.FailFast(null); + Console.WriteLine("TestFields"); if (!TestFields()) Environment.FailFast(null); + Console.WriteLine("TestStructRefFields"); if (!TestStructRefFields()) Environment.FailFast(null); + Console.WriteLine("TestSpecialFields"); if (!TestSpecialFields()) Environment.FailFast(null); + Console.WriteLine("TestFloat"); if (!TestFloat()) Environment.FailFast(null); + Console.WriteLine("TestLocalloc"); if (!TestLocalloc()) Environment.FailFast(null); + Console.WriteLine("TestVirtual"); if (!TestVirtual()) Environment.FailFast(null); + Console.WriteLine("TestBoxing"); if (!TestBoxing()) Environment.FailFast(null); + Console.WriteLine("TestArray"); if (!TestArray()) Environment.FailFast(null); + Console.WriteLine("TestXxObj"); if (!TestXxObj()) Environment.FailFast(null); + Console.WriteLine("TestSizeof"); if (!TestSizeof()) Environment.FailFast(null); + Console.WriteLine("TestLdtoken"); if (!TestLdtoken()) Environment.FailFast(null); /* if (!TestMdArray()) Environment.FailFast(null); */ + Console.WriteLine("TestExceptionHandling"); TestExceptionHandling(); + Console.WriteLine("TestStringCtor"); + if (!TestStringCtor()) + Environment.FailFast(null); + + Console.WriteLine("TestSharedGenerics"); + if (!TestSharedGenerics()) + Environment.FailFast(null); + System.GC.Collect(); + + Console.WriteLine("All tests passed successfully!"); } public static void TestExceptionHandling() @@ -1251,23 +1278,109 @@ public static bool TestVirtual() BaseClass bc = new DerivedClass(); ITest itest = bc; + Console.WriteLine("bc.NonVirtualMethod"); if (bc.NonVirtualMethod() != 0xbaba) return false; + Console.WriteLine("bc.VirtualMethod"); if (bc.VirtualMethod() != 0xdede) return false; + Console.WriteLine("itest.VirtualMethod"); if (itest.VirtualMethod() != 0xdede) return false; bc = new BaseClass(); itest = bc; + Console.WriteLine("bc.NonVirtualMethod"); if (bc.NonVirtualMethod() != 0xbaba) return false; + Console.WriteLine("bc.VirtualMethod"); if (bc.VirtualMethod() != 0xbebe) return false; + Console.WriteLine("itest.VirtualMethod"); if (itest.VirtualMethod() != 0xbebe) return false; return true; } + public static bool TestStringCtor() + { + string s = new string('a', 4); + if (s.Length != 4) + return false; + if (s[0] != 'a') + return false; + if (s != "aaaa") + return false; + return true; + } + + private static Type LoadType() + { + return typeof(T); + } + + class GenericClass + { + public Type GetTypeOfTInstance() + { + return typeof(T); + } + public static Type GetTypeOfTStatic() + { + return typeof(T); + } + } + + public static bool TestSharedGenerics() + { + if (!TestSharedGenerics_CallsTo()) + return false; + + Console.WriteLine("Test calls to shared generics from generic code (unshared generics)"); + if (!TestGenerics_CallsFrom()) + return false; + Console.WriteLine("Test calls to shared generics from generic code (shared generics)"); + if (!TestGenerics_CallsFrom()) + return false; + + return true; + } + + public static bool TestSharedGenerics_CallsTo() + { + Console.WriteLine("Test calls to shared generics from non-generic code"); + if (LoadType() != typeof(string)) + return false; + if (LoadType() != typeof(object)) + return false; + + if (new GenericClass().GetTypeOfTInstance() != typeof(string)) + return false; + if (new GenericClass().GetTypeOfTInstance() != typeof(object)) + return false; + + if (GenericClass.GetTypeOfTStatic() != typeof(object)) + return false; + + if (GenericClass.GetTypeOfTStatic() != typeof(string)) + return false; + + return true; + } + + public static bool TestGenerics_CallsFrom() + { + if (LoadType() != typeof(T)) + return false; + + if (new GenericClass().GetTypeOfTInstance() != typeof(T)) + return false; + + if (GenericClass.GetTypeOfTStatic() != typeof(T)) + return false; + + return true; + } + public static bool TestBoxing() { int l = 7, r = 4; From 3016e5944f8aad55ef0009fc484213aade18e2e0 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 6 Jun 2025 09:53:07 -0700 Subject: [PATCH 4/7] Fix build breaks due to extra field in MethodDesc --- src/coreclr/vm/amd64/asmconstants.h | 4 ++++ src/coreclr/vm/arm64/asmconstants.h | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/coreclr/vm/amd64/asmconstants.h b/src/coreclr/vm/amd64/asmconstants.h index 9b46af243c6345..9c935d0fa74e0f 100644 --- a/src/coreclr/vm/amd64/asmconstants.h +++ b/src/coreclr/vm/amd64/asmconstants.h @@ -160,7 +160,11 @@ ASMCONSTANTS_C_ASSERT(OFFSETOF__GenericDictionaryDynamicHelperStubData__SlotOffs ASMCONSTANTS_C_ASSERT(OFFSETOF__GenericDictionaryDynamicHelperStubData__HandleArgs == offsetof(GenericDictionaryDynamicHelperStubData, HandleArgs)); +#ifdef FEATURE_INTERPRETER #define OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo DBG_FRE(0x48, 0x20) +#else +#define OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo DBG_FRE(0x40, 0x18) +#endif // FEATURE_INTERPRETER ASMCONSTANTS_C_ASSERT(OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo == offsetof(InstantiatedMethodDesc, m_pPerInstInfo)); diff --git a/src/coreclr/vm/arm64/asmconstants.h b/src/coreclr/vm/arm64/asmconstants.h index fb262a617e8bba..3178ad1e297b1b 100644 --- a/src/coreclr/vm/arm64/asmconstants.h +++ b/src/coreclr/vm/arm64/asmconstants.h @@ -133,7 +133,11 @@ ASMCONSTANTS_C_ASSERT(OFFSETOF__GenericDictionaryDynamicHelperStubData__SlotOffs ASMCONSTANTS_C_ASSERT(OFFSETOF__GenericDictionaryDynamicHelperStubData__HandleArgs == offsetof(GenericDictionaryDynamicHelperStubData, HandleArgs)); +#ifdef FEATURE_INTERPRETER +#define OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo DBG_FRE(0x48, 0x20) +#else #define OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo DBG_FRE(0x40, 0x18) +#endif // FEATURE_INTERPRETER ASMCONSTANTS_C_ASSERT(OFFSETOF__InstantiatedMethodDesc__m_pPerInstInfo == offsetof(InstantiatedMethodDesc, m_pPerInstInfo)); From 21b0b4732853fe48a5872e81dc7293471de32d35 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 6 Jun 2025 11:31:14 -0700 Subject: [PATCH 5/7] Fix factoring and naming of generic lookups --- src/coreclr/interpreter/compiler.cpp | 83 ++++++++++++++++------------ src/coreclr/interpreter/compiler.h | 21 ++++++- 2 files changed, 67 insertions(+), 37 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index b212073fbe29e7..32621a5b51f603 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2237,18 +2237,28 @@ int InterpCompiler::getParamArgIndex() return m_paramArgIndex; } -int InterpCompiler::FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, bool embedParent, bool onlyIfNeedsRuntimeLookup, int existingTemp) +InterpCompiler::InterpEmbedGenericResult InterpCompiler::EmitGenericHandle(CORINFO_RESOLVED_TOKEN* resolvedToken, GenericHandleEmbedOptions options) { CORINFO_GENERICHANDLE_RESULT embedInfo; - m_compHnd->embedGenericHandle(resolvedToken, embedParent, m_methodInfo->ftn, &embedInfo); - - int resultVar = existingTemp; - if ((resultVar == -1) && (!onlyIfNeedsRuntimeLookup || (embedInfo.lookup.lookupKind.needsRuntimeLookup))) + InterpEmbedGenericResult result; + m_compHnd->embedGenericHandle(resolvedToken, HasFlag(options, GenericHandleEmbedOptions::EmbedParent), m_methodInfo->ftn, &embedInfo); + if (HasFlag(options, GenericHandleEmbedOptions::VarOnly) || embedInfo.lookup.lookupKind.needsRuntimeLookup) { - PushStackType(StackTypeI, NULL); - resultVar = m_pStackPointer[-1].var; - m_pStackPointer--; + result.var = EmitGenericHandleAsVar(embedInfo); } + else + { + assert(embedInfo.lookup.constLookup.accessType == IAT_VALUE); + result.dataItemIndex = GetDataItemIndex(embedInfo.lookup.constLookup.handle); + } + return result; +} + +int InterpCompiler::EmitGenericHandleAsVar(const CORINFO_GENERICHANDLE_RESULT &embedInfo) +{ + PushStackType(StackTypeI, NULL); + int resultVar = m_pStackPointer[-1].var; + m_pStackPointer--; if (embedInfo.lookup.lookupKind.needsRuntimeLookup) { @@ -2272,7 +2282,7 @@ int InterpCompiler::FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, m_pLastNewIns->SetSVar(getParamArgIndex()); m_pLastNewIns->SetDVar(resultVar); } - else if (!onlyIfNeedsRuntimeLookup) + else { AddIns(INTOP_LDPTR); m_pLastNewIns->SetDVar(resultVar); @@ -2347,7 +2357,7 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea } callArgs[numArgs] = -1; - int32_t newObjTypeVar = -1; + InterpEmbedGenericResult newObjType; int32_t newObjThisVar = -1; int32_t newObjDVar = -1; InterpType ctorType = InterpTypeO; @@ -2367,7 +2377,7 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea PushInterpType(ctorType, resolvedCallToken.hClass); PushInterpType(ctorType, resolvedCallToken.hClass); - newObjTypeVar = FillTempVarWithToken(&resolvedCallToken, true/*embedParent*/, true /*onlyIfNeedsRuntimeLookup*/); + newObjType = EmitGenericHandle(&resolvedCallToken, GenericHandleEmbedOptions::EmbedParent); } newObjDVar = m_pStackPointer[-2].var; newObjThisVar = m_pStackPointer[-1].var; @@ -2381,41 +2391,44 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea if (extraParamArgLocation != INT_MAX) { - PushStackType(StackTypeI, NULL); - m_pStackPointer--; - int contextParamVar = m_pStackPointer[0].var; - callArgs[extraParamArgLocation] = contextParamVar; - + int contextParamVar = -1; + // Instantiated generic method CORINFO_CONTEXT_HANDLE exactContextHnd = callInfo.contextHandle; if (((SIZE_T)exactContextHnd & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD) { assert(exactContextHnd != METHOD_BEING_COMPILED_CONTEXT()); - + CORINFO_METHOD_HANDLE exactMethodHandle = - (CORINFO_METHOD_HANDLE)((SIZE_T)exactContextHnd & ~CORINFO_CONTEXTFLAGS_MASK); - + (CORINFO_METHOD_HANDLE)((SIZE_T)exactContextHnd & ~CORINFO_CONTEXTFLAGS_MASK); + if (!callInfo.exactContextNeedsRuntimeLookup) { + PushStackType(StackTypeI, NULL); + m_pStackPointer--; + contextParamVar = m_pStackPointer[0].var; AddIns(INTOP_LDPTR); m_pLastNewIns->SetDVar(contextParamVar); m_pLastNewIns->data[0] = GetDataItemIndex((void*)exactMethodHandle); } else { - FillTempVarWithToken(&resolvedCallToken, false, true, contextParamVar); + contextParamVar = EmitGenericHandle(&resolvedCallToken, GenericHandleEmbedOptions::VarOnly).var; } } - + // otherwise must be an instance method in a generic struct, // a static method in a generic type, or a runtime-generated array method else { assert(((SIZE_T)exactContextHnd & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS); CORINFO_CLASS_HANDLE exactClassHandle = getClassFromContext(exactContextHnd); - + if ((callInfo.classFlags & CORINFO_FLG_ARRAY) && readonly) { + PushStackType(StackTypeI, NULL); + m_pStackPointer--; + contextParamVar = m_pStackPointer[0].var; // We indicate "readonly" to the Address operation by using a null // instParam. AddIns(INTOP_LDPTR); @@ -2424,15 +2437,19 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea } else if (!callInfo.exactContextNeedsRuntimeLookup) { + PushStackType(StackTypeI, NULL); + m_pStackPointer--; + contextParamVar = m_pStackPointer[0].var; AddIns(INTOP_LDPTR); m_pLastNewIns->SetDVar(contextParamVar); m_pLastNewIns->data[0] = GetDataItemIndex((void*)exactClassHandle); } else { - FillTempVarWithToken(&resolvedCallToken, true, true, contextParamVar); + contextParamVar = EmitGenericHandle(&resolvedCallToken, GenericHandleEmbedOptions::VarOnly | GenericHandleEmbedOptions::EmbedParent).var; } } + callArgs[extraParamArgLocation] = contextParamVar; } // Process dVar @@ -2486,17 +2503,17 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea } else { - if (newObjTypeVar != -1) + if (newObjType.var != -1) { // newobj of type known only through a generic dictionary lookup. AddIns(INTOP_NEWOBJ_VAR); - m_pLastNewIns->SetSVars2(CALL_ARGS_SVAR, newObjTypeVar); + m_pLastNewIns->SetSVars2(CALL_ARGS_SVAR, newObjType.var); } else { // Normal newobj call AddIns(INTOP_NEWOBJ); - m_pLastNewIns->data[1] = GetDataItemIndex(resolvedCallToken.hClass); + m_pLastNewIns->data[1] = newObjType.dataItemIndex; } } m_pLastNewIns->data[0] = GetDataItemIndex(callInfo.hMethod); @@ -4466,23 +4483,17 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) CORINFO_RESOLVED_TOKEN resolvedToken; ResolveToken(getU4LittleEndian(m_ip + 1), CORINFO_TOKENKIND_Ldtoken, &resolvedToken); - CORINFO_GENERICHANDLE_RESULT embedInfo; - m_compHnd->embedGenericHandle(&resolvedToken, false, m_methodInfo->ftn, &embedInfo); + InterpEmbedGenericResult resolvedEmbedResult = EmitGenericHandle(&resolvedToken, GenericHandleEmbedOptions::None); - if (embedInfo.lookup.lookupKind.needsRuntimeLookup) + if (resolvedEmbedResult.var != -1) { - int runtimeLookupVar = FillTempVarWithToken(&resolvedToken, false, true); - m_pLastNewIns->SetSVar(getParamArgIndex()); - m_pLastNewIns->SetDVar(runtimeLookupVar); - AddIns(INTOP_LDTOKEN_VAR); - m_pLastNewIns->SetSVar(runtimeLookupVar); + m_pLastNewIns->SetSVar(resolvedEmbedResult.var); } else { AddIns(INTOP_LDTOKEN); - assert(embedInfo.lookup.constLookup.accessType == IAT_VALUE); - m_pLastNewIns->data[1] = GetDataItemIndex(embedInfo.lookup.constLookup.handle); + m_pLastNewIns->data[1] = resolvedEmbedResult.dataItemIndex; } CORINFO_CLASS_HANDLE clsHnd = m_compHnd->getTokenTypeAsHandle(&resolvedToken); diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 728330fe6d3c81..6c9453de8f657a 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -6,6 +6,7 @@ #include "intops.h" #include "datastructs.h" +#include "enum_class_flags.h" TArray PrintMethodName(COMP_HANDLE comp, CORINFO_CLASS_HANDLE clsHnd, @@ -366,7 +367,25 @@ class InterpCompiler CORINFO_CLASS_HANDLE ResolveClassToken(uint32_t token); CORINFO_CLASS_HANDLE getClassFromContext(CORINFO_CONTEXT_HANDLE context); int getParamArgIndex(); // Get the index into the m_pVars array of the Parameter argument. This is either the this pointer, a methoddesc or a class handle - int FillTempVarWithToken(CORINFO_RESOLVED_TOKEN* resolvedToken, bool embedParent, bool onlyIfNeedsRuntimeLookup, int existingVar = -1); + + struct InterpEmbedGenericResult + { + // If var is != -1, then the var holds the result of the lookup + int var = -1; + // If var == -1, then the data item holds the result of the lookup + int dataItemIndex = -1; + }; + + enum class GenericHandleEmbedOptions + { + support_use_as_flags = -1, + + None = 0, + VarOnly = 1, + EmbedParent = 2, + }; + InterpEmbedGenericResult EmitGenericHandle(CORINFO_RESOLVED_TOKEN* resolvedToken, GenericHandleEmbedOptions options); + int EmitGenericHandleAsVar(const CORINFO_GENERICHANDLE_RESULT &embedInfo); void* AllocMethodData(size_t numBytes); public: From d475b987a4720d874846dcca467be3ff29e38a70 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 6 Jun 2025 14:55:15 -0700 Subject: [PATCH 6/7] Attempt to fix build issues and code review comments --- src/coreclr/interpreter/compiler.cpp | 8 ++++++++ src/coreclr/interpreter/compiler.h | 2 +- src/coreclr/interpreter/interpretershared.h | 8 ++++---- src/coreclr/vm/interpexec.cpp | 11 +++++------ src/coreclr/vm/interpexec.h | 2 +- src/coreclr/vm/prestub.cpp | 2 +- 6 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 32621a5b51f603..caacf1c4cfcf4a 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1382,6 +1382,14 @@ void InterpCompiler::CreateILVars() INTERP_DUMP("\nCreate IL Vars:\n"); + // NOTE: There is special handling for the param arg, which is stored after the IL locals in the m_pVars array. + // The param arg is not part of the set of arguments defined by the IL method signature, but instead is needed + // to support shared generics codegen, and to be able to determine which exact intantiation of a method is in use. + // The param arg is stashed into the m_pVars array at an unnatural index relative to its position in the physical stack + // so that when parsing the MSIL byte stream it is simple to determine the index of the normal argumentes + // and IL locals, by just knowing the number of IL defined arguments. This allows all of the special handling for + // the param arg to be localized to this function, and the small set of helper functions that directly use it. + CORINFO_ARG_LIST_HANDLE sigArg = m_methodInfo->args.args; for (int i = 0; i < (numArgs + hasParamArg); i++) { InterpType interpType; diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 6c9453de8f657a..083b686a02321f 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -378,7 +378,7 @@ class InterpCompiler enum class GenericHandleEmbedOptions { - support_use_as_flags = -1, + support_use_as_flags = -1, // Magic value which in combination with enum_class_flags.h allows the use of bitwise operations and the HasFlag helper method None = 0, VarOnly = 1, diff --git a/src/coreclr/interpreter/interpretershared.h b/src/coreclr/interpreter/interpretershared.h index aaea4ee0df5272..9d55ae47761d16 100644 --- a/src/coreclr/interpreter/interpretershared.h +++ b/src/coreclr/interpreter/interpretershared.h @@ -53,13 +53,13 @@ struct InterpMethod struct InterpByteCodeStart { #ifndef DPTR - InterpMethod* const InterpMethod; // Pointer to the InterpMethod structure + InterpMethod* const Method; // Pointer to the InterpMethod structure #else - DPTR(InterpMethod) const InterpMethod; // Pointer to the InterpMethod structure + DPTR(InterpMethod) const Method; // Pointer to the InterpMethod structure #endif - const int* GetByteCodes() const + const int32_t* GetByteCodes() const { - return reinterpret_cast(this + 1); + return reinterpret_cast(this + 1); } }; diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index d5b090176b999f..9814484e4942a4 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -113,7 +113,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr const int32_t *ip; int8_t *stack; - InterpMethod *pMethod = pFrame->startIp->InterpMethod; + InterpMethod *pMethod = pFrame->startIp->Method; assert(pMethod->CheckIntegrity()); pThreadContext->pStackPointer = pFrame->pStack + pMethod->allocaSize; @@ -144,7 +144,6 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr } int32_t returnOffset, callArgsOffset, methodSlot; - PTR_InterpByteCodeStart targetIp; MethodDesc* targetMethod; MAIN_LOOP: @@ -1173,7 +1172,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr CALL_INTERP_SLOT: targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; CALL_INTERP_METHOD: - targetIp = targetMethod->GetInterpreterCode(); + InterpByteCodeStart* targetIp = targetMethod->GetInterpreterCode(); if (targetIp == NULL) { { @@ -1219,7 +1218,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr assert (((size_t)pFrame->pStack % INTERP_STACK_ALIGNMENT) == 0); // Set execution state for the new frame - pMethod = pFrame->startIp->InterpMethod; + pMethod = pFrame->startIp->Method; assert(pMethod->CheckIntegrity()); stack = pFrame->pStack; ip = pFrame->startIp->GetByteCodes(); @@ -1659,7 +1658,7 @@ do { \ ip = (int32_t*)resumeIP; stack = pFrame->pStack; - pMethod = pFrame->startIp->InterpMethod; + pMethod = pFrame->startIp->Method; assert(pMethod->CheckIntegrity()); pThreadContext->pStackPointer = pFrame->pStack + pMethod->allocaSize; goto MAIN_LOOP; @@ -1676,7 +1675,7 @@ do { \ pFrame = pFrame->pParent; ip = pFrame->ip; stack = pFrame->pStack; - pMethod = pFrame->startIp->InterpMethod; + pMethod = pFrame->startIp->Method; assert(pMethod->CheckIntegrity()); pFrame->ip = NULL; diff --git a/src/coreclr/vm/interpexec.h b/src/coreclr/vm/interpexec.h index 3e43692b2ce890..27d920bb97b666 100644 --- a/src/coreclr/vm/interpexec.h +++ b/src/coreclr/vm/interpexec.h @@ -34,7 +34,7 @@ struct InterpMethodContextFrame PTR_InterpMethodContextFrame pNext; #ifndef DACCESS_COMPILE - void ReInit(InterpMethodContextFrame *pParent, PTR_InterpByteCodeStart startIp, int8_t *pRetVal, int8_t *pStack) + void ReInit(InterpMethodContextFrame *pParent, InterpByteCodeStart* startIp, int8_t *pRetVal, int8_t *pStack) { this->pParent = pParent; this->startIp = startIp; diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 815c150089c1f0..fa1fb7a25b18f3 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -437,7 +437,7 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) amt.SuppressRelease(); pCode = PINSTRToPCODE(pPrecode->GetEntryPoint()); SetNativeCodeInterlocked(pCode); - SetInterpreterCode(dac_cast(pPrecode->GetData()->ByteCodeAddr)); + SetInterpreterCode(dac_cast(pPrecode->GetData()->ByteCodeAddr)); } #endif // FEATURE_INTERPRETER } From 017362b0bff776f6c9b6d342eee485ef1911aa23 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 6 Jun 2025 16:14:13 -0700 Subject: [PATCH 7/7] Improve factoring --- src/coreclr/interpreter/compiler.cpp | 103 +++++++++++---------------- src/coreclr/interpreter/compiler.h | 2 + 2 files changed, 44 insertions(+), 61 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index caacf1c4cfcf4a..d3451322dbecad 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1362,13 +1362,12 @@ int32_t InterpCompiler::GetInterpTypeStackSize(CORINFO_CLASS_HANDLE clsHnd, Inte return size; } - void InterpCompiler::CreateILVars() { bool hasThis = m_methodInfo->args.hasThis(); bool hasParamArg = m_methodInfo->args.hasTypeArg(); int paramArgIndex = hasParamArg ? hasThis ? 1 : 0 : INT_MAX; - int32_t offset, size, align; + int32_t offset; int numArgs = hasThis + m_methodInfo->args.numArgs; int numILLocals = m_methodInfo->locals.numArgs; m_numILVars = numArgs + numILLocals; @@ -1391,50 +1390,30 @@ void InterpCompiler::CreateILVars() // the param arg to be localized to this function, and the small set of helper functions that directly use it. CORINFO_ARG_LIST_HANDLE sigArg = m_methodInfo->args.args; - for (int i = 0; i < (numArgs + hasParamArg); i++) { - InterpType interpType; - CORINFO_CLASS_HANDLE argClass; - int iArgToSet = i; - if (iArgToSet > paramArgIndex) - { - iArgToSet--; // The param arg is stored after the IL locals in the m_pVars array - } - if (hasThis && i == 0) - { - argClass = m_compHnd->getMethodClass(m_methodInfo->ftn); - if (m_compHnd->isValueClass(argClass)) - interpType = InterpTypeByRef; - else - interpType = InterpTypeO; - } - else if (i == paramArgIndex) - { - iArgToSet = m_varsSize - 1; // The param arg is stored after the IL locals in the m_pVars array - m_paramArgIndex = iArgToSet; - INTERP_DUMP("Param arg at index m_pVars[%d]\n", iArgToSet); - interpType = InterpTypeI; - argClass = NULL; // No class for the param arg - } - else - { - CorInfoType argCorType; - argCorType = strip(m_compHnd->getArgType(&m_methodInfo->args, sigArg, &argClass)); - interpType = GetInterpType(argCorType); - sigArg = m_compHnd->getArgNext(sigArg); - } - size = GetInterpTypeStackSize(argClass, interpType, &align); - new (&m_pVars[iArgToSet]) InterpVar(interpType, argClass, size); + int argIndexOffset = 0; + if (hasThis) + { + CORINFO_CLASS_HANDLE argClass = m_compHnd->getMethodClass(m_methodInfo->ftn); + InterpType interpType = m_compHnd->isValueClass(argClass) ? InterpTypeByRef : InterpTypeO; + CreateNextLocalVar(0, argClass, interpType, &offset); + argIndexOffset++; + } - m_pVars[iArgToSet].global = true; - m_pVars[iArgToSet].ILGlobal = true; - m_pVars[iArgToSet].size = size; - offset = ALIGN_UP_TO(offset, align); - m_pVars[iArgToSet].offset = offset; - INTERP_DUMP("alloc arg var %d to offset %d in m_pVars[%d]\n", i, offset, iArgToSet); - offset += size; + if (hasParamArg) + { + m_paramArgIndex = m_varsSize - 1; // The param arg is stored after the IL locals in the m_pVars array + CreateNextLocalVar(m_paramArgIndex, NULL, InterpTypeI, &offset); } + for (int i = argIndexOffset; i < numArgs; i++) + { + CORINFO_CLASS_HANDLE argClass; + CorInfoType argCorType = strip(m_compHnd->getArgType(&m_methodInfo->args, sigArg, &argClass)); + InterpType interpType = GetInterpType(argCorType); + sigArg = m_compHnd->getArgNext(sigArg); + CreateNextLocalVar(i, argClass, interpType, &offset); + } offset = ALIGN_UP_TO(offset, INTERP_STACK_ALIGNMENT); sigArg = m_methodInfo->locals.args; @@ -1442,21 +1421,10 @@ void InterpCompiler::CreateILVars() int index = numArgs; for (int i = 0; i < numILLocals; i++) { - InterpType interpType; CORINFO_CLASS_HANDLE argClass; - CorInfoType argCorType = strip(m_compHnd->getArgType(&m_methodInfo->locals, sigArg, &argClass)); - interpType = GetInterpType(argCorType); - size = GetInterpTypeStackSize(argClass, interpType, &align); - - new (&m_pVars[index]) InterpVar(interpType, argClass, size); - - m_pVars[index].global = true; - m_pVars[index].ILGlobal = true; - offset = ALIGN_UP_TO(offset, align); - m_pVars[index].offset = offset; - INTERP_DUMP("alloc local var %d to offset %d\n", index, offset); - offset += size; + InterpType interpType = GetInterpType(argCorType); + CreateNextLocalVar(index, argClass, interpType, &offset); sigArg = m_compHnd->getArgNext(sigArg); index++; } @@ -1464,6 +1432,7 @@ void InterpCompiler::CreateILVars() if (hasParamArg) { // The param arg is stored after the IL locals in the m_pVars array + assert(index == m_paramArgIndex); index++; } @@ -1476,18 +1445,29 @@ void InterpCompiler::CreateILVars() for (unsigned int i = 0; i < m_methodInfo->EHcount; i++) { - new (&m_pVars[index]) InterpVar(InterpTypeO, NULL, INTERP_STACK_SLOT_SIZE); - m_pVars[index].global = true; - m_pVars[index].ILGlobal = true; - m_pVars[index].offset = offset; - INTERP_DUMP("alloc clause var %d to offset %d\n", index, offset); - offset += INTERP_STACK_SLOT_SIZE; + CreateNextLocalVar(index, NULL, InterpTypeO, &offset); index++; } m_totalVarsStackSize = offset; } +void InterpCompiler::CreateNextLocalVar(int iArgToSet, CORINFO_CLASS_HANDLE argClass, InterpType interpType, int32_t *pOffset) +{ + int32_t align; + int32_t size = GetInterpTypeStackSize(argClass, interpType, &align); + + new (&m_pVars[iArgToSet]) InterpVar(interpType, argClass, size); + + m_pVars[iArgToSet].global = true; + m_pVars[iArgToSet].ILGlobal = true; + m_pVars[iArgToSet].size = size; + *pOffset = ALIGN_UP_TO(*pOffset, align); + m_pVars[iArgToSet].offset = *pOffset; + INTERP_DUMP("alloc arg var %d to offset %d\n", iArgToSet, *pOffset); + *pOffset += size; +} + // Create finally call island basic blocks for all try regions with finally clauses that the leave exits. // That means when the leaveOffset is inside the try region and the target is outside of it. // These finally call island blocks are used for non-exceptional finally execution. @@ -4731,6 +4711,7 @@ static const char* s_jitHelperNames[CORINFO_HELP_COUNT] = { #define JITHELPER(code, pfnHelper, binderId) #code, #define DYNAMICJITHELPER(code, pfnHelper, binderId) #code, #include "jithelpers.h" +#include "compiler.h" }; const char* CorInfoHelperToName(CorInfoHelpFunc helper) diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 083b686a02321f..7648fddc064644 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -462,6 +462,8 @@ class InterpCompiler int32_t GetInterpTypeStackSize(CORINFO_CLASS_HANDLE clsHnd, InterpType interpType, int32_t *pAlign); void CreateILVars(); + void CreateNextLocalVar(int iArgToSet, CORINFO_CLASS_HANDLE argClass, InterpType interpType, int32_t *pOffset); + // Stack StackInfo *m_pStackPointer, *m_pStackBase; int32_t m_stackCapacity;