From f7a172ab1ed3920283ff2901a8a655bd7f7b6af6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Jun 2024 04:43:06 +0200 Subject: [PATCH 1/9] clrinterp: add IsReferenceOrContainsReferences intrinsic --- src/coreclr/vm/interpreter.cpp | 18 ++++++++++++++++++ src/coreclr/vm/interpreter.h | 1 + 2 files changed, 19 insertions(+) diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index ecc46af9ecf03c..8b46179119ef9e 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -9264,6 +9264,14 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T m_curStackHt++; didIntrinsic = true; break; #endif // INTERP_ILSTUBS + + case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences: + OpStackSet(m_curStackHt, GetMethodTableFromClsHnd(callInfoPtr->sig.sigInst.methInst[0])->ContainsPointers()); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); + m_curStackHt++; + didIntrinsic = true; + break; + default: #if INTERP_TRACING InterlockedIncrement(&s_totalInterpCallsToIntrinsicsUnhandled); @@ -11762,6 +11770,16 @@ 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; + } + } + } } } } diff --git a/src/coreclr/vm/interpreter.h b/src/coreclr/vm/interpreter.h index 511aa21a64afd7..e5bf0521d920e9 100644 --- a/src/coreclr/vm/interpreter.h +++ b/src/coreclr/vm/interpreter.h @@ -920,6 +920,7 @@ class Interpreter NI_Illegal = 0, NI_System_StubHelpers_GetStubContext, NI_System_Runtime_InteropService_MemoryMarshal_GetArrayDataReference, + NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences, }; 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); From fd15cdfcca06cc65ff4eb6075f4c8960e2694e9d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Jun 2024 05:05:46 +0200 Subject: [PATCH 2/9] fix nre --- src/coreclr/vm/interpreter.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index 8b46179119ef9e..af14e2abc2d820 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -9266,10 +9266,19 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T #endif // INTERP_ILSTUBS case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences: - OpStackSet(m_curStackHt, GetMethodTableFromClsHnd(callInfoPtr->sig.sigInst.methInst[0])->ContainsPointers()); + { + CORINFO_SIG_INFO sigInfoFull; + { + GCX_PREEMP(); + m_interpCeeInfo.getMethodSig(CORINFO_METHOD_HANDLE(methToCall), &sigInfoFull, nullptr); + } + + MethodTable* typeArg = GetMethodTableFromClsHnd(sigInfoFull.sigInst.methInst[0]); + OpStackSet(m_curStackHt, typeArg->ContainsPointers()); OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); m_curStackHt++; didIntrinsic = true; + } break; default: From c5c0eb12c64136d25ff00bb0b950c6983d71703a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Jun 2024 06:17:28 +0200 Subject: [PATCH 3/9] Add missing intrinsics --- src/coreclr/vm/interpreter.cpp | 209 +++++++++++++++++++++++++++++++-- src/coreclr/vm/interpreter.h | 7 ++ 2 files changed, 203 insertions(+), 13 deletions(-) diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index af14e2abc2d820..3fd6f37a02613a 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -9257,6 +9257,7 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T DoGetArrayDataReference(); didIntrinsic = true; break; + #if INTERP_ILSTUBS case NI_System_StubHelpers_GetStubContext: OpStackSet(m_curStackHt, GetStubContext()); @@ -9266,20 +9267,21 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T #endif // INTERP_ILSTUBS case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsReferenceOrContainsReferences: - { - CORINFO_SIG_INFO sigInfoFull; - { - GCX_PREEMP(); - m_interpCeeInfo.getMethodSig(CORINFO_METHOD_HANDLE(methToCall), &sigInfoFull, nullptr); - } - - MethodTable* typeArg = GetMethodTableFromClsHnd(sigInfoFull.sigInst.methInst[0]); - OpStackSet(m_curStackHt, typeArg->ContainsPointers()); - OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); - m_curStackHt++; + DoIsReferenceOrContainsReferences(reinterpret_cast(methToCall)); didIntrinsic = true; - } - break; + break; + + case NI_System_Threading_Interlocked_CompareExchange: + didIntrinsic = DoInterlockedCompareExchange(); + break; + + case NI_System_Threading_Interlocked_Exchange: + didIntrinsic = DoInterlockedExchange(); + break; + + case NI_System_Threading_Interlocked_ExchangeAdd: + didIntrinsic = DoInterlockedExchangeAdd(); + break; default: #if INTERP_TRACING @@ -10920,6 +10922,169 @@ void Interpreter::DoGetArrayDataReference() OpStackTypeSet(ind, InterpreterType(CORINFO_TYPE_BYREF)); } +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]); + OpStackSet(m_curStackHt, typeArg->ContainsPointers()); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); + m_curStackHt++; +} + +bool Interpreter::DoInterlockedCompareExchange() +{ + 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 comparand's type (stack - 1): + unsigned comparandInd = m_curStackHt - 1; + unsigned valueInd = m_curStackHt - 2; + unsigned locationInd = m_curStackHt - 3; + switch (OpStackTypeGet(comparandInd).ToCorInfoType()) + { + case CORINFO_TYPE_LONG: + m_curStackHt -= 3; + OpStackSet(m_curStackHt, InterlockedCompareExchange64( + OpStackGet(locationInd), + OpStackGet(valueInd), + OpStackGet(comparandInd))); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_LONG)); + m_curStackHt++; + return true; + + case CORINFO_TYPE_INT: + m_curStackHt -= 3; + OpStackSet(m_curStackHt, InterlockedCompareExchange( + OpStackGet(locationInd), + OpStackGet(valueInd), + OpStackGet(comparandInd))); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); + 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() +{ + 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 value's type (stack - 1): + unsigned valueInd = m_curStackHt - 1; + unsigned locationInd = m_curStackHt - 2; + switch (OpStackTypeGet(valueInd).ToCorInfoType()) + { + case CORINFO_TYPE_LONG: + m_curStackHt -= 2; + OpStackSet(m_curStackHt, InterlockedExchange64( + OpStackGet(locationInd), + OpStackGet(valueInd))); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_LONG)); + m_curStackHt++; + return true; + + case CORINFO_TYPE_INT: + m_curStackHt -= 2; + OpStackSet(m_curStackHt, InterlockedExchange( + OpStackGet(locationInd), + OpStackGet(valueInd))); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); + 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() +{ + 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 value's type (stack - 1): + unsigned valueInd = m_curStackHt - 1; + unsigned locationInd = m_curStackHt - 2; + switch (OpStackTypeGet(valueInd).ToCorInfoType()) + { + case CORINFO_TYPE_LONG: + m_curStackHt -= 2; + OpStackSet(m_curStackHt, InterlockedExchangeAdd64( + OpStackGet(locationInd), + OpStackGet(valueInd))); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_LONG)); + m_curStackHt++; + return true; + + case CORINFO_TYPE_INT: + m_curStackHt -= 2; + OpStackSet(m_curStackHt, InterlockedExchangeAdd( + OpStackGet(locationInd), + OpStackGet(valueInd))); + OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); + m_curStackHt++; + return true; + + default: + // Non must-expand intrinsics + return false; + } +} + void Interpreter::RecordConstrainedCall() { CONTRACTL { @@ -11790,6 +11955,24 @@ Interpreter::InterpreterNamedIntrinsics Interpreter::getNamedIntrinsicID(CEEInfo } } } + 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; + } + } + } } } return result; diff --git a/src/coreclr/vm/interpreter.h b/src/coreclr/vm/interpreter.h index e5bf0521d920e9..31310b2b03ad0c 100644 --- a/src/coreclr/vm/interpreter.h +++ b/src/coreclr/vm/interpreter.h @@ -921,6 +921,9 @@ class Interpreter 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); @@ -1791,6 +1794,10 @@ class Interpreter void DoSIMDHwAccelerated(); void DoGetIsSupported(); void DoGetArrayDataReference(); + void DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method); + bool DoInterlockedCompareExchange(); + bool DoInterlockedExchange(); + bool DoInterlockedExchangeAdd(); // Returns the proper generics context for use in resolving tokens ("precise" in the sense of including generic instantiation // information). From 83f5514cf9a0565c830788a5d857a5b75ada11fd Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Jun 2024 06:24:12 +0200 Subject: [PATCH 4/9] Fix byref structs in IsReferenceOrContainsReferences --- src/coreclr/vm/interpreter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index 3fd6f37a02613a..a34f801e02b5e6 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -10937,7 +10937,7 @@ void Interpreter::DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method } MethodTable* typeArg = GetMethodTableFromClsHnd(sigInfoFull.sigInst.methInst[0]); - OpStackSet(m_curStackHt, typeArg->ContainsPointers()); + OpStackSet(m_curStackHt, typeArg->IsByRefLike || typeArg->ContainsPointers()); OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); m_curStackHt++; } From b1369d66df9a9f9cf36723fdb0aa3de10a0eb5bf Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Jun 2024 06:26:17 +0200 Subject: [PATCH 5/9] oops --- src/coreclr/vm/interpreter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index a34f801e02b5e6..10485f4ae03894 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -10937,7 +10937,7 @@ void Interpreter::DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method } MethodTable* typeArg = GetMethodTableFromClsHnd(sigInfoFull.sigInst.methInst[0]); - OpStackSet(m_curStackHt, typeArg->IsByRefLike || typeArg->ContainsPointers()); + OpStackSet(m_curStackHt, typeArg->IsByRefLike() || typeArg->ContainsPointers()); OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); m_curStackHt++; } From 79791ee52846b416eb045bda76269dbe3048aecb Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Jun 2024 06:38:07 +0200 Subject: [PATCH 6/9] disable TLS opt --- src/coreclr/vm/jitinterface.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 8022543639ea51..3463ffdddc7179 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -1590,6 +1590,11 @@ void CEEInfo::getFieldInfo (CORINFO_RESOLVED_TOKEN * pResolvedToken, optimizeThreadStaticAccess = GetTlsIndexObjectAddress() != nullptr; #endif // !TARGET_OSX && TARGET_UNIX && TARGET_AMD64 +#if defined(FEATURE_INTERPRETER) + // Not yet supported by the interpreter + optimizeThreadStaticAccess = false; +#endif + if (optimizeThreadStaticAccess) { // For windows x64/x86/arm64, linux x64/arm64/loongarch64/riscv64: From cf18e07652bf79ba179e6d221843845f859a7774 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Jun 2024 07:21:12 +0200 Subject: [PATCH 7/9] Address feedback --- src/coreclr/vm/interpreter.cpp | 38 ++++++++++++++++++---------------- src/coreclr/vm/interpreter.h | 6 +++--- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index 10485f4ae03894..99e8d7e630fee6 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -9272,15 +9272,17 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T break; case NI_System_Threading_Interlocked_CompareExchange: - didIntrinsic = DoInterlockedCompareExchange(); + // 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(); + didIntrinsic = DoInterlockedExchange(sigInfo.retType); break; case NI_System_Threading_Interlocked_ExchangeAdd: - didIntrinsic = DoInterlockedExchangeAdd(); + didIntrinsic = DoInterlockedExchangeAdd(sigInfo.retType); break; default: @@ -10942,7 +10944,7 @@ void Interpreter::DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method m_curStackHt++; } -bool Interpreter::DoInterlockedCompareExchange() +bool Interpreter::DoInterlockedCompareExchange(CorInfoType retType) { CONTRACTL{ THROWS; @@ -10957,11 +10959,11 @@ bool Interpreter::DoInterlockedCompareExchange() // ushort CompareExchange(ref ushort location1, ushort value, ushort comparand) // byte CompareExchange(ref byte location1, byte value, byte comparand) // - // Detect these by comparand's type (stack - 1): + // Detect these by retType (signature) unsigned comparandInd = m_curStackHt - 1; unsigned valueInd = m_curStackHt - 2; unsigned locationInd = m_curStackHt - 3; - switch (OpStackTypeGet(comparandInd).ToCorInfoType()) + switch (retType) { case CORINFO_TYPE_LONG: m_curStackHt -= 3; @@ -10969,7 +10971,7 @@ bool Interpreter::DoInterlockedCompareExchange() OpStackGet(locationInd), OpStackGet(valueInd), OpStackGet(comparandInd))); - OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_LONG)); + OpStackTypeSet(m_curStackHt, InterpreterType(retType)); m_curStackHt++; return true; @@ -10979,7 +10981,7 @@ bool Interpreter::DoInterlockedCompareExchange() OpStackGet(locationInd), OpStackGet(valueInd), OpStackGet(comparandInd))); - OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); + OpStackTypeSet(m_curStackHt, InterpreterType(retType)); m_curStackHt++; return true; @@ -10994,7 +10996,7 @@ bool Interpreter::DoInterlockedCompareExchange() } } -bool Interpreter::DoInterlockedExchange() +bool Interpreter::DoInterlockedExchange(CorInfoType retType) { CONTRACTL{ THROWS; @@ -11009,17 +11011,17 @@ bool Interpreter::DoInterlockedExchange() // ushort Exchange(ref ushort location1, ushort value) // byte Exchange(ref byte location1, byte value) // - // Detect these by value's type (stack - 1): + // Detect these by retType (signature) unsigned valueInd = m_curStackHt - 1; unsigned locationInd = m_curStackHt - 2; - switch (OpStackTypeGet(valueInd).ToCorInfoType()) + switch (retType) { case CORINFO_TYPE_LONG: m_curStackHt -= 2; OpStackSet(m_curStackHt, InterlockedExchange64( OpStackGet(locationInd), OpStackGet(valueInd))); - OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_LONG)); + OpStackTypeSet(m_curStackHt, InterpreterType(retType)); m_curStackHt++; return true; @@ -11028,7 +11030,7 @@ bool Interpreter::DoInterlockedExchange() OpStackSet(m_curStackHt, InterlockedExchange( OpStackGet(locationInd), OpStackGet(valueInd))); - OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); + OpStackTypeSet(m_curStackHt, InterpreterType(retType)); m_curStackHt++; return true; @@ -11043,7 +11045,7 @@ bool Interpreter::DoInterlockedExchange() } } -bool Interpreter::DoInterlockedExchangeAdd() +bool Interpreter::DoInterlockedExchangeAdd(CorInfoType retType) { CONTRACTL{ THROWS; @@ -11056,17 +11058,17 @@ bool Interpreter::DoInterlockedExchangeAdd() // long ExchangeAdd(ref long location1, long value) // int ExchangeAdd(ref int location1, int value) // - // Detect these by value's type (stack - 1): + // Detect these by retType (signature) unsigned valueInd = m_curStackHt - 1; unsigned locationInd = m_curStackHt - 2; - switch (OpStackTypeGet(valueInd).ToCorInfoType()) + switch (retType) { case CORINFO_TYPE_LONG: m_curStackHt -= 2; OpStackSet(m_curStackHt, InterlockedExchangeAdd64( OpStackGet(locationInd), OpStackGet(valueInd))); - OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_LONG)); + OpStackTypeSet(m_curStackHt, InterpreterType(retType)); m_curStackHt++; return true; @@ -11075,7 +11077,7 @@ bool Interpreter::DoInterlockedExchangeAdd() OpStackSet(m_curStackHt, InterlockedExchangeAdd( OpStackGet(locationInd), OpStackGet(valueInd))); - OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); + OpStackTypeSet(m_curStackHt, InterpreterType(retType)); m_curStackHt++; return true; diff --git a/src/coreclr/vm/interpreter.h b/src/coreclr/vm/interpreter.h index 31310b2b03ad0c..3124951dd2219b 100644 --- a/src/coreclr/vm/interpreter.h +++ b/src/coreclr/vm/interpreter.h @@ -1795,9 +1795,9 @@ class Interpreter void DoGetIsSupported(); void DoGetArrayDataReference(); void DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method); - bool DoInterlockedCompareExchange(); - bool DoInterlockedExchange(); - bool DoInterlockedExchangeAdd(); + 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). From f35e849f8195e1a87e0a9108ea645f56cf572270 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 13 Jun 2024 09:55:37 +0200 Subject: [PATCH 8/9] Address feedback --- src/coreclr/vm/interpreter.cpp | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index 99e8d7e630fee6..908077f7ba3899 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -10924,6 +10924,25 @@ 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{ @@ -10939,7 +10958,16 @@ void Interpreter::DoIsReferenceOrContainsReferences(CORINFO_METHOD_HANDLE method } MethodTable* typeArg = GetMethodTableFromClsHnd(sigInfoFull.sigInst.methInst[0]); - OpStackSet(m_curStackHt, typeArg->IsByRefLike() || typeArg->ContainsPointers()); + + 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(m_curStackHt, containsGcPtrs); OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_INT)); m_curStackHt++; } From f8b86c31ffa34033469716a0a4aa882968207441 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 13 Jun 2024 10:06:03 +0200 Subject: [PATCH 9/9] disable TLS opt for interp --- src/coreclr/vm/threadstatics.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/vm/threadstatics.cpp b/src/coreclr/vm/threadstatics.cpp index f76b2f12739709..2be691fffb3674 100644 --- a/src/coreclr/vm/threadstatics.cpp +++ b/src/coreclr/vm/threadstatics.cpp @@ -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)