diff options
-rw-r--r-- | src/inc/corinfo.h | 2 | ||||
-rw-r--r-- | src/jit/compiler.h | 3 | ||||
-rw-r--r-- | src/jit/importer.cpp | 55 | ||||
-rw-r--r-- | src/mscorlib/model.xml | 3 | ||||
-rw-r--r-- | src/mscorlib/src/System/ByReference.cs | 12 | ||||
-rw-r--r-- | src/vm/argdestination.h | 4 | ||||
-rw-r--r-- | src/vm/jitinterface.cpp | 83 | ||||
-rw-r--r-- | src/vm/method.cpp | 13 | ||||
-rw-r--r-- | src/vm/siginfo.cpp | 4 |
9 files changed, 154 insertions, 25 deletions
diff --git a/src/inc/corinfo.h b/src/inc/corinfo.h index e899a23379..f515fcbd6d 100644 --- a/src/inc/corinfo.h +++ b/src/inc/corinfo.h @@ -988,6 +988,8 @@ enum CorInfoIntrinsics CORINFO_INTRINSIC_MemoryBarrier, CORINFO_INTRINSIC_GetCurrentManagedThread, CORINFO_INTRINSIC_GetManagedThreadId, + CORINFO_INTRINSIC_ByReference_Ctor, + CORINFO_INTRINSIC_ByReference_Value, CORINFO_INTRINSIC_Count, CORINFO_INTRINSIC_Illegal = -1, // Not a true intrinsic, diff --git a/src/jit/compiler.h b/src/jit/compiler.h index acf858a1a5..2a5c561ea5 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -2836,7 +2836,8 @@ protected: void impImportLeave(BasicBlock* block); void impResetLeaveBlock(BasicBlock* block, unsigned jmpAddr); - GenTreePtr impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, + GenTreePtr impIntrinsic(GenTreePtr newobjThis, + CORINFO_CLASS_HANDLE clsHnd, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig, int memberRef, diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index dd9f75f57d..1d9144b68f 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -1489,17 +1489,16 @@ var_types Compiler::impNormStructType(CORINFO_CLASS_HANDLE structHnd, const DWORD structFlags = info.compCompHnd->getClassAttribs(structHnd); var_types structType = TYP_STRUCT; -#ifdef FEATURE_CORECLR - const bool hasGCPtrs = (structFlags & CORINFO_FLG_CONTAINS_GC_PTR) != 0; -#else - // Desktop CLR won't report FLG_CONTAINS_GC_PTR for RefAnyClass - need to check explicitly. - const bool isRefAny = (structHnd == impGetRefAnyClass()); - const bool hasGCPtrs = isRefAny || ((structFlags & CORINFO_FLG_CONTAINS_GC_PTR) != 0); -#endif + // On coreclr the check for GC includes a "may" to account for the special + // ByRef like span structs. The added check for "CONTAINS_STACK_PTR" is the particular bit. + // When this is set the struct will contain a ByRef that could be a GC pointer or a native + // pointer. + const bool mayContainGCPtrs = + ((structFlags & CORINFO_FLG_CONTAINS_STACK_PTR) != 0 || ((structFlags & CORINFO_FLG_CONTAINS_GC_PTR) != 0)); #ifdef FEATURE_SIMD // Check to see if this is a SIMD type. - if (featureSIMD && !hasGCPtrs) + if (featureSIMD && !mayContainGCPtrs) { unsigned originalSize = info.compCompHnd->getClassSize(structHnd); @@ -1532,9 +1531,10 @@ var_types Compiler::impNormStructType(CORINFO_CLASS_HANDLE structHnd, // Verify that the quick test up above via the class attributes gave a // safe view of the type's GCness. // - // Note there are cases where hasGCPtrs is true but getClassGClayout + // Note there are cases where mayContainGCPtrs is true but getClassGClayout // does not report any gc fields. - assert(hasGCPtrs || (numGCVars == 0)); + + assert(mayContainGCPtrs || (numGCVars == 0)); if (pNumGCVars != nullptr) { @@ -3271,7 +3271,8 @@ GenTreePtr Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig) // Returns the GenTree that should be used to do the intrinsic instead of the call. // Returns NULL if an intrinsic cannot be used -GenTreePtr Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, +GenTreePtr Compiler::impIntrinsic(GenTreePtr newobjThis, + CORINFO_CLASS_HANDLE clsHnd, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig, int memberRef, @@ -3283,7 +3284,7 @@ GenTreePtr Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, #if COR_JIT_EE_VERSION > 460 CorInfoIntrinsics intrinsicID = info.compCompHnd->getIntrinsicID(method, &mustExpand); #else - CorInfoIntrinsics intrinsicID = info.compCompHnd->getIntrinsicID(method); + CorInfoIntrinsics intrinsicID = info.compCompHnd->getIntrinsicID(method); #endif *pIntrinsicID = intrinsicID; @@ -3607,7 +3608,33 @@ GenTreePtr Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, retNode = op1; break; #endif - + // Implement ByReference Ctor. This wraps the assignment of the ref into a byref-like field + // in a value type. The canonical example of this is Span<T>. In effect this is just a + // substitution. The parameter byref will be assigned into the newly allocated object. + case CORINFO_INTRINSIC_ByReference_Ctor: + { + // Remove call to constructor and directly assign the byref passed + // to the call to the first slot of the ByReference struct. + op1 = impPopStack().val; + GenTreePtr thisptr = newobjThis; + CORINFO_FIELD_HANDLE fldHnd = info.compCompHnd->getFieldInClass(clsHnd, 0); + GenTreePtr field = gtNewFieldRef(TYP_BYREF, fldHnd, thisptr, 0, false); + GenTreePtr assign = gtNewAssignNode(field, op1); + GenTreePtr byReferenceStruct = gtCloneExpr(thisptr->gtGetOp1()); + assert(byReferenceStruct != nullptr); + impPushOnStack(byReferenceStruct, typeInfo(TI_STRUCT, clsHnd)); + retNode = assign; + break; + } + // Implement ptr value getter for ByReference struct. + case CORINFO_INTRINSIC_ByReference_Value: + { + op1 = impPopStack().val; + CORINFO_FIELD_HANDLE fldHnd = info.compCompHnd->getFieldInClass(clsHnd, 0); + GenTreePtr field = gtNewFieldRef(TYP_BYREF, fldHnd, op1, 0, false); + retNode = field; + break; + } default: /* Unknown intrinsic */ break; @@ -6538,7 +6565,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // <NICE> Factor this into getCallInfo </NICE> if ((mflags & CORINFO_FLG_INTRINSIC) && !pConstrainedResolvedToken) { - call = impIntrinsic(clsHnd, methHnd, sig, pResolvedToken->token, readonlyCall, + call = impIntrinsic(newobjThis, clsHnd, methHnd, sig, pResolvedToken->token, readonlyCall, (canTailCall && (tailCall != 0)), &intrinsicID); if (call != nullptr) diff --git a/src/mscorlib/model.xml b/src/mscorlib/model.xml index 75846ae612..0f34b34fbf 100644 --- a/src/mscorlib/model.xml +++ b/src/mscorlib/model.xml @@ -12719,5 +12719,8 @@ <Member MemberType="Field" Name="Never" /> <Member MemberType="Field" Name="Advanced" /> </Type> + <Type Status="ImplRoot" Name="System.Runtime.CompilerServices.Unsafe"> + <Member Status="ImplRoot" Name="AsPointer<T>(T@)" /> + </Type> </Assembly> </ThinModel> diff --git a/src/mscorlib/src/System/ByReference.cs b/src/mscorlib/src/System/ByReference.cs index 6f8bb2281e..833dab0d55 100644 --- a/src/mscorlib/src/System/ByReference.cs +++ b/src/mscorlib/src/System/ByReference.cs @@ -15,16 +15,20 @@ namespace System public ByReference(ref T value) { - // TODO-SPAN: This has GC hole. It needs to be JIT intrinsic instead - unsafe { _value = (IntPtr)Unsafe.AsPointer(ref value); } + // Implemented as a JIT intrinsic - This default implementation is for + // completeness and to provide a concrete error if called via reflection + // or if intrinsic is missed. + throw new System.PlatformNotSupportedException(); } public ref T Value { get { - // TODO-SPAN: This has GC hole. It needs to be JIT intrinsic instead - unsafe { return ref Unsafe.As<IntPtr, T>(ref *(IntPtr*)_value); } + // Implemented as a JIT intrinsic - This default implementation is for + // completeness and to provide a concrete error if called via reflection + // or if the intrinsic is missed. + throw new System.PlatformNotSupportedException(); } } } diff --git a/src/vm/argdestination.h b/src/vm/argdestination.h index d8f6c854b2..e90217f93a 100644 --- a/src/vm/argdestination.h +++ b/src/vm/argdestination.h @@ -176,8 +176,10 @@ public: { LIMITED_METHOD_CONTRACT; + // SPAN-TODO: GC reporting - https://github.com/dotnet/coreclr/issues/8517 + _ASSERTE(IsStructPassedInRegs()); - + TADDR genRegDest = dac_cast<TADDR>(GetStructGenRegDestinationAddress()); INDEBUG(int remainingBytes = fieldBytes;) diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp index 9d57836bac..ee22e723e9 100644 --- a/src/vm/jitinterface.cpp +++ b/src/vm/jitinterface.cpp @@ -2286,6 +2286,58 @@ BOOL CEEInfo::checkMethodModifier(CORINFO_METHOD_HANDLE hMethod, } /*********************************************************************/ +static unsigned ComputeGCLayout(MethodTable * pMT, BYTE* gcPtrs) +{ + STANDARD_VM_CONTRACT; + + unsigned result = 0; + + _ASSERTE(pMT->IsValueType()); + + ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); + for (FieldDesc *pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) + { + int fieldStartIndex = pFD->GetOffset() / sizeof(void*); + + if (pFD->GetFieldType() != ELEMENT_TYPE_VALUETYPE) + { + if (pFD->IsObjRef()) + { + if (gcPtrs[fieldStartIndex] == TYPE_GC_NONE) + { + gcPtrs[fieldStartIndex] = TYPE_GC_REF; + result++; + } + else if (gcPtrs[fieldStartIndex] != TYPE_GC_REF) + { + COMPlusThrowHR(COR_E_BADIMAGEFORMAT); + } + } + } + else + { + MethodTable * pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable(); + if (pFieldMT->HasSameTypeDefAs(g_pByReferenceClass)) + { + if (gcPtrs[fieldStartIndex] == TYPE_GC_NONE) + { + gcPtrs[fieldStartIndex] = TYPE_GC_BYREF; + result++; + } + else if (gcPtrs[fieldStartIndex] != TYPE_GC_BYREF) + { + COMPlusThrowHR(COR_E_BADIMAGEFORMAT); + } + } + else + { + result += ComputeGCLayout(pFieldMT, gcPtrs + fieldStartIndex); + } + } + } + return result; +} + unsigned CEEInfo::getClassGClayout (CORINFO_CLASS_HANDLE clsHnd, BYTE* gcPtrs) { CONTRACTL { @@ -2313,10 +2365,12 @@ unsigned CEEInfo::getClassGClayout (CORINFO_CLASS_HANDLE clsHnd, BYTE* gcPtrs) } else { - // TODO-SPAN: Proper GC reporting memset(gcPtrs, TYPE_GC_NONE, - (VMClsHnd.GetSize() + sizeof(void*) -1)/ sizeof(void*)); - result = 0; + (VMClsHnd.GetSize() + sizeof(void*) - 1) / sizeof(void*)); + // Note: This case is more complicated than the TypedReference case + // due to ByRefLike structs being included as fields in other value + // types (TypedReference can not be.) + result = ComputeGCLayout(VMClsHnd.AsMethodTable(), gcPtrs); } } else if (VMClsHnd.IsNativeValueType()) @@ -8764,6 +8818,29 @@ CorInfoIntrinsics CEEInfo::getIntrinsicID(CORINFO_METHOD_HANDLE methodHnd, { result = ECall::GetIntrinsicID(method); } + else + { + MethodTable * pMT = method->GetMethodTable(); + if (pMT->IsByRefLike() && pMT->GetModule()->IsSystem()) + { + if (pMT->HasSameTypeDefAs(g_pByReferenceClass)) + { + // ByReference<T> has just two methods: constructor and Value property + if (method->IsCtor()) + { + result = CORINFO_INTRINSIC_ByReference_Ctor; + } + else + { + _ASSERTE(strcmp(method->GetName(), "get_Value") == 0); + result = CORINFO_INTRINSIC_ByReference_Value; + } + *pMustExpand = true; + } + + // TODO-SPAN: Span<T> intrinsics for optimizations + } + } EE_TO_JIT_TRANSITION(); diff --git a/src/vm/method.cpp b/src/vm/method.cpp index 7afe0e9de2..07636d6950 100644 --- a/src/vm/method.cpp +++ b/src/vm/method.cpp @@ -2466,7 +2466,18 @@ MethodDesc* Entry2MethodDesc(PCODE entryPoint, MethodTable *pMT) BOOL MethodDesc::IsFCallOrIntrinsic() { WRAPPER_NO_CONTRACT; - return (IsFCall() || IsArray()); + + if (IsFCall() || IsArray()) + return TRUE; + +#ifdef FEATURE_SPAN_OF_T + // Intrinsic methods on ByReference<T> or Span<T> + MethodTable * pMT = GetMethodTable(); + if (pMT->IsByRefLike() && pMT->GetModule()->IsSystem()) + return TRUE; +#endif + + return FALSE; } //******************************************************************************* diff --git a/src/vm/siginfo.cpp b/src/vm/siginfo.cpp index 9adfb4998c..12803ec062 100644 --- a/src/vm/siginfo.cpp +++ b/src/vm/siginfo.cpp @@ -4956,7 +4956,9 @@ void PromoteCarefully(promote_func fn, void ReportPointersFromValueType(promote_func *fn, ScanContext *sc, PTR_MethodTable pMT, PTR_VOID pSrc) { WRAPPER_NO_CONTRACT; - + + // SPAN-TODO: GC reporting - https://github.com/dotnet/coreclr/issues/8517 + if (!pMT->ContainsPointers()) return; |