Skip to content

clrinterp: Add missing must-expand intrinsics #103326

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Jun 14, 2024
Merged

clrinterp: Add missing must-expand intrinsics #103326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f7a172a
clrinterp: add IsReferenceOrContainsReferences intrinsic
EgorBo Jun 12, 2024
fd15cdf
fix nre
EgorBo Jun 12, 2024
c5c0eb1
Add missing intrinsics
EgorBo Jun 12, 2024
83f5514
Fix byref structs in IsReferenceOrContainsReferences
EgorBo Jun 12, 2024
b1369d6
oops
EgorBo Jun 12, 2024
79791ee
disable TLS opt
EgorBo Jun 12, 2024
cf18e07
Address feedback
EgorBo Jun 12, 2024
f35e849
Address feedback
EgorBo Jun 13, 2024
4083c1a
Merge branch 'main' into fix-IsReferenceOrContainsReferences-interp
EgorBo Jun 13, 2024
f8b86c3
disable TLS opt for interp
EgorBo Jun 13, 2024
76b35a0
Merge branch 'main' of github.com:dotnet/runtime into fix-IsReference…
EgorBo Jun 14, 2024
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
240 changes: 240 additions & 0 deletions src/coreclr/vm/interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9257,13 +9257,34 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T
DoGetArrayDataReference();
didIntrinsic = true;
break;

#if INTERP_ILSTUBS
case NI_System_StubHelpers_GetStubContext:
OpStackSet<void*>(m_curStackHt, GetStubContext());
OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_NATIVEINT));
m_curStackHt++; didIntrinsic = true;
break;
#endif // INTERP_ILSTUBS

case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences:
DoIsReferenceOrContainsReferences(reinterpret_cast<CORINFO_METHOD_HANDLE>(methToCall));
didIntrinsic = true;
break;

case NI_System_Threading_Interlocked_CompareExchange:
// Here and in other Interlocked.* intrinsics we use sigInfo.retType to be able
// to detect small-integer overloads.
didIntrinsic = DoInterlockedCompareExchange(sigInfo.retType);
break;

case NI_System_Threading_Interlocked_Exchange:
didIntrinsic = DoInterlockedExchange(sigInfo.retType);
break;

case NI_System_Threading_Interlocked_ExchangeAdd:
didIntrinsic = DoInterlockedExchangeAdd(sigInfo.retType);
break;

default:
#if INTERP_TRACING
InterlockedIncrement(&s_totalInterpCallsToIntrinsicsUnhandled);
Expand Down Expand Up @@ -10903,6 +10924,197 @@ void Interpreter::DoGetArrayDataReference()
OpStackTypeSet(ind, InterpreterType(CORINFO_TYPE_BYREF));
}

static bool HasByrefFields(MethodTable* pMT)
{
// Inspect all instance fields recursively
ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS);
for (FieldDesc* pFD = fieldIterator.Next(); pFD != nullptr; pFD = fieldIterator.Next())
{
if (pFD->IsByRef())
{
return true;
}
if ((pFD->GetFieldType() == ELEMENT_TYPE_VALUETYPE &&
HasByrefFields(pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable())))
{
return true;
}
}
return false;
}

void Interpreter::DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method)
{
CONTRACTL{
THROWS;
GC_TRIGGERS;
MODE_COOPERATIVE;
} CONTRACTL_END;

CORINFO_SIG_INFO sigInfoFull;
{
GCX_PREEMP();
m_interpCeeInfo.getMethodSig(method, & sigInfoFull, nullptr);
}

MethodTable* typeArg = GetMethodTableFromClsHnd(sigInfoFull.sigInst.methInst[0]);

bool containsGcPtrs = typeArg->ContainsPointers();

// Return true for byref-like structs with ref fields (they might not have them)
if (!containsGcPtrs && typeArg->IsByRefLike())
{
containsGcPtrs |= HasByrefFields(typeArg);
}

OpStackSet<BOOL>(m_curStackHt, containsGcPtrs);
OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT));
m_curStackHt++;
}

bool Interpreter::DoInterlockedCompareExchange(CorInfoType retType)
{
CONTRACTL{
THROWS;
GC_TRIGGERS;
MODE_COOPERATIVE;
} CONTRACTL_END;

// These CompareExchange are must-expand:
//
// long CompareExchange(ref long location1, long value, long comparand)
// int CompareExchange(ref int location1, int value, int comparand)
// ushort CompareExchange(ref ushort location1, ushort value, ushort comparand)
// byte CompareExchange(ref byte location1, byte value, byte comparand)
//
// Detect these by retType (signature)
unsigned comparandInd = m_curStackHt - 1;
unsigned valueInd = m_curStackHt - 2;
unsigned locationInd = m_curStackHt - 3;
switch (retType)
{
case CORINFO_TYPE_LONG:
m_curStackHt -= 3;
OpStackSet<int64_t>(m_curStackHt, InterlockedCompareExchange64(
OpStackGet<int64_t*>(locationInd),
OpStackGet<int64_t>(valueInd),
OpStackGet<int64_t>(comparandInd)));
OpStackTypeSet(m_curStackHt, InterpreterType(retType));
m_curStackHt++;
return true;

case CORINFO_TYPE_INT:
m_curStackHt -= 3;
OpStackSet<LONG>(m_curStackHt, InterlockedCompareExchange(
OpStackGet<LONG*>(locationInd),
OpStackGet<LONG>(valueInd),
OpStackGet<LONG>(comparandInd)));
OpStackTypeSet(m_curStackHt, InterpreterType(retType));
m_curStackHt++;
return true;

case CORINFO_TYPE_SHORT:
case CORINFO_TYPE_BYTE:
NYI_INTERP("TODO: Implement must-expand atomics for small types.");
return false;

default:
// Non must-expand intrinsics
return false;
}
}

bool Interpreter::DoInterlockedExchange(CorInfoType retType)
{
CONTRACTL{
THROWS;
GC_TRIGGERS;
MODE_COOPERATIVE;
} CONTRACTL_END;

// These Exchange are must-expand:
//
// long Exchange(ref long location1, long value)
// int Exchange(ref int location1, int value)
// ushort Exchange(ref ushort location1, ushort value)
// byte Exchange(ref byte location1, byte value)
//
// Detect these by retType (signature)
unsigned valueInd = m_curStackHt - 1;
unsigned locationInd = m_curStackHt - 2;
switch (retType)
{
case CORINFO_TYPE_LONG:
m_curStackHt -= 2;
OpStackSet<int64_t>(m_curStackHt, InterlockedExchange64(
OpStackGet<int64_t*>(locationInd),
OpStackGet<int64_t>(valueInd)));
OpStackTypeSet(m_curStackHt, InterpreterType(retType));
m_curStackHt++;
return true;

case CORINFO_TYPE_INT:
m_curStackHt -= 2;
OpStackSet<LONG>(m_curStackHt, InterlockedExchange(
OpStackGet<LONG*>(locationInd),
OpStackGet<LONG>(valueInd)));
OpStackTypeSet(m_curStackHt, InterpreterType(retType));
m_curStackHt++;
return true;

case CORINFO_TYPE_SHORT:
case CORINFO_TYPE_BYTE:
NYI_INTERP("TODO: Implement must-expand Exchange for small types.");
return false;

default:
// Non must-expand intrinsics
return false;
}
}

bool Interpreter::DoInterlockedExchangeAdd(CorInfoType retType)
{
CONTRACTL{
THROWS;
GC_TRIGGERS;
MODE_COOPERATIVE;
} CONTRACTL_END;

// These ExchangeAdd are must-expand:
//
// long ExchangeAdd(ref long location1, long value)
// int ExchangeAdd(ref int location1, int value)
//
// Detect these by retType (signature)
unsigned valueInd = m_curStackHt - 1;
unsigned locationInd = m_curStackHt - 2;
switch (retType)
{
case CORINFO_TYPE_LONG:
m_curStackHt -= 2;
OpStackSet<int64_t>(m_curStackHt, InterlockedExchangeAdd64(
OpStackGet<int64_t*>(locationInd),
OpStackGet<int64_t>(valueInd)));
OpStackTypeSet(m_curStackHt, InterpreterType(retType));
m_curStackHt++;
return true;

case CORINFO_TYPE_INT:
m_curStackHt -= 2;
OpStackSet<LONG>(m_curStackHt, InterlockedExchangeAdd(
OpStackGet<LONG*>(locationInd),
OpStackGet<LONG>(valueInd)));
OpStackTypeSet(m_curStackHt, InterpreterType(retType));
m_curStackHt++;
return true;

default:
// Non must-expand intrinsics
return false;
}
}

void Interpreter::RecordConstrainedCall()
{
CONTRACTL {
Expand Down Expand Up @@ -11762,6 +11974,34 @@ Interpreter::InterpreterNamedIntrinsics Interpreter::getNamedIntrinsicID(CEEInfo
}
}
}
else if (strcmp(namespaceName, "CompilerServices") == 0)
{
if (strcmp(className, "RuntimeHelpers") == 0)
{
if (strcmp(methodName, "IsReferenceOrContainsReferences") == 0)
{
result = NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences;
}
}
}
}
else if (strncmp(namespaceName, "Threading", 8) == 0)
{
if (strcmp(className, "Interlocked") == 0)
{
if (strcmp(methodName, "CompareExchange") == 0)
{
result = NI_System_Threading_Interlocked_CompareExchange;
}
else if (strcmp(methodName, "Exchange") == 0)
{
result = NI_System_Threading_Interlocked_Exchange;
}
else if (strcmp(methodName, "ExchangeAdd") == 0)
{
result = NI_System_Threading_Interlocked_ExchangeAdd;
}
}
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/vm/interpreter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,10 @@ class Interpreter
NI_Illegal = 0,
NI_System_StubHelpers_GetStubContext,
NI_System_Runtime_InteropService_MemoryMarshal_GetArrayDataReference,
NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences,
NI_System_Threading_Interlocked_CompareExchange,
NI_System_Threading_Interlocked_Exchange,
NI_System_Threading_Interlocked_ExchangeAdd,
};
static InterpreterNamedIntrinsics getNamedIntrinsicID(CEEInfo* info, CORINFO_METHOD_HANDLE methodHnd);
static const char* getMethodName(CEEInfo* info, CORINFO_METHOD_HANDLE hnd, const char** className, const char** namespaceName = NULL, const char **enclosingClassName = NULL);
Expand Down Expand Up @@ -1790,6 +1794,10 @@ class Interpreter
void DoSIMDHwAccelerated();
void DoGetIsSupported();
void DoGetArrayDataReference();
void DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method);
bool DoInterlockedCompareExchange(CorInfoType retType);
bool DoInterlockedExchange(CorInfoType retType);
bool DoInterlockedExchangeAdd(CorInfoType retType);

// Returns the proper generics context for use in resolving tokens ("precise" in the sense of including generic instantiation
// information).
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/threadstatics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,8 @@ bool CanJITOptimizeTLSAccess()
// Optimization is disabled for linux musl arm64
#elif defined(TARGET_FREEBSD) && defined(TARGET_ARM64)
// Optimization is disabled for FreeBSD/arm64
#elif defined(FEATURE_INTERPRETER)
// Optimization is disabled when interpreter may be used
#else
optimizeThreadStaticAccess = true;
#if !defined(TARGET_OSX) && defined(TARGET_UNIX) && defined(TARGET_AMD64)
Expand Down