Skip to content

Add multi-dim array support to the interpreter #115916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/coreclr/interpreter/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3008,7 +3008,7 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)
ctorClass = m_compHnd->getMethodClass(ctorMethod);
int32_t numArgs = ctorSignature.numArgs;

// TODO Special case array ctor / string ctor
// TODO Special case array ctor. We detect string ctor at execution time
m_pStackPointer -= numArgs;

// Allocate callArgs for the call, this + numArgs + terminator
Expand Down Expand Up @@ -3042,11 +3042,15 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)
{
AddIns(INTOP_NEWOBJ_VT);
m_pLastNewIns->data[1] = (int32_t)ALIGN_UP_TO(vtsize, INTERP_STACK_SLOT_SIZE);
m_pLastNewIns->data[2] = 0;
}
else
{
AddIns(INTOP_NEWOBJ);
m_pLastNewIns->data[1] = GetDataItemIndex(ctorClass);
m_pLastNewIns->data[2] = (m_compHnd->getArrayRank(ctorClass) > 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to get a rank of 0? What does it mean in that case?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it, we call this even for non-arrays and that's what gets us rank of 0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once this is rebased onto the updated string ctors PR, it will be using a dedicated opcode so it should be less confusing.

? GetDataItemIndexForHelperFtn(CORINFO_HELP_NEW_MDARR)
: 0;
}
m_pLastNewIns->data[0] = GetMethodDataItemIndex(ctorMethod);
m_pLastNewIns->SetSVar(CALL_ARGS_SVAR);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/interpreter/intops.def
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ OPDEF(INTOP_LDFLDA, "ldflda", 4, 1, 1, InterpOpInt)
// Calls
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, "newobj", 6, 1, 1, InterpOpMethodHandle)
OPDEF(INTOP_NEWOBJ_VT, "newobj.vt", 5, 1, 1, InterpOpMethodHandle)

OPDEF(INTOP_CALL_HELPER_PP, "call.helper.pp", 5, 1, 0, InterpOpThreeInts)
Expand Down
13 changes: 13 additions & 0 deletions src/coreclr/vm/callstubgenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,14 @@ CallStubHeader *CallStubGenerator::GenerateCallStub(MethodDesc *pMD, AllocMemTra

_ASSERTE(pMD != NULL);

// Classes like System.String have special constructors that are fcalls. When invoking these, we need to override
// HasThis (there's no thisref) and the return type (the return value is the new instance, not void).
bool isSpecialConstructor = pMD->IsCtor() && pMD->GetMethodTable()->HasComponentSize();

MetaSig sig(pMD);
if (isSpecialConstructor)
sig.ClearHasThis();

ArgIterator argIt(&sig);

m_r1 = NoRange; // indicates that there is no active range of general purpose registers
Expand Down Expand Up @@ -679,6 +686,12 @@ CallStubHeader *CallStubGenerator::GenerateCallStub(MethodDesc *pMD, AllocMemTra
{
TypeHandle thReturnValueType;
CorElementType thReturnType = sig.GetReturnTypeNormalized(&thReturnValueType);
if (isSpecialConstructor)
{
assert(thReturnType == ELEMENT_TYPE_VOID);
// FIXME: String?
thReturnType = ELEMENT_TYPE_OBJECT;
}

switch (thReturnType)
{
Expand Down
120 changes: 95 additions & 25 deletions src/coreclr/vm/interpexec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include "interpexec.h"
#include "callstubgenerator.h"

void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet)
void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE pCode)
{
CONTRACTL
{
Expand Down Expand Up @@ -41,14 +41,15 @@ void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet)
}
}

pHeader->SetTarget(pMD->GetNativeCode()); // The method to call
pHeader->SetTarget(pCode); // The method to call

pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize);
}

typedef void* (*HELPER_FTN_PP)(void*);
typedef void* (*HELPER_FTN_BOX_UNBOX)(MethodTable*, void*);
typedef Object* (*HELPER_FTN_NEWARR)(CORINFO_CLASS_HANDLE, intptr_t);
typedef Object* (*HELPER_FTN_NEW_MDARR)(CORINFO_CLASS_HANDLE, int, void*);

InterpThreadContext::InterpThreadContext()
{
Expand Down Expand Up @@ -1118,22 +1119,35 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
// 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
// would need to seek through the frames to find it.
// An alternative approach would be to update the topmost frame during stack walk
// to make the probability that the next stack walk will need to search only a
// small subset of frames high.
pInterpreterFrame->SetTopInterpMethodContextFrame(pFrame);
GCX_PREEMP();
pMD->PrepareInitialCode(CallerGCMode::Coop);
code = pMD->GetNativeCode();
if (pMD->IsArray() || pMD->IsFCall())
{
// For FCalls and array special methods we can't go through the regular caching path
// because it will try to do pMD->GetNativeCode() and then crash.
PCODE code = pMD->TryGetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY);
assert(code);
InvokeCompiledMethod(pMD, stack + callArgsOffset, stack + returnOffset, code);
// FIXME: Caching in dataItems somehow
break;
}
else
{
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
// would need to seek through the frames to find it.
// An alternative approach would be to update the topmost frame during stack walk
// to make the probability that the next stack walk will need to search only a
// small subset of frames high.
pInterpreterFrame->SetTopInterpMethodContextFrame(pFrame);
GCX_PREEMP();
pMD->PrepareInitialCode(CallerGCMode::Coop);
code = pMD->GetNativeCode();
}
pMethod->pDataItems[methodSlot] = (void*)code;
targetIp = (const int32_t*)code;
}
pMethod->pDataItems[methodSlot] = (void*)code;
targetIp = (const int32_t*)code;
}
else
{
Expand All @@ -1151,7 +1165,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
else if (codeInfo.GetCodeManager() != ExecutionManager::GetInterpreterCodeManager())
{
MethodDesc *pMD = codeInfo.GetMethodDesc();
InvokeCompiledMethod(pMD, stack + callArgsOffset, stack + returnOffset);
InvokeCompiledMethod(pMD, stack + callArgsOffset, stack + returnOffset, pMD->GetNativeCode());
break;
}

Expand Down Expand Up @@ -1186,15 +1200,71 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
callArgsOffset = ip[2];
methodSlot = ip[3];

OBJECTREF objRef = AllocateObject((MethodTable*)pMethod->pDataItems[ip[4]]);
MethodTable *pClass = (MethodTable*)pMethod->pDataItems[ip[4]];
// FIXME: Duplicated code from CALL_INTERP_SLOT
size_t targetMethod = (size_t)pMethod->pDataItems[methodSlot];
MethodDesc *pMD = nullptr;
if (targetMethod & INTERP_METHOD_HANDLE_TAG)
{
pMD = (MethodDesc*)(targetMethod & ~INTERP_METHOD_HANDLE_TAG);
}

// This is return value
LOCAL_VAR(returnOffset, OBJECTREF) = objRef;
// Set `this` arg for ctor call
LOCAL_VAR (callArgsOffset, OBJECTREF) = objRef;
ip += 5;
// If we are constructing a type with a component size (i.e. a string) its constructor is a special
// fcall that is basically a static method that returns the new instance.
if (pMD && pClass->HasComponentSize())
{
// The compiler didn't know about this so it reserved space for a this-reference. We need to skip
// past that reserved space, since the callee doesn't expect it.
LOCAL_VAR(callArgsOffset, void*) = nullptr; // and zero it too! it might get scanned by the GC.
callArgsOffset += sizeof(StackVal);

goto CALL_INTERP_SLOT;
if (pClass->IsArray())
{
// mdarray ctors are implemented via a helper ftn we grabbed at compile time
size_t helperDirectOrIndirect = (size_t)pMethod->pDataItems[ip[5]];
HELPER_FTN_NEW_MDARR helper = nullptr;
if (helperDirectOrIndirect & INTERP_INDIRECT_HELPER_TAG)
helper = *(HELPER_FTN_NEW_MDARR *)(helperDirectOrIndirect & ~INTERP_INDIRECT_HELPER_TAG);
else
helper = (HELPER_FTN_NEW_MDARR)helperDirectOrIndirect;
assert(helper);

// mdarray ctor wants an array of ints, we can't pass the callArgsOffset directly since
// it's technically a stream of intptrs even if each value is an int
int rank = pClass->GetRank();
int dims[8] = { 0 };
assert(rank < 8);
for (int i = 0; i < rank; i++)
dims[i] = LOCAL_VAR(callArgsOffset + (i * sizeof(StackVal)), int);

LOCAL_VAR(returnOffset, Object*) = helper((CORINFO_CLASS_HANDLE)pClass, rank, dims);
}
else
{
// Get the address of the fcall that implements the ctor
PCODE code = pMD->TryGetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY);
assert(code);

// callArgsOffset now only points to the ctor arguments, which are what the fcall expects.
// returnOffset still points to where the new instance goes, and the fcall will write it there.
InvokeCompiledMethod(pMD, stack + callArgsOffset, stack + returnOffset, code);
}

ip += 6;
break;
}
else
{
OBJECTREF objRef = AllocateObject(pClass);

// This is return value
LOCAL_VAR(returnOffset, OBJECTREF) = objRef;
// Set `this` arg for ctor call
LOCAL_VAR(callArgsOffset, OBJECTREF) = objRef;
ip += 6;

goto CALL_INTERP_SLOT;
}
}
case INTOP_NEWOBJ_VT:
{
Expand Down
54 changes: 47 additions & 7 deletions src/tests/JIT/interpreter/Interpreter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,9 @@ public static void RunInterpreterTests()
if (!TestBoxing())
Environment.FailFast(null);

if (!TestStringCtor())
Environment.FailFast(null);

if (!TestArray())
Environment.FailFast(null);

Expand All @@ -421,10 +424,9 @@ public static void RunInterpreterTests()

if (!TestLdtoken())
Environment.FailFast(null);
/*

if (!TestMdArray())
Environment.FailFast(null);
*/

System.GC.Collect();
}
Expand Down Expand Up @@ -705,6 +707,18 @@ public static bool TestBoxing()
return result == 3;
}

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;
}

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
static object BoxedSubtraction (object lhs, object rhs) {
return (int)lhs - (int)rhs;
Expand Down Expand Up @@ -985,12 +999,38 @@ public static bool TestLdtoken()

public static bool TestMdArray()
{
// FIXME: This generates roughly:
// newobj int[,].ctor
// ldtoken int[,]
// call System.Runtime.CompilerServices.RuntimeHelpers.InitializeArray
// The newobj currently fails because int[,].ctor isn't a real method, the interp needs to use getCallInfo to determine how to invoke it
/* FIXME: This generates roughly:
newobj int[,].ctor
ldtoken int[,]
call System.Runtime.CompilerServices.RuntimeHelpers.InitializeArray
While the newobj now works, the InitializeArray call fails:
Assert failure(PID 61692 [0x0000f0fc], Thread: 40872 [0x9fa8]): Consistency check failed:
FAILED: (!implSlot.IsNull() || pMT->IsInterface()) && "Valid method implementation was not found."

CORECLR! CHECK::Trigger + 0x20F (0x00007ffa`807af47f)
CORECLR! VirtualCallStubManager::Resolver + 0xFAA (0x00007ffa`80e387fa)
CORECLR! VirtualCallStubManager::ResolveWorker + 0xB3C (0x00007ffa`80e36d2c)
CORECLR! ExternalMethodFixupWorker + 0xDCC (0x00007ffa`80d5d49c)
CORECLR! DelayLoad_MethodCall + 0x71 (0x00007ffa`813a0601)
SYSTEM.PRIVATE.CORELIB! <no symbol> + 0x0 (0x00007ffa`77af0098)
CORECLR! CallJittedMethodRetVoid + 0x14 (0x00007ffa`8139e7e4)
*/
/*
int[,] a = {{1, 2}, {3, 4}};
return a[0, 1] == 2;
*/

int[,] a = new int[2,2];
if (a[0, 1] != 0)
return false;

a[0, 0] = 1;
a[0, 1] = 2;
a[1, 0] = 3;
a[1, 1] = 4;
if (a[0, 1] != 2)
return false;

return true;
}
}
Loading