From da7b00b4fb57321773e5a8f5bed1efcc46108307 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 25 Feb 2023 09:35:45 -0800 Subject: [PATCH 01/30] initial port from the prototype --- src/coreclr/inc/corinfo.h | 2 +- .../tools/Common/JitInterface/CorInfoImpl.cs | 12 +++ .../tools/Common/JitInterface/CorInfoTypes.cs | 2 +- .../Common/MetadataFieldLayoutAlgorithm.cs | 45 +++++++++ .../Utilities/GCPointerMap.Algorithm.cs | 44 +++++++-- src/coreclr/vm/class.h | 12 ++- src/coreclr/vm/jitinterface.cpp | 4 + src/coreclr/vm/methodtablebuilder.cpp | 96 ++++++++++++++----- src/coreclr/vm/methodtablebuilder.h | 1 + 9 files changed, 181 insertions(+), 37 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 28531e00bee5f5..b405a6edfaa17c 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -852,7 +852,7 @@ enum CorInfoFlag CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently) CORINFO_FLG_OVERLAPPING_FIELDS = 0x00100000, // struct or class has fields that overlap (aka union) CORINFO_FLG_INTERFACE = 0x00200000, // it is an interface - CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't ask field info, AOT can't rely on it (used for types outside of AOT compilation version bubble) + CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't ask field info (used for types outside of AOT compilation version bubble and value arrays) CORINFO_FLG_CUSTOMLAYOUT = 0x00800000, // does this struct have custom layout? CORINFO_FLG_CONTAINS_GC_PTR = 0x01000000, // does the class contain a gc ptr ? CORINFO_FLG_DELEGATE = 0x02000000, // is this a subclass of delegate or multicast delegate ? diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index fbb17f280b5e56..90004d1fda84c9 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2043,6 +2043,18 @@ private uint getClassAttribsInternal(TypeDesc type) result |= CorInfoFlag.CORINFO_FLG_ABSTRACT; } + // TODO: VS disable struct promotion for valArr + //if (type is InstantiatedType it) + //{ + // if (it.Name == "ValueArray`2" && it.Namespace == "System") + // { + // if (it.Instantiation[1] is ArrayType arr && arr.Rank > 1) + // { + // result |= CorInfoFlag.CORINFO_FLG_DONT_DIG_FIELDS; + // } + // } + //} + #if READYTORUN if (!_compilation.CompilationModuleGroup.VersionsWithType(type)) { diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index a3d920666ed494..f1b57271461b50 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -602,7 +602,7 @@ public enum CorInfoFlag : uint CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently) CORINFO_FLG_OVERLAPPING_FIELDS = 0x00100000, // struct or class has fields that overlap (aka union) CORINFO_FLG_INTERFACE = 0x00200000, // it is an interface - CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't try to ask about fields outside of AOT compilation version bubble + CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't try to ask about fields outside of AOT compilation version bubble or value arrays CORINFO_FLG_CUSTOMLAYOUT = 0x00800000, // does this struct have custom layout? CORINFO_FLG_CONTAINS_GC_PTR = 0x01000000, // does the class contain a gc ptr ? CORINFO_FLG_DELEGATE = 0x02000000, // is this a subclass of delegate or multicast delegate ? diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index f3b7b9442f5bbe..645efdd873c4b4 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -358,6 +358,28 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty layoutMetadata.Size, out instanceByteSizeAndAlignment); + // TODO: VS adjust size + //if (type is InstantiatedType it) + //{ + // if (it.Name == "ValueArray`2" && it.Namespace == "System") + // { + // if (it.Instantiation[1] is ArrayType arr) + // { + // int repeat = arr.Rank; + + // if (!instanceSizeAndAlignment.Size.IsIndeterminate) + // { + // instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); + // } + + // if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) + // { + // instanceByteSizeAndAlignment.Size = new LayoutInt(instanceByteSizeAndAlignment.Size.AsInt * repeat); + // } + // } + // } + //} + ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout { IsAutoLayoutOrHasAutoLayoutFields = hasAutoLayoutField, @@ -746,6 +768,29 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, classLayoutSize: 0, byteCount: out instanceByteSizeAndAlignment); + // TODO: VS adjust size + //if (type is InstantiatedType it) + //{ + // if (it.Name == "ValueArray`2" && it.Namespace == "System") + // { + // if (it.Instantiation[1] is ArrayType arr) + // { + // int repeat = arr.Rank; + + // if (!instanceSizeAndAlignment.Size.IsIndeterminate) + // { + // instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); + // } + + // if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) + // { + // instanceByteSizeAndAlignment.Size = new LayoutInt(instanceByteSizeAndAlignment.Size.AsInt * repeat); + // } + // } + // } + //} + + ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout { IsAutoLayoutOrHasAutoLayoutFields = true, diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs index eca01c4b3a4587..28eb4a25eef12d 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs @@ -14,21 +14,36 @@ public static GCPointerMap FromInstanceLayout(DefType type) { Debug.Assert(type.ContainsGCPointers); - GCPointerMapBuilder builder = new GCPointerMapBuilder(type.InstanceByteCount.AsInt, type.Context.Target.PointerSize); - FromInstanceLayoutHelper(ref builder, type); + int pointerSize = type.Context.Target.PointerSize; + GCPointerMapBuilder builder = new GCPointerMapBuilder(type.InstanceByteCount.AsInt, pointerSize); + FromInstanceLayoutHelper(ref builder, type, pointerSize); return builder.ToGCMap(); } - private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, DefType type) + private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, DefType type, int pointerSize) { if (!type.IsValueType && type.HasBaseType) { DefType baseType = type.BaseType; GCPointerMapBuilder baseLayoutBuilder = builder.GetInnerBuilder(0, baseType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref baseLayoutBuilder, baseType); + FromInstanceLayoutHelper(ref baseLayoutBuilder, baseType, pointerSize); } + int repeat = 1; + + //TODO: VS compute repeat + //if (type is InstantiatedType it) + //{ + // if (it.Name == "ValueArray`2" && it.Namespace == "System") + // { + // if (it.Instantiation[1] is ArrayType arr) + // { + // repeat = arr.Rank; + // } + // } + //} + foreach (FieldDesc field in type.GetFields()) { if (field.IsStatic) @@ -37,16 +52,23 @@ private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, De TypeDesc fieldType = field.FieldType; if (fieldType.IsGCPointer) { - builder.MarkGCPointer(field.Offset.AsInt); + for (int i = 0; i < repeat; i++) + { + builder.MarkGCPointer(field.Offset.AsInt + pointerSize * i); + } } else if (fieldType.IsValueType) { var fieldDefType = (DefType)fieldType; if (fieldDefType.ContainsGCPointers) { - GCPointerMapBuilder innerBuilder = - builder.GetInnerBuilder(field.Offset.AsInt, fieldDefType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref innerBuilder, fieldDefType); + for (int i = 0; i < repeat; i++) + { + int fieldSize = fieldDefType.InstanceByteCount.AsInt; + GCPointerMapBuilder innerBuilder = + builder.GetInnerBuilder(field.Offset.AsInt + fieldSize * i, fieldSize); + FromInstanceLayoutHelper(ref innerBuilder, fieldDefType, pointerSize); + } } } } @@ -76,9 +98,10 @@ public static GCPointerMap FromStaticLayout(DefType type) var fieldDefType = (DefType)fieldType; if (fieldDefType.ContainsGCPointers) { + int pointerSize = type.Context.Target.PointerSize; GCPointerMapBuilder innerBuilder = builder.GetInnerBuilder(field.Offset.AsInt, fieldDefType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref innerBuilder, fieldDefType); + FromInstanceLayoutHelper(ref innerBuilder, fieldDefType, pointerSize); } } } @@ -109,9 +132,10 @@ public static GCPointerMap FromThreadStaticLayout(DefType type) var fieldDefType = (DefType)fieldType; if (fieldDefType.ContainsGCPointers) { + int pointerSize = type.Context.Target.PointerSize; GCPointerMapBuilder innerBuilder = builder.GetInnerBuilder(field.Offset.AsInt, fieldDefType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref innerBuilder, fieldDefType); + FromInstanceLayoutHelper(ref innerBuilder, fieldDefType, pointerSize); } } } diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 22ef2b9919cc19..0ee0e5352dfb38 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -1384,6 +1384,16 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! LIMITED_METHOD_CONTRACT; m_VMFlags |= (DWORD)VMFLAG_HAS_FIELDS_WHICH_MUST_BE_INITED; } + BOOL HasValueArrayFlagSet() + { + LIMITED_METHOD_CONTRACT; + return (m_VMFlags & VMFLAG_VALUE_ARRAY); + } + void SetValueArrayFlag() + { + LIMITED_METHOD_CONTRACT; + m_VMFlags |= (DWORD)VMFLAG_VALUE_ARRAY; + } void SetCannotBeBlittedByObjectCloner() { /* no op */ @@ -1723,7 +1733,7 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! VMFLAG_BESTFITMAPPING = 0x00004000, // BestFitMappingAttribute.Value VMFLAG_THROWONUNMAPPABLECHAR = 0x00008000, // BestFitMappingAttribute.ThrowOnUnmappableChar - // unused = 0x00010000, + VMFLAG_VALUE_ARRAY = 0x00010000, VMFLAG_NO_GUID = 0x00020000, VMFLAG_HASNONPUBLICFIELDS = 0x00040000, VMFLAG_HAS_CUSTOM_FIELD_ALIGNMENT = 0x00080000, diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 8e888542fecfdc..30d7145bc8020c 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -3552,6 +3552,10 @@ uint32_t CEEInfo::getClassAttribsInternal (CORINFO_CLASS_HANDLE clsHnd) } if (pClass->HasExplicitFieldOffsetLayout() && pClass->HasOverlaidField()) ret |= CORINFO_FLG_OVERLAPPING_FIELDS; + + if (pClass->HasValueArrayFlagSet()) + ret |= CORINFO_FLG_DONT_DIG_FIELDS; + if (VMClsHnd.IsCanonicalSubtype()) ret |= CORINFO_FLG_SHAREDINST; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 35a7ddb0360a6d..9b8cfa1853eb00 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1694,6 +1694,25 @@ MethodTableBuilder::BuildMethodTableThrowing( &pByValueClassCache, bmtMFDescs, bmtFP, &totalDeclaredFieldSize); + // TODO: VS detect valArr + //if (!bmtGenericsInfo->fContainsGenericVariables && + // g_pValueArrayClass != NULL && + // g_pValueArrayClass->GetCl() == GetCl() && + // g_pValueArrayClass->GetModule() == bmtInternal->pType->GetModule()) + //{ + // TypeHandle lengthMarker = bmtGenericsInfo->GetInstantiation()[1]; + // if (lengthMarker.IsArray()) + // { + // DWORD rank = lengthMarker.GetRank(); + // if (rank > 1) + // { + // bmtFP->NumValueArrayElements = rank; + // // there is no scenario when tearing a value array into pieces is desirable. + // GetHalfBakedClass()->SetValueArrayFlag(); + // } + // } + //} + // Place regular static fields PlaceRegularStaticFields(); @@ -1713,6 +1732,11 @@ MethodTableBuilder::BuildMethodTableThrowing( _ASSERTE(HasLayout()); + if (bmtFP->NumValueArrayElements) + { + GetLayoutInfo()->m_cbManagedSize *= bmtFP->NumValueArrayElements; + } + bmtFP->NumInstanceFieldBytes = GetLayoutInfo()->m_cbManagedSize; // For simple Blittable types we still need to check if they have any overlapping @@ -8318,6 +8342,16 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable ** pByValueClassCach BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); } + if (bmtFP->NumValueArrayElements > 0) + { + dwNumInstanceFieldBytes *= bmtFP->NumValueArrayElements; + + if (pFieldDescList[0].IsByValue()) + { + dwNumGCPointerSeries *= bmtFP->NumValueArrayElements; + } + } + bmtFP->NumInstanceFieldBytes = dwNumInstanceFieldBytes; bmtFP->NumGCPointerSeries = dwNumGCPointerSeries; @@ -11494,12 +11528,19 @@ VOID MethodTableBuilder::HandleGCForValueClasses(MethodTable ** pByValueClassCac } + DWORD repeat = 1; + if (bmtFP->NumValueArrayElements > 0) + { + _ASSERTE(bmtEnumFields->dwNumInstanceFields == 1); + repeat = bmtFP->NumValueArrayElements; + } + // Build the pointer series map for this pointers in this instance pSeries = ((CGCDesc*)pMT)->GetLowestSeries(); if (bmtFP->NumInstanceGCPointerFields) { // See gcdesc.h for an explanation of why we adjust by subtracting BaseSize - pSeries->SetSeriesSize( (size_t) (bmtFP->NumInstanceGCPointerFields * TARGET_POINTER_SIZE) - (size_t) pMT->GetBaseSize()); + pSeries->SetSeriesSize((size_t)(bmtFP->NumInstanceGCPointerFields * repeat * TARGET_POINTER_SIZE) - (size_t)pMT->GetBaseSize()); pSeries->SetSeriesOffset(bmtFP->GCPointerFieldStart + OBJECT_SIZE); pSeries++; } @@ -11509,44 +11550,51 @@ VOID MethodTableBuilder::HandleGCForValueClasses(MethodTable ** pByValueClassCac { if (pFieldDescList[i].IsByValue()) { - MethodTable *pByValueMT = pByValueClassCache[i]; + MethodTable* pByValueMT = pByValueClassCache[i]; if (pByValueMT->ContainsPointers()) { // Offset of the by value class in the class we are building, does NOT include Object DWORD dwCurrentOffset = pFieldDescList[i].GetOffset_NoLogging(); + DWORD dwElementSize = pByValueMT->GetBaseSize() - OBJECT_BASESIZE; - // The by value class may have more than one pointer series - CGCDescSeries * pByValueSeries = CGCDesc::GetCGCDescFromMT(pByValueMT)->GetLowestSeries(); - SIZE_T dwNumByValueSeries = CGCDesc::GetCGCDescFromMT(pByValueMT)->GetNumSeries(); - - for (SIZE_T j = 0; j < dwNumByValueSeries; j++) + for (DWORD r = 0; r < repeat; r++) { - size_t cbSeriesSize; - size_t cbSeriesOffset; + // The by value class may have more than one pointer series + CGCDescSeries* pByValueSeries = CGCDesc::GetCGCDescFromMT(pByValueMT)->GetLowestSeries(); + SIZE_T dwNumByValueSeries = CGCDesc::GetCGCDescFromMT(pByValueMT)->GetNumSeries(); - _ASSERTE(pSeries <= CGCDesc::GetCGCDescFromMT(pMT)->GetHighestSeries()); + for (SIZE_T j = 0; j < dwNumByValueSeries; j++) + { + size_t cbSeriesSize; + size_t cbSeriesOffset; + + _ASSERTE(pSeries <= CGCDesc::GetCGCDescFromMT(pMT)->GetHighestSeries()); - cbSeriesSize = pByValueSeries->GetSeriesSize(); + cbSeriesSize = pByValueSeries->GetSeriesSize(); - // Add back the base size of the by value class, since it's being transplanted to this class - cbSeriesSize += pByValueMT->GetBaseSize(); + // Add back the base size of the by value class, since it's being transplanted to this class + cbSeriesSize += pByValueMT->GetBaseSize(); - // Subtract the base size of the class we're building - cbSeriesSize -= pMT->GetBaseSize(); + // Subtract the base size of the class we're building + cbSeriesSize -= pMT->GetBaseSize(); - // Set current series we're building - pSeries->SetSeriesSize(cbSeriesSize); + // Set current series we're building + pSeries->SetSeriesSize(cbSeriesSize); - // Get offset into the value class of the first pointer field (includes a +Object) - cbSeriesOffset = pByValueSeries->GetSeriesOffset(); + // Get offset into the value class of the first pointer field (includes a +Object) + cbSeriesOffset = pByValueSeries->GetSeriesOffset(); - // Add it to the offset of the by value class in our class - cbSeriesOffset += dwCurrentOffset; + // Add element N offset + cbSeriesOffset += r * dwElementSize; - pSeries->SetSeriesOffset(cbSeriesOffset); // Offset of field - pSeries++; - pByValueSeries++; + // Add it to the offset of the by value class in our class + cbSeriesOffset += dwCurrentOffset; + + pSeries->SetSeriesOffset(cbSeriesOffset); // Offset of field + pSeries++; + pByValueSeries++; + } } } } diff --git a/src/coreclr/vm/methodtablebuilder.h b/src/coreclr/vm/methodtablebuilder.h index 739a8e530f1320..e1361707051ce7 100644 --- a/src/coreclr/vm/methodtablebuilder.h +++ b/src/coreclr/vm/methodtablebuilder.h @@ -2011,6 +2011,7 @@ class MethodTableBuilder DWORD NumInstanceGCPointerFields; // does not include inherited pointer fields DWORD NumGCPointerSeries; DWORD NumInstanceFieldBytes; + DWORD NumValueArrayElements; bool fIsByRefLikeType; bool fHasFixedAddressValueTypes; From 170a306a29aaafa3f9504b7e3f3e89a3454dea5e Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 25 Feb 2023 18:44:52 -0800 Subject: [PATCH 02/30] parse the attribute in MT builder --- src/coreclr/vm/methodtablebuilder.cpp | 61 ++++++++++++++++++--------- src/coreclr/vm/siginfo.cpp | 8 +++- src/coreclr/vm/wellknownattributes.h | 3 ++ 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 9b8cfa1853eb00..8dc637b8fe61fa 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1694,24 +1694,44 @@ MethodTableBuilder::BuildMethodTableThrowing( &pByValueClassCache, bmtMFDescs, bmtFP, &totalDeclaredFieldSize); - // TODO: VS detect valArr - //if (!bmtGenericsInfo->fContainsGenericVariables && - // g_pValueArrayClass != NULL && - // g_pValueArrayClass->GetCl() == GetCl() && - // g_pValueArrayClass->GetModule() == bmtInternal->pType->GetModule()) - //{ - // TypeHandle lengthMarker = bmtGenericsInfo->GetInstantiation()[1]; - // if (lengthMarker.IsArray()) - // { - // DWORD rank = lengthMarker.GetRank(); - // if (rank > 1) - // { - // bmtFP->NumValueArrayElements = rank; - // // there is no scenario when tearing a value array into pieces is desirable. - // GetHalfBakedClass()->SetValueArrayFlag(); - // } - // } - //} + if (IsValueClass()) + { + const void* pVal; // The custom value. + ULONG cbVal; // Size of the custom value. + HRESULT hr = GetCustomAttribute(bmtInternal->pType->GetTypeDefToken(), + WellKnownAttribute::InlineArrayAttribute, + &pVal, &cbVal); + + if (hr != S_FALSE) + { + if (bmtEnumFields->dwNumInstanceFields != 1) + { + // TODO: diagnostics, must have one field. + BuildMethodTableThrowException(IDS_CLASSLOAD_GENERAL); + } + + if (cbVal >= (sizeof(INT32) + 2)) + { + INT32 repeat = GET_UNALIGNED_VAL32((byte*)pVal + 2); + if (repeat > 0) + { + bmtFP->NumValueArrayElements = repeat; + GetHalfBakedClass()->SetValueArrayFlag(); + } + else + { + // TODO: diagnostics, repeat must be > 0 + BuildMethodTableThrowException(IDS_CLASSLOAD_GENERAL); + } + + if (HasExplicitFieldOffsetLayout()) + { + // TODO: diagnostics, must not have explicit offsets + BuildMethodTableThrowException(IDS_CLASSLOAD_GENERAL); + } + } + } + } // Place regular static fields PlaceRegularStaticFields(); @@ -8342,8 +8362,9 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable ** pByValueClassCach BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); } - if (bmtFP->NumValueArrayElements > 0) + if (bmtFP->NumValueArrayElements > 1) { + // TODO: VS validate that size < FIELD_OFFSET_LAST_REAL_OFFSET dwNumInstanceFieldBytes *= bmtFP->NumValueArrayElements; if (pFieldDescList[0].IsByValue()) @@ -11529,7 +11550,7 @@ VOID MethodTableBuilder::HandleGCForValueClasses(MethodTable ** pByValueClassCac } DWORD repeat = 1; - if (bmtFP->NumValueArrayElements > 0) + if (bmtFP->NumValueArrayElements > 1) { _ASSERTE(bmtEnumFields->dwNumInstanceFields == 1); repeat = bmtFP->NumValueArrayElements; diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index 0a15fbb91488fe..3815ae4f70c31f 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -4846,6 +4846,9 @@ class ByRefPointerOffsetsReporter _ASSERTE(pMT != nullptr); _ASSERTE(pMT->IsByRefLike()); + int repeat = 1; // TODO: VS fetch val arr repeat + UINT repeatSize = 0; // TODO: VS fetch actual repeat size + ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); for (FieldDesc* pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) { @@ -4859,7 +4862,10 @@ class ByRefPointerOffsetsReporter } else if (pFD->IsByRef()) { - Report(baseOffset + pFD->GetOffset()); + for (int i = 0; i < repeat; i++) + { + Report(baseOffset + (repeatSize * i) + pFD->GetOffset()); + } } } } diff --git a/src/coreclr/vm/wellknownattributes.h b/src/coreclr/vm/wellknownattributes.h index f1ac8a69767505..837f75c2ac083b 100644 --- a/src/coreclr/vm/wellknownattributes.h +++ b/src/coreclr/vm/wellknownattributes.h @@ -36,6 +36,7 @@ enum class WellKnownAttribute : DWORD WinRTMarshalingBehaviorAttribute, PreserveBaseOverridesAttribute, ObjectiveCTrackedTypeAttribute, + InlineArrayAttribute, CountOfWellKnownAttributes }; @@ -104,6 +105,8 @@ inline const char *GetWellKnownAttributeName(WellKnownAttribute attribute) return "System.Runtime.CompilerServices.PreserveBaseOverridesAttribute"; case WellKnownAttribute::ObjectiveCTrackedTypeAttribute: return "System.Runtime.InteropServices.ObjectiveC.ObjectiveCTrackedTypeAttribute"; + case WellKnownAttribute::InlineArrayAttribute: + return "System.Runtime.CompilerServices.InlineArrayAttribute"; case WellKnownAttribute::CountOfWellKnownAttributes: default: break; // Silence compiler warnings From bb1c3db399f3eeedbb0ec020d4cd8c5084e18ecd Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 25 Feb 2023 18:56:36 -0800 Subject: [PATCH 03/30] validate replicated size --- src/coreclr/vm/methodtablebuilder.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 8dc637b8fe61fa..84c15a5a4c0eb3 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -8364,8 +8364,12 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable ** pByValueClassCach if (bmtFP->NumValueArrayElements > 1) { - // TODO: VS validate that size < FIELD_OFFSET_LAST_REAL_OFFSET - dwNumInstanceFieldBytes *= bmtFP->NumValueArrayElements; + INT64 extendedSize = (INT64)dwNumInstanceFieldBytes * (INT64)bmtFP->NumValueArrayElements; + if (extendedSize > FIELD_OFFSET_LAST_REAL_OFFSET) { + BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); + } + + dwNumInstanceFieldBytes = (DWORD)extendedSize; if (pFieldDescList[0].IsByValue()) { From 0a4cd782d542a3963a78bf439dc7af0f7f2598fd Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sun, 26 Feb 2023 11:01:39 -0800 Subject: [PATCH 04/30] aot changes --- .../tools/Common/JitInterface/CorInfoImpl.cs | 15 +---- .../Common/InstantiatedType.Metadata.cs | 5 ++ .../TypeSystem/Common/InstantiatedType.cs | 3 + .../Common/MetadataFieldLayoutAlgorithm.cs | 65 +++++++------------ .../Common/TypeSystem/Common/MetadataType.cs | 7 ++ .../Common/TypeSystem/Common/TypeDesc.cs | 11 ++++ .../Common/TypeSystem/Common/TypeFlags.cs | 7 +- .../Utilities/GCPointerMap.Algorithm.cs | 16 ++--- .../tools/Common/TypeSystem/Ecma/EcmaType.cs | 20 ++++++ .../ReadyToRun/GCRefMapBuilder.cs | 23 ++++++- src/coreclr/vm/siginfo.cpp | 39 +++++++---- 11 files changed, 127 insertions(+), 84 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 90004d1fda84c9..064a1a60fba26f 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2027,6 +2027,9 @@ private uint getClassAttribsInternal(TypeDesc type) if (type.IsIntrinsic) result |= CorInfoFlag.CORINFO_FLG_INTRINSIC_TYPE; + if (type.IsValueArray) + result |= CorInfoFlag.CORINFO_FLG_DONT_DIG_FIELDS; + if (metadataType != null) { if (metadataType.ContainsGCPointers) @@ -2043,18 +2046,6 @@ private uint getClassAttribsInternal(TypeDesc type) result |= CorInfoFlag.CORINFO_FLG_ABSTRACT; } - // TODO: VS disable struct promotion for valArr - //if (type is InstantiatedType it) - //{ - // if (it.Name == "ValueArray`2" && it.Namespace == "System") - // { - // if (it.Instantiation[1] is ArrayType arr && arr.Rank > 1) - // { - // result |= CorInfoFlag.CORINFO_FLG_DONT_DIG_FIELDS; - // } - // } - //} - #if READYTORUN if (!_compilation.CompilationModuleGroup.VersionsWithType(type)) { diff --git a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs index d917b342e56a4b..13c82bb314bba5 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs @@ -86,6 +86,11 @@ public override bool HasCustomAttribute(string attributeNamespace, string attrib return _typeDef.HasCustomAttribute(attributeNamespace, attributeName); } + public override int GetValueArrayLength() + { + return _typeDef.GetValueArrayLength(); + } + public override MetadataType GetNestedType(string name) { // Return the result from the typical type definition. diff --git a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs index 0df6089d42320d..ab97f2a9b415e5 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs @@ -106,6 +106,9 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask) if (_typeDef.IsByRefLike) flags |= TypeFlags.IsByRefLike; + if (_typeDef.IsValueArray) + flags |= TypeFlags.IsValueArray; + AddComputedIntrinsicFlag(ref flags); } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 645efdd873c4b4..bafdcf50304e8b 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; - using Debug = System.Diagnostics.Debug; namespace Internal.TypeSystem @@ -358,27 +357,11 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty layoutMetadata.Size, out instanceByteSizeAndAlignment); - // TODO: VS adjust size - //if (type is InstantiatedType it) - //{ - // if (it.Name == "ValueArray`2" && it.Namespace == "System") - // { - // if (it.Instantiation[1] is ArrayType arr) - // { - // int repeat = arr.Rank; - - // if (!instanceSizeAndAlignment.Size.IsIndeterminate) - // { - // instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); - // } - - // if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) - // { - // instanceByteSizeAndAlignment.Size = new LayoutInt(instanceByteSizeAndAlignment.Size.AsInt * repeat); - // } - // } - // } - //} + // value array cannot have explicit layout + if(type.IsValueArray) + { + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); + } ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout { @@ -768,28 +751,24 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, classLayoutSize: 0, byteCount: out instanceByteSizeAndAlignment); - // TODO: VS adjust size - //if (type is InstantiatedType it) - //{ - // if (it.Name == "ValueArray`2" && it.Namespace == "System") - // { - // if (it.Instantiation[1] is ArrayType arr) - // { - // int repeat = arr.Rank; - - // if (!instanceSizeAndAlignment.Size.IsIndeterminate) - // { - // instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); - // } - - // if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) - // { - // instanceByteSizeAndAlignment.Size = new LayoutInt(instanceByteSizeAndAlignment.Size.AsInt * repeat); - // } - // } - // } - //} + if (type.IsValueArray) + { + int repeat = type.GetValueArrayLength(); + + // UNDONE: VS validate resulting size. + // UNDONE: VS validate repeat > 0. + + if (!instanceSizeAndAlignment.Size.IsIndeterminate) + { + instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); + } + + if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) + { + instanceByteSizeAndAlignment.Size = new LayoutInt(instanceByteSizeAndAlignment.Size.AsInt * repeat); + } + } ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout { diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs index 72b0aeadba86c4..eff3f996d83f54 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; namespace Internal.TypeSystem { @@ -90,6 +91,12 @@ public virtual bool IsModuleType /// doesn't exist. /// public abstract MetadataType GetNestedType(string name); + + public virtual int GetValueArrayLength() + { + Debug.Assert(this.IsValueArray); + return 1; + } } public struct ClassLayoutMetadata diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs index ae83551e2bd0e3..8600be5ab7438d 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs @@ -676,6 +676,17 @@ public bool IsByRefLike } } + /// + /// Gets a value indicating whether this is a value array type + /// + public bool IsValueArray + { + get + { + return (GetTypeFlags(TypeFlags.IsValueArray | TypeFlags.AttributeCacheComputed) & TypeFlags.IsValueArray) != 0; + } + } + /// /// Gets a value indicating whether this type implements IDynamicInterfaceCastable /// diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeFlags.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeFlags.cs index 6331f50a891b28..526ea27a197f3e 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeFlags.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeFlags.cs @@ -57,10 +57,11 @@ public enum TypeFlags HasFinalizer = 0x2000, IsByRefLike = 0x04000, - AttributeCacheComputed = 0x08000, + IsValueArray = 0x08000, IsIntrinsic = 0x10000, + AttributeCacheComputed = 0x20000, - IsIDynamicInterfaceCastable = 0x20000, - IsIDynamicInterfaceCastableComputed = 0x40000, + IsIDynamicInterfaceCastable = 0x40000, + IsIDynamicInterfaceCastableComputed = 0x80000, } } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs index 28eb4a25eef12d..bfac78495a22fb 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs @@ -31,18 +31,10 @@ private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, De } int repeat = 1; - - //TODO: VS compute repeat - //if (type is InstantiatedType it) - //{ - // if (it.Name == "ValueArray`2" && it.Namespace == "System") - // { - // if (it.Instantiation[1] is ArrayType arr) - // { - // repeat = arr.Rank; - // } - // } - //} + if (type.IsValueArray) + { + repeat = ((MetadataType)type).GetValueArrayLength(); + } foreach (FieldDesc field in type.GetFields()) { diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs index 348b2f6e79c5a8..332b8dd758e068 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Reflection; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; @@ -260,6 +261,13 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask) if (stringComparer.Equals(nameHandle, "IntrinsicAttribute") && stringComparer.Equals(namespaceHandle, "System.Runtime.CompilerServices")) flags |= TypeFlags.IsIntrinsic; + + if (isValueType && + stringComparer.Equals(nameHandle, "InlineArrayAttribute") && + stringComparer.Equals(namespaceHandle, "System.Runtime.CompilerServices")) + { + flags |= TypeFlags.IsValueArray; + } } } } @@ -525,6 +533,18 @@ public override bool HasCustomAttribute(string attributeNamespace, string attrib attributeNamespace, attributeName).IsNil; } + public override int GetValueArrayLength() + { + Debug.Assert(this.IsValueArray); + + var attr = MetadataReader.GetCustomAttribute(MetadataReader.GetCustomAttributeHandle(_typeDefinition.GetCustomAttributes(), + "System.Runtime.CompilerServices", "InlineArrayAttribute")); + + var value = attr.DecodeValue(new CustomAttributeTypeProvider(_module)).FixedArguments[0].Value; + + return value is int intValue ? intValue : 0; + } + public override ClassLayoutMetadata GetClassLayout() { TypeLayout layout = _typeDefinition.GetLayout(); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs index 8f9478897a7464..86d079b1c5e704 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs @@ -4,7 +4,8 @@ using System; using System.Collections.Generic; using System.Diagnostics; - +using System.Linq; +using System.Xml.Linq; using Internal.TypeSystem; // The GCRef map is used to encode GC type of arguments for callsites. Logically, it is sequence where pos is @@ -280,12 +281,30 @@ private void GcScanValueType(TypeDesc type, in ArgDestination argDest, int delta Debug.Assert(type is DefType); DefType defType = (DefType)type; + bool isValArray = defType.IsValueArray; foreach (FieldDesc field in defType.GetFields()) { if (field.IsStatic) continue; - GcScanRoots(field.FieldType, in argDest, delta + field.Offset.AsInt, frame, topLevel: false); + if (isValArray) + { + var elementSize = field.FieldType.GetElementSize().AsInt; + var totalSize = defType.InstanceFieldSize.AsInt; + + for (int offset = 0; offset < totalSize; offset += elementSize) + { + GcScanRoots(field.FieldType, in argDest, delta + offset, frame, topLevel: false); + } + + // there is only one formal instance field in a value array + Debug.Assert(field.Offset.AsInt == 0); + break; + } + else + { + GcScanRoots(field.FieldType, in argDest, delta + field.Offset.AsInt, frame, topLevel: false); + } } } diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index 3815ae4f70c31f..d8c325b792393a 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -4840,32 +4840,47 @@ class ByRefPointerOffsetsReporter WRAPPER_NO_CONTRACT; } + void Find(FieldDesc* pFD, SIZE_T baseOffset) + { + if (pFD->GetFieldType() == ELEMENT_TYPE_VALUETYPE) + { + PTR_MethodTable pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable(); + if (pFieldMT->IsByRefLike()) + { + Find(pFieldMT, baseOffset + pFD->GetOffset()); + } + } + else if (pFD->IsByRef()) + { + Report(baseOffset + pFD->GetOffset()); + } + } + void Find(PTR_MethodTable pMT, SIZE_T baseOffset) { WRAPPER_NO_CONTRACT; _ASSERTE(pMT != nullptr); _ASSERTE(pMT->IsByRefLike()); - int repeat = 1; // TODO: VS fetch val arr repeat - UINT repeatSize = 0; // TODO: VS fetch actual repeat size - + bool isValArray = pMT->GetClass()->HasValueArrayFlagSet(); ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); for (FieldDesc* pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) { - if (pFD->GetFieldType() == ELEMENT_TYPE_VALUETYPE) + if(isValArray) { - PTR_MethodTable pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable(); - if (pFieldMT->IsByRefLike()) + DWORD elementSize = pFD->GetSize(); + DWORD totalSize = pMT->GetNumInstanceFieldBytes(); + for (DWORD offset = 0; offset < totalSize; offset += elementSize) { - Find(pFieldMT, baseOffset + pFD->GetOffset()); + Find(pFD, baseOffset + offset); } + + // there is only one instance field + break; } - else if (pFD->IsByRef()) + else { - for (int i = 0; i < repeat; i++) - { - Report(baseOffset + (repeatSize * i) + pFD->GetOffset()); - } + Find(pFD, baseOffset); } } } From 6eeb887f40f48819b7f5a403554ff8826a010e07 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 27 Feb 2023 14:31:17 -0800 Subject: [PATCH 05/30] refmap --- src/coreclr/vm/jitinterface.cpp | 50 ++++++++++++++++++++++++--------- src/coreclr/vm/siginfo.cpp | 4 +-- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 30d7145bc8020c..2e2f6fc05c0970 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -2162,31 +2162,53 @@ static unsigned MarkGCField(BYTE* gcPtrs, CorInfoGCType type) return 0; } -/*********************************************************************/ -static unsigned ComputeGCLayout(MethodTable * pMT, BYTE* gcPtrs) +static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs); + +static unsigned ComputeGCLayout(FieldDesc* pFD, BYTE* gcPtrs) +{ + unsigned result = 0; + if (pFD->GetFieldType() == ELEMENT_TYPE_VALUETYPE) + { + MethodTable* pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable(); + result += ComputeGCLayout(pFieldMT, gcPtrs); + } + else if (pFD->IsObjRef()) + { + result += MarkGCField(gcPtrs, TYPE_GC_REF); + } + else if (pFD->IsByRef()) + { + result += MarkGCField(gcPtrs, TYPE_GC_BYREF); + } + + return result; +} + +static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs) { STANDARD_VM_CONTRACT; _ASSERTE(pMT->IsValueType()); unsigned result = 0; + bool isValueArray = pMT->GetClass()->HasValueArrayFlagSet(); ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); for (FieldDesc *pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) { - int fieldStartIndex = pFD->GetOffset() / TARGET_POINTER_SIZE; - - if (pFD->GetFieldType() == ELEMENT_TYPE_VALUETYPE) + if (isValueArray) { - MethodTable * pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable(); - result += ComputeGCLayout(pFieldMT, gcPtrs + fieldStartIndex); - } - else if (pFD->IsObjRef()) - { - result += MarkGCField(gcPtrs + fieldStartIndex, TYPE_GC_REF); + _ASSERTE(pFD->GetOffset() == 0); + DWORD totalSize = pMT->GetNumInstanceFieldBytes(); + DWORD elementSize = pFD->GetSize(); + for (DWORD offset = 0; offset < totalSize; offset += elementSize) + { + result += ComputeGCLayout(pFD, gcPtrs + (offset / TARGET_POINTER_SIZE)); + } } - else if (pFD->IsByRef()) + else { - result += MarkGCField(gcPtrs + fieldStartIndex, TYPE_GC_BYREF); + int fieldStartIndex = pFD->GetOffset() / TARGET_POINTER_SIZE; + result += ComputeGCLayout(pFD, gcPtrs + fieldStartIndex); } } return result; @@ -2229,7 +2251,7 @@ unsigned CEEInfo::getClassGClayoutStatic(TypeHandle VMClsHnd, BYTE* gcPtrs) { memset(gcPtrs, TYPE_GC_NONE, (VMClsHnd.GetSize() + TARGET_POINTER_SIZE - 1) / TARGET_POINTER_SIZE); - // ByRefLike structs can be included as fields in other value types. + // ByRefLike structs can contain byref fields or other ByRefLike structs result = ComputeGCLayout(VMClsHnd.AsMethodTable(), gcPtrs); } else diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index d8c325b792393a..8d3a60c245370c 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -4868,15 +4868,13 @@ class ByRefPointerOffsetsReporter { if(isValArray) { + _ASSERTE(pFD->GetOffset() == 0); DWORD elementSize = pFD->GetSize(); DWORD totalSize = pMT->GetNumInstanceFieldBytes(); for (DWORD offset = 0; offset < totalSize; offset += elementSize) { Find(pFD, baseOffset + offset); } - - // there is only one instance field - break; } else { From f0d156b921246367c0dcedae833b60ad2219451b Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 27 Feb 2023 14:47:27 -0800 Subject: [PATCH 06/30] validate total size in ilc --- .../Common/MetadataFieldLayoutAlgorithm.cs | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index bafdcf50304e8b..3ce2fc0eb6bedf 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -755,18 +755,30 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, { int repeat = type.GetValueArrayLength(); - // UNDONE: VS validate resulting size. - // UNDONE: VS validate repeat > 0. - - - if (!instanceSizeAndAlignment.Size.IsIndeterminate) + if (repeat <= 0) { - instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); } if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) { - instanceByteSizeAndAlignment.Size = new LayoutInt(instanceByteSizeAndAlignment.Size.AsInt * repeat); + long size = instanceByteSizeAndAlignment.Size.AsInt; + size *= repeat; + + // matching the max size validation in MethodTableBuilder + // see: FIELD_OFFSET_LAST_REAL_OFFSET + const int maxSize = ((1 << 27) - 1) - 6; + if (size > maxSize) + { + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadValueClassTooLarge, type); + } + + instanceByteSizeAndAlignment.Size = new LayoutInt((int)size); + } + + if (!instanceSizeAndAlignment.Size.IsIndeterminate) + { + instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); } } From 470aee2c7535cdc8934639d0f9969b62f2c4ab0c Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 27 Feb 2023 16:05:49 -0800 Subject: [PATCH 07/30] add the actual attribute --- .../System.Private.CoreLib.Shared.projitems | 1 + .../CompilerServices/InlineArrayAttribute.cs | 19 +++++++++++++++++++ .../System.Runtime/ref/System.Runtime.cs | 7 +++++++ 3 files changed, 27 insertions(+) create mode 100644 src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 6f2d52f360447d..35dbe01116bbcd 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -808,6 +808,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs new file mode 100644 index 00000000000000..eb966ca090d5b0 --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel; + +namespace System.Runtime.CompilerServices +{ + [EditorBrowsable(EditorBrowsableState.Never)] + [AttributeUsage(AttributeTargets.Struct, AllowMultiple = false)] + public sealed class InlineArrayAttribute : Attribute + { + public InlineArrayAttribute(int length) + { + Length = length; + } + + public int Length { get; } + } +} diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 53f5e047b232a0..6a18583fdb46da 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -12498,6 +12498,13 @@ public sealed partial class InterpolatedStringHandlerAttribute : System.Attribut { public InterpolatedStringHandlerAttribute() { } } + [AttributeUsage(System.AttributeTargets.Struct, AllowMultiple = false)] + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] + public sealed partial class InlineArrayAttribute : System.Attribute + { + public InlineArrayAttribute(int length) { } + public int Length { get { throw null; } } + } [System.AttributeUsageAttribute(System.AttributeTargets.Struct)] [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)] public sealed partial class IsByRefLikeAttribute : System.Attribute From 78ab52308dfd12c8f81ad6bd886d6383853e98e3 Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Tue, 28 Feb 2023 08:38:06 -0800 Subject: [PATCH 08/30] Apply suggestions from code review Co-authored-by: Jan Kotas --- .../Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 3ce2fc0eb6bedf..fe679ec28802c6 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -358,7 +358,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty out instanceByteSizeAndAlignment); // value array cannot have explicit layout - if(type.IsValueArray) + if (type.IsValueArray) { ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); } From a452be746842901fa748ca1daf67552c195b44c2 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 28 Feb 2023 15:51:31 -0800 Subject: [PATCH 09/30] add a small use of InlineArray in CoreLib - to get some crossgen coverage. --- .../src/System/ParamsArray.cs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/ParamsArray.cs b/src/libraries/System.Private.CoreLib/src/System/ParamsArray.cs index 8e9d35f6b0076b..a4b5fd9bcdff2b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ParamsArray.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ParamsArray.cs @@ -7,8 +7,48 @@ // Suppress warnings for unused private fields #pragma warning disable CS0169, CA1823, IDE0051, IDE0044 +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; + namespace System { +#if CORECLR || NATIVEAOT + + [InlineArray(Length)] + internal struct TwoObjects + { + private const int Length = 2; + internal object? Arg0; + + [UnscopedRef] + private ref object? this[int i] => ref Unsafe.Add(ref Arg0, i); + + public TwoObjects(object? arg0, object? arg1) + { + this[0] = arg0; + this[1] = arg1; + } + } + + [InlineArray(Length)] + internal struct ThreeObjects + { + private const int Length = 3; + internal object? Arg0; + + [UnscopedRef] + private ref object? this[int i] => ref Unsafe.Add(ref Arg0, i); + + public ThreeObjects(object? arg0, object? arg1, object? arg2) + { + this[0] = arg0; + this[1] = arg1; + this[2] = arg2; + } + } + +#else + internal struct TwoObjects { internal object? Arg0; @@ -34,4 +74,5 @@ public ThreeObjects(object? arg0, object? arg1, object? arg2) _arg2 = arg2; } } +#endif } From 2801f508efcb1be0dd99a5ab038cd4a0d21693e6 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 1 Mar 2023 03:23:17 -0800 Subject: [PATCH 10/30] a few more uses in CorLib --- .../Reflection/Emit/DynamicMethod.CoreCLR.cs | 4 +- .../Reflection/RuntimeMethodInfo.CoreCLR.cs | 4 +- .../src/System/Reflection/MethodBase.cs | 37 ++++++++++++------- .../Reflection/RuntimeConstructorInfo.cs | 8 ++-- .../System/Reflection/RuntimeMethodInfo.cs | 4 +- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs index 2e4c5d034933e8..9633a784519a33 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs @@ -150,8 +150,8 @@ Signature LazyCreateSignature() { Debug.Assert(parameters != null); StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0, argCount); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0, argCount); + Span copyOfParameters = new(ref argStorage._arg0._arg0, argCount); + Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, argCount); StackAllocatedByRefs byrefStorage = default; #pragma warning disable CS8500 diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs index 3ed1028e564c06..5afcf0d87fddea 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs @@ -311,9 +311,9 @@ public override MethodImplAttributes GetMethodImplementationFlags() unsafe { StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0, 1); + Span copyOfParameters = new(ref argStorage._arg0._arg0, 1); ReadOnlySpan parameters = new(in parameter); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0, 1); + Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, 1); StackAllocatedByRefs byrefStorage = default; #pragma warning disable 8500 diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs index bd25e409799d90..82a5ff83c10467 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs @@ -274,6 +274,21 @@ BindingFlags invokeAttr internal const int MaxStackAllocArgCount = 4; +#if CORECLR + [InlineArray(MaxStackAllocArgCount)] +#endif + private protected struct ArgumentData + { + internal T _arg0; +#if !CORECLR +#pragma warning disable CA1823, CS0169, IDE0051, IDE0044 // accessed via 'CheckArguments' ref arithmetic + private object? _arg1; + private object? _arg2; + private object? _arg3; +#pragma warning restore CA1823, CS0169, IDE0051, IDE0044 +#endif + } + // Helper struct to avoid intermediate object[] allocation in calls to the native reflection stack. // When argument count <= MaxStackAllocArgCount, define a local of type default(StackAllocatedByRefs) // and pass it to CheckArguments(). @@ -282,31 +297,25 @@ BindingFlags invokeAttr [StructLayout(LayoutKind.Sequential)] private protected ref struct StackAllocedArguments { - internal object? _arg0; -#pragma warning disable CA1823, CS0169, IDE0051, IDE0044 // accessed via 'CheckArguments' ref arithmetic - private object? _arg1; - private object? _arg2; - private object? _arg3; -#pragma warning restore CA1823, CS0169, IDE0051, IDE0044 - internal ParameterCopyBackAction _copyBack0; -#pragma warning disable CA1823, CS0169, IDE0051, IDE0044 // accessed via 'CheckArguments' ref arithmetic - private ParameterCopyBackAction _copyBack1; - private ParameterCopyBackAction _copyBack2; - private ParameterCopyBackAction _copyBack3; -#pragma warning restore CA1823, CS0169, IDE0051, IDE0044 + internal ArgumentData _arg0; + internal ArgumentData _copyBack0; } // Helper struct to avoid intermediate IntPtr[] allocation and RegisterForGCReporting in calls to the native reflection stack. - [StructLayout(LayoutKind.Sequential)] +#if CORECLR + [InlineArray(MaxStackAllocArgCount)] +#endif private protected ref struct StackAllocatedByRefs { internal ref byte _arg0; +#if !CORECLR #pragma warning disable CA1823, CS0169, IDE0051 // accessed via 'CheckArguments' ref arithmetic private ref byte _arg1; private ref byte _arg2; private ref byte _arg3; #pragma warning restore CA1823, CS0169, IDE0051 +#endif } #endif - } + } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs index a16f6f48531ae9..e096965c66b23c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -142,8 +142,8 @@ internal void ThrowNoInvokeException() { Debug.Assert(parameters != null); StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0, argCount); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0, argCount); + Span copyOfParameters = new(ref argStorage._arg0._arg0, argCount); + Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, argCount); StackAllocatedByRefs byrefStorage = default; #pragma warning disable 8500 @@ -289,8 +289,8 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] { Debug.Assert(parameters != null); StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0, argCount); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0, argCount); + Span copyOfParameters = new(ref argStorage._arg0._arg0, argCount); + Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, argCount); StackAllocatedByRefs byrefStorage = default; #pragma warning disable 8500 diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs index 28d530bcfc6c20..cb87a6969ca483 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs @@ -139,8 +139,8 @@ private void ThrowNoInvokeException() { Debug.Assert(parameters != null); StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0, argCount); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0, argCount); + Span copyOfParameters = new(ref argStorage._arg0._arg0, argCount); + Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, argCount); StackAllocatedByRefs byrefStorage = default; #pragma warning disable 8500 From e3284d19222f95e5f2ed0161f743615eb343f6c1 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 1 Mar 2023 03:30:04 -0800 Subject: [PATCH 11/30] fix for sequential layout --- .../tools/Common/JitInterface/CorInfoImpl.cs | 16 +++++ .../Common/MetadataFieldLayoutAlgorithm.cs | 64 +++++++++++-------- 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 064a1a60fba26f..d479058ed15672 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2246,6 +2246,7 @@ private int MarkGcField(byte* gcPtrs, CorInfoGCType gcType) private int GatherClassGCLayout(TypeDesc type, byte* gcPtrs) { int result = 0; + bool isValueArray = type.IsValueArray; foreach (var field in type.GetFields()) { @@ -2287,6 +2288,21 @@ private int GatherClassGCLayout(TypeDesc type, byte* gcPtrs) { result += MarkGcField(fieldGcPtrs, gcType); } + + if (isValueArray) + { + Debug.Assert(field.Offset.AsInt == 0); + int totalLayoutSize = type.GetElementSize().AsInt / PointerSize; + int elementLayoutSize = fieldType.GetElementSize().AsInt / PointerSize; + for (int offset = elementLayoutSize; offset < totalLayoutSize; offset += elementLayoutSize) + { + Buffer.MemoryCopy(gcPtrs, gcPtrs + offset, elementLayoutSize, elementLayoutSize); + result += elementLayoutSize; + } + + // value array has only one element field + break; + } } return result; } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index fe679ec28802c6..501c25faea0077 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -434,6 +434,11 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType layoutMetadata.Size, out instanceByteSizeAndAlignment); + if (type.IsValueArray) + { + AdjustForValueArray(type, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); + } + ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout { IsAutoLayoutOrHasAutoLayoutFields = hasAutoLayoutField, @@ -449,6 +454,37 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType return computedLayout; } + private static void AdjustForValueArray(MetadataType type, ref SizeAndAlignment instanceByteSizeAndAlignment, ref SizeAndAlignment instanceSizeAndAlignment) + { + int repeat = type.GetValueArrayLength(); + + if (repeat <= 0) + { + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); + } + + if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) + { + long size = instanceByteSizeAndAlignment.Size.AsInt; + size *= repeat; + + // matching the max size validation in MethodTableBuilder + // see: FIELD_OFFSET_LAST_REAL_OFFSET + const int maxSize = ((1 << 27) - 1) - 6; + if (size > maxSize) + { + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadValueClassTooLarge, type); + } + + instanceByteSizeAndAlignment.Size = new LayoutInt((int)size); + } + + if (!instanceSizeAndAlignment.Size.IsIndeterminate) + { + instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); + } + } + protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8, bool requiresAlignedBase) { } @@ -753,33 +789,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, if (type.IsValueArray) { - int repeat = type.GetValueArrayLength(); - - if (repeat <= 0) - { - ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); - } - - if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) - { - long size = instanceByteSizeAndAlignment.Size.AsInt; - size *= repeat; - - // matching the max size validation in MethodTableBuilder - // see: FIELD_OFFSET_LAST_REAL_OFFSET - const int maxSize = ((1 << 27) - 1) - 6; - if (size > maxSize) - { - ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadValueClassTooLarge, type); - } - - instanceByteSizeAndAlignment.Size = new LayoutInt((int)size); - } - - if (!instanceSizeAndAlignment.Size.IsIndeterminate) - { - instanceSizeAndAlignment.Size = new LayoutInt(instanceSizeAndAlignment.Size.AsInt * repeat); - } + AdjustForValueArray(type, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); } ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout From 4ff0a254c27d48dc8b5f72d33fc9fbbe9b8ed068 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 2 Mar 2023 13:24:44 -0800 Subject: [PATCH 12/30] Standardize on use of "InlineArray" in the implementation --- .../tools/Common/JitInterface/CorInfoImpl.cs | 6 +++--- .../Common/InstantiatedType.Metadata.cs | 4 ++-- .../TypeSystem/Common/InstantiatedType.cs | 4 ++-- .../Common/MetadataFieldLayoutAlgorithm.cs | 14 +++++++------- .../Common/TypeSystem/Common/MetadataType.cs | 4 ++-- .../tools/Common/TypeSystem/Common/TypeDesc.cs | 4 ++-- .../Common/TypeSystem/Common/TypeFlags.cs | 2 +- .../Common/Utilities/GCPointerMap.Algorithm.cs | 4 ++-- .../tools/Common/TypeSystem/Ecma/EcmaType.cs | 6 +++--- .../ReadyToRun/GCRefMapBuilder.cs | 4 ++-- src/coreclr/vm/class.h | 10 +++++----- src/coreclr/vm/jitinterface.cpp | 6 +++--- src/coreclr/vm/methodtablebuilder.cpp | 18 +++++++++--------- src/coreclr/vm/methodtablebuilder.h | 2 +- src/coreclr/vm/siginfo.cpp | 2 +- 15 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index d479058ed15672..5d266b8c64d162 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2027,7 +2027,7 @@ private uint getClassAttribsInternal(TypeDesc type) if (type.IsIntrinsic) result |= CorInfoFlag.CORINFO_FLG_INTRINSIC_TYPE; - if (type.IsValueArray) + if (type.IsInlineArray) result |= CorInfoFlag.CORINFO_FLG_DONT_DIG_FIELDS; if (metadataType != null) @@ -2246,7 +2246,7 @@ private int MarkGcField(byte* gcPtrs, CorInfoGCType gcType) private int GatherClassGCLayout(TypeDesc type, byte* gcPtrs) { int result = 0; - bool isValueArray = type.IsValueArray; + bool isInlineArray = type.IsInlineArray; foreach (var field in type.GetFields()) { @@ -2289,7 +2289,7 @@ private int GatherClassGCLayout(TypeDesc type, byte* gcPtrs) result += MarkGcField(fieldGcPtrs, gcType); } - if (isValueArray) + if (isInlineArray) { Debug.Assert(field.Offset.AsInt == 0); int totalLayoutSize = type.GetElementSize().AsInt / PointerSize; diff --git a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs index 13c82bb314bba5..1c642b46ebfa7c 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.Metadata.cs @@ -86,9 +86,9 @@ public override bool HasCustomAttribute(string attributeNamespace, string attrib return _typeDef.HasCustomAttribute(attributeNamespace, attributeName); } - public override int GetValueArrayLength() + public override int GetInlineArrayLength() { - return _typeDef.GetValueArrayLength(); + return _typeDef.GetInlineArrayLength(); } public override MetadataType GetNestedType(string name) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs index ab97f2a9b415e5..7dc5e9ee3c3282 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs @@ -106,8 +106,8 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask) if (_typeDef.IsByRefLike) flags |= TypeFlags.IsByRefLike; - if (_typeDef.IsValueArray) - flags |= TypeFlags.IsValueArray; + if (_typeDef.IsInlineArray) + flags |= TypeFlags.IsInlineArray; AddComputedIntrinsicFlag(ref flags); } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 501c25faea0077..5bb0cd320f3904 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -358,7 +358,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty out instanceByteSizeAndAlignment); // value array cannot have explicit layout - if (type.IsValueArray) + if (type.IsInlineArray) { ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); } @@ -434,9 +434,9 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType layoutMetadata.Size, out instanceByteSizeAndAlignment); - if (type.IsValueArray) + if (type.IsInlineArray) { - AdjustForValueArray(type, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); + AdjustForInlineArray(type, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); } ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout @@ -454,9 +454,9 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType return computedLayout; } - private static void AdjustForValueArray(MetadataType type, ref SizeAndAlignment instanceByteSizeAndAlignment, ref SizeAndAlignment instanceSizeAndAlignment) + private static void AdjustForInlineArray(MetadataType type, ref SizeAndAlignment instanceByteSizeAndAlignment, ref SizeAndAlignment instanceSizeAndAlignment) { - int repeat = type.GetValueArrayLength(); + int repeat = type.GetInlineArrayLength(); if (repeat <= 0) { @@ -787,9 +787,9 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, classLayoutSize: 0, byteCount: out instanceByteSizeAndAlignment); - if (type.IsValueArray) + if (type.IsInlineArray) { - AdjustForValueArray(type, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); + AdjustForInlineArray(type, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); } ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs index eff3f996d83f54..98ab4cecb4cfdd 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs @@ -92,9 +92,9 @@ public virtual bool IsModuleType /// public abstract MetadataType GetNestedType(string name); - public virtual int GetValueArrayLength() + public virtual int GetInlineArrayLength() { - Debug.Assert(this.IsValueArray); + Debug.Assert(this.IsInlineArray); return 1; } } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs index 8600be5ab7438d..77f474667a5b33 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs @@ -679,11 +679,11 @@ public bool IsByRefLike /// /// Gets a value indicating whether this is a value array type /// - public bool IsValueArray + public bool IsInlineArray { get { - return (GetTypeFlags(TypeFlags.IsValueArray | TypeFlags.AttributeCacheComputed) & TypeFlags.IsValueArray) != 0; + return (GetTypeFlags(TypeFlags.IsInlineArray | TypeFlags.AttributeCacheComputed) & TypeFlags.IsInlineArray) != 0; } } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeFlags.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeFlags.cs index 526ea27a197f3e..2f5ced2caf73f0 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeFlags.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeFlags.cs @@ -57,7 +57,7 @@ public enum TypeFlags HasFinalizer = 0x2000, IsByRefLike = 0x04000, - IsValueArray = 0x08000, + IsInlineArray = 0x08000, IsIntrinsic = 0x10000, AttributeCacheComputed = 0x20000, diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs index bfac78495a22fb..f1c794d1455555 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs @@ -31,9 +31,9 @@ private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, De } int repeat = 1; - if (type.IsValueArray) + if (type.IsInlineArray) { - repeat = ((MetadataType)type).GetValueArrayLength(); + repeat = ((MetadataType)type).GetInlineArrayLength(); } foreach (FieldDesc field in type.GetFields()) diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs index 332b8dd758e068..36944af4eab8f6 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs @@ -266,7 +266,7 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask) stringComparer.Equals(nameHandle, "InlineArrayAttribute") && stringComparer.Equals(namespaceHandle, "System.Runtime.CompilerServices")) { - flags |= TypeFlags.IsValueArray; + flags |= TypeFlags.IsInlineArray; } } } @@ -533,9 +533,9 @@ public override bool HasCustomAttribute(string attributeNamespace, string attrib attributeNamespace, attributeName).IsNil; } - public override int GetValueArrayLength() + public override int GetInlineArrayLength() { - Debug.Assert(this.IsValueArray); + Debug.Assert(this.IsInlineArray); var attr = MetadataReader.GetCustomAttribute(MetadataReader.GetCustomAttributeHandle(_typeDefinition.GetCustomAttributes(), "System.Runtime.CompilerServices", "InlineArrayAttribute")); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs index 86d079b1c5e704..7d919b67272784 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs @@ -281,13 +281,13 @@ private void GcScanValueType(TypeDesc type, in ArgDestination argDest, int delta Debug.Assert(type is DefType); DefType defType = (DefType)type; - bool isValArray = defType.IsValueArray; + bool isInlineArray = defType.IsInlineArray; foreach (FieldDesc field in defType.GetFields()) { if (field.IsStatic) continue; - if (isValArray) + if (isInlineArray) { var elementSize = field.FieldType.GetElementSize().AsInt; var totalSize = defType.InstanceFieldSize.AsInt; diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 0ee0e5352dfb38..74554d12d34274 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -1384,15 +1384,15 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! LIMITED_METHOD_CONTRACT; m_VMFlags |= (DWORD)VMFLAG_HAS_FIELDS_WHICH_MUST_BE_INITED; } - BOOL HasValueArrayFlagSet() + BOOL HasInlineArrayFlagSet() { LIMITED_METHOD_CONTRACT; - return (m_VMFlags & VMFLAG_VALUE_ARRAY); + return (m_VMFlags & VMFLAG_INLINE_ARRAY); } - void SetValueArrayFlag() + void SetInlineArrayFlag() { LIMITED_METHOD_CONTRACT; - m_VMFlags |= (DWORD)VMFLAG_VALUE_ARRAY; + m_VMFlags |= (DWORD)VMFLAG_INLINE_ARRAY; } void SetCannotBeBlittedByObjectCloner() { @@ -1733,7 +1733,7 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! VMFLAG_BESTFITMAPPING = 0x00004000, // BestFitMappingAttribute.Value VMFLAG_THROWONUNMAPPABLECHAR = 0x00008000, // BestFitMappingAttribute.ThrowOnUnmappableChar - VMFLAG_VALUE_ARRAY = 0x00010000, + VMFLAG_INLINE_ARRAY = 0x00010000, VMFLAG_NO_GUID = 0x00020000, VMFLAG_HASNONPUBLICFIELDS = 0x00040000, VMFLAG_HAS_CUSTOM_FIELD_ALIGNMENT = 0x00080000, diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 2e2f6fc05c0970..659e7fc4cac5d0 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -2191,11 +2191,11 @@ static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs) _ASSERTE(pMT->IsValueType()); unsigned result = 0; - bool isValueArray = pMT->GetClass()->HasValueArrayFlagSet(); + bool isInlineArray = pMT->GetClass()->HasInlineArrayFlagSet(); ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); for (FieldDesc *pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) { - if (isValueArray) + if (isInlineArray) { _ASSERTE(pFD->GetOffset() == 0); DWORD totalSize = pMT->GetNumInstanceFieldBytes(); @@ -3575,7 +3575,7 @@ uint32_t CEEInfo::getClassAttribsInternal (CORINFO_CLASS_HANDLE clsHnd) if (pClass->HasExplicitFieldOffsetLayout() && pClass->HasOverlaidField()) ret |= CORINFO_FLG_OVERLAPPING_FIELDS; - if (pClass->HasValueArrayFlagSet()) + if (pClass->HasInlineArrayFlagSet()) ret |= CORINFO_FLG_DONT_DIG_FIELDS; if (VMClsHnd.IsCanonicalSubtype()) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 84c15a5a4c0eb3..4505bca531b371 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1715,8 +1715,8 @@ MethodTableBuilder::BuildMethodTableThrowing( INT32 repeat = GET_UNALIGNED_VAL32((byte*)pVal + 2); if (repeat > 0) { - bmtFP->NumValueArrayElements = repeat; - GetHalfBakedClass()->SetValueArrayFlag(); + bmtFP->NumInlineArrayElements = repeat; + GetHalfBakedClass()->SetInlineArrayFlag(); } else { @@ -1752,9 +1752,9 @@ MethodTableBuilder::BuildMethodTableThrowing( _ASSERTE(HasLayout()); - if (bmtFP->NumValueArrayElements) + if (bmtFP->NumInlineArrayElements) { - GetLayoutInfo()->m_cbManagedSize *= bmtFP->NumValueArrayElements; + GetLayoutInfo()->m_cbManagedSize *= bmtFP->NumInlineArrayElements; } bmtFP->NumInstanceFieldBytes = GetLayoutInfo()->m_cbManagedSize; @@ -8362,9 +8362,9 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable ** pByValueClassCach BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); } - if (bmtFP->NumValueArrayElements > 1) + if (bmtFP->NumInlineArrayElements > 1) { - INT64 extendedSize = (INT64)dwNumInstanceFieldBytes * (INT64)bmtFP->NumValueArrayElements; + INT64 extendedSize = (INT64)dwNumInstanceFieldBytes * (INT64)bmtFP->NumInlineArrayElements; if (extendedSize > FIELD_OFFSET_LAST_REAL_OFFSET) { BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); } @@ -8373,7 +8373,7 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable ** pByValueClassCach if (pFieldDescList[0].IsByValue()) { - dwNumGCPointerSeries *= bmtFP->NumValueArrayElements; + dwNumGCPointerSeries *= bmtFP->NumInlineArrayElements; } } @@ -11554,10 +11554,10 @@ VOID MethodTableBuilder::HandleGCForValueClasses(MethodTable ** pByValueClassCac } DWORD repeat = 1; - if (bmtFP->NumValueArrayElements > 1) + if (bmtFP->NumInlineArrayElements > 1) { _ASSERTE(bmtEnumFields->dwNumInstanceFields == 1); - repeat = bmtFP->NumValueArrayElements; + repeat = bmtFP->NumInlineArrayElements; } // Build the pointer series map for this pointers in this instance diff --git a/src/coreclr/vm/methodtablebuilder.h b/src/coreclr/vm/methodtablebuilder.h index e1361707051ce7..0064be068fcfd9 100644 --- a/src/coreclr/vm/methodtablebuilder.h +++ b/src/coreclr/vm/methodtablebuilder.h @@ -2011,7 +2011,7 @@ class MethodTableBuilder DWORD NumInstanceGCPointerFields; // does not include inherited pointer fields DWORD NumGCPointerSeries; DWORD NumInstanceFieldBytes; - DWORD NumValueArrayElements; + DWORD NumInlineArrayElements; bool fIsByRefLikeType; bool fHasFixedAddressValueTypes; diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index 8d3a60c245370c..d333cfee9e249a 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -4862,7 +4862,7 @@ class ByRefPointerOffsetsReporter _ASSERTE(pMT != nullptr); _ASSERTE(pMT->IsByRefLike()); - bool isValArray = pMT->GetClass()->HasValueArrayFlagSet(); + bool isValArray = pMT->GetClass()->HasInlineArrayFlagSet(); ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); for (FieldDesc* pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) { From b406dfeb6f62439195e4297d2b35e30bcf4a8762 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 2 Mar 2023 13:46:13 -0800 Subject: [PATCH 13/30] simpler layout replication --- src/coreclr/vm/jitinterface.cpp | 55 ++++++++++++++------------------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 659e7fc4cac5d0..7b13040a0711dd 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -2162,28 +2162,6 @@ static unsigned MarkGCField(BYTE* gcPtrs, CorInfoGCType type) return 0; } -static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs); - -static unsigned ComputeGCLayout(FieldDesc* pFD, BYTE* gcPtrs) -{ - unsigned result = 0; - if (pFD->GetFieldType() == ELEMENT_TYPE_VALUETYPE) - { - MethodTable* pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable(); - result += ComputeGCLayout(pFieldMT, gcPtrs); - } - else if (pFD->IsObjRef()) - { - result += MarkGCField(gcPtrs, TYPE_GC_REF); - } - else if (pFD->IsByRef()) - { - result += MarkGCField(gcPtrs, TYPE_GC_BYREF); - } - - return result; -} - static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs) { STANDARD_VM_CONTRACT; @@ -2195,20 +2173,35 @@ static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs) ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); for (FieldDesc *pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) { + int fieldStartIndex = pFD->GetOffset() / TARGET_POINTER_SIZE; + + if (pFD->GetFieldType() == ELEMENT_TYPE_VALUETYPE) + { + MethodTable* pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable(); + result += ComputeGCLayout(pFieldMT, gcPtrs + fieldStartIndex); + } + else if (pFD->IsObjRef()) + { + result += MarkGCField(gcPtrs + fieldStartIndex, TYPE_GC_REF); + } + else if (pFD->IsByRef()) + { + result += MarkGCField(gcPtrs + fieldStartIndex, TYPE_GC_BYREF); + } + if (isInlineArray) { _ASSERTE(pFD->GetOffset() == 0); - DWORD totalSize = pMT->GetNumInstanceFieldBytes(); - DWORD elementSize = pFD->GetSize(); - for (DWORD offset = 0; offset < totalSize; offset += elementSize) + DWORD totalLayoutSize = pMT->GetNumInstanceFieldBytes() / TARGET_POINTER_SIZE; + DWORD elementLayoutSize = pFD->GetSize() / TARGET_POINTER_SIZE; + for (DWORD offset = elementLayoutSize; offset < totalLayoutSize; offset += elementLayoutSize) { - result += ComputeGCLayout(pFD, gcPtrs + (offset / TARGET_POINTER_SIZE)); + memcpy(gcPtrs + offset, gcPtrs, elementLayoutSize); + result += elementLayoutSize; } - } - else - { - int fieldStartIndex = pFD->GetOffset() / TARGET_POINTER_SIZE; - result += ComputeGCLayout(pFD, gcPtrs + fieldStartIndex); + + // value array has only one element field + break; } } return result; From 7f197d699c81f449085ac5e237fcdc7b573ab492 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 2 Mar 2023 19:10:53 -0800 Subject: [PATCH 14/30] some initial tests (will add more) --- .../InlineArray/InlineArrayInvalid.cs | 113 ++++++++++++++++++ .../InlineArray/InlineArrayInvalid.csproj | 13 ++ .../InlineArray/InlineArrayValid.cs | 25 ++++ .../InlineArray/InlineArrayValid.csproj | 13 ++ src/tests/issues.targets | 4 + 5 files changed, 168 insertions(+) create mode 100644 src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs create mode 100644 src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.csproj create mode 100644 src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs create mode 100644 src/tests/Loader/classloader/InlineArray/InlineArrayValid.csproj diff --git a/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs b/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs new file mode 100644 index 00000000000000..09bff64d16b995 --- /dev/null +++ b/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs @@ -0,0 +1,113 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +using Xunit; + +unsafe class Validate +{ + [InlineArray(1)] + [StructLayout(LayoutKind.Explicit)] + private struct Explicit + { + [FieldOffset(0)] + public Guid Guid; + } + + [Fact] + public static void Explicit_Fails() + { + Console.WriteLine($"{nameof(Explicit_Fails)}..."); + Assert.Throws(() => { var t = typeof(Explicit); }); + + Assert.Throws(() => + { + return sizeof(Explicit); + }); + } + + [InlineArray(0)] + private struct ZeroLength + { + public int field; + } + + [Fact] + public static void ZeroLength_Fails() + { + Console.WriteLine($"{nameof(ZeroLength_Fails)}..."); + Assert.Throws(() => { var t = typeof(ZeroLength); }); + + Assert.Throws(() => + { + var t = new ZeroLength() + { + field = 1 + }; + return t; + }); + } + + [InlineArray(0x20000000)] + private struct TooLarge + { + public long field; + } + + [Fact] + public static void TooLarge_Fails() + { + Console.WriteLine($"{nameof(TooLarge_Fails)}..."); + Assert.Throws(() => { var t = typeof(TooLarge); }); + + Assert.Throws(() => + { + var t = new TooLarge() + { + field = 1 + }; + return t; + }); + } + + [InlineArray(123)] + private struct NoFields + { + public static int x; + } + + [Fact] + public static void NoFields_Fails() + { + Console.WriteLine($"{nameof(NoFields_Fails)}..."); + Assert.Throws(() => { var t = typeof(NoFields); }); + + Assert.Throws(() => + { + return (new NoFields()).ToString(); + }); + } + + [InlineArray(1)] + private struct TwoFields + { + int a; + int b; + } + + [Fact] + public static void TwoFields_Fails() + { + Console.WriteLine($"{nameof(TwoFields_Fails)}..."); + Assert.Throws(() => { var t = typeof(TwoFields); }); + + Assert.Throws(() => + { + return new TwoFields[12]; + }); + } +} diff --git a/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.csproj b/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.csproj new file mode 100644 index 00000000000000..148e8a4fadff02 --- /dev/null +++ b/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.csproj @@ -0,0 +1,13 @@ + + + true + Exe + true + + + + + + + + diff --git a/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs new file mode 100644 index 00000000000000..142f23612ef6fa --- /dev/null +++ b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +using Xunit; + +[InlineArray(42)] +struct FourtyTwoBytes +{ + byte b; +} + +unsafe class Validate +{ + [Fact] + public static void Sizeof() + { + Console.WriteLine($"{nameof(Sizeof)}..."); + Assert.Equal(42, sizeof(FourtyTwoBytes)); + } +} diff --git a/src/tests/Loader/classloader/InlineArray/InlineArrayValid.csproj b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.csproj new file mode 100644 index 00000000000000..96c6ab6ea953ac --- /dev/null +++ b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.csproj @@ -0,0 +1,13 @@ + + + true + Exe + true + + + + + + + + diff --git a/src/tests/issues.targets b/src/tests/issues.targets index d508f5c2122199..f477910a6496b1 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1529,6 +1529,10 @@ Static virtual methods are not yet implemented in the Mono runtime. + + InlineArray support is not yet implemented in the Mono runtime. + + needs triage From 1852993d0d8836917500ecba16c7e1e44ffeea7c Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 3 Mar 2023 16:08:49 -0800 Subject: [PATCH 15/30] more tests --- .../InlineArray/InlineArrayValid.cs | 350 +++++++++++++++++- 1 file changed, 346 insertions(+), 4 deletions(-) diff --git a/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs index 142f23612ef6fa..ca8a36ed4a9dd4 100644 --- a/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs +++ b/src/tests/Loader/classloader/InlineArray/InlineArrayValid.cs @@ -3,23 +3,365 @@ using System; using System.IO; +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using Xunit; -[InlineArray(42)] -struct FourtyTwoBytes +// we will be doing "sizeof" with arrays containing managed references. +#pragma warning disable CS8500 // This takes the address of, gets the size of, or declares a pointer to a managed type + +[InlineArray(LengthConst)] +struct MyArray : IEnumerable { - byte b; + private const int LengthConst = 42; + private T _element; + + public int Length => LengthConst; + + [UnscopedRef] + public ref T this[int i] + { + get + { + if ((uint)i >= (uint)Length) + throw new IndexOutOfRangeException(nameof(i)); + + return ref Unsafe.Add(ref _element, i); + } + } + + [UnscopedRef] + public Span AsSpan() => MemoryMarshal.CreateSpan(ref _element, Length); + + IEnumerator IEnumerable.GetEnumerator() => (IEnumerator)this.GetEnumerator(); + + public IEnumerator GetEnumerator() + { + for (int i =0; i < Length; i++) + { + yield return this[i]; + } + } } unsafe class Validate { + // ====================== SizeOf ============================================================== + [InlineArray(42)] + struct FourtyTwoBytes + { + byte b; + } + [Fact] public static void Sizeof() { Console.WriteLine($"{nameof(Sizeof)}..."); Assert.Equal(42, sizeof(FourtyTwoBytes)); - } + Assert.Equal(84, sizeof(MyArray)); + } + + // ====================== OneElement ========================================================== + [InlineArray(1)] + struct OneObj + { + public object obj; + } + + [Fact] + public static void OneElement() + { + Console.WriteLine($"{nameof(OneElement)}..."); + Assert.Equal(sizeof(nint), sizeof(OneObj)); + } + + // ====================== UseOnStack ========================================================== + class One { } + class Two { } + class Three { } + class Four { } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static unsafe Arr1 Initialize(Arr1 s) + { + s[0].o = new One(); + s[1].o = new Two(); + s[2].o = new Three(); + s[3].o = new Four(); + return s; + } + + struct E + { + public int x; + public int y; + public object o; + } + + [InlineArray(Length)] + struct Arr1 + { + public const int Length = 42; + public E e; + + [UnscopedRef] + public ref E this[int i] => ref Unsafe.Add(ref e, i); + } + + static object s; + private static unsafe void MakeGarbage() + { + // make garbage + for (int i = 0; i < 10000; i++) + { + s = new int[i]; + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void PassByValueDoGcAndValidate(Arr1 s1) + { + MakeGarbage(); + + GC.Collect(2, GCCollectionMode.Forced, true, true); + GC.Collect(2, GCCollectionMode.Forced, true, true); + + Assert.Equal("One", s1[0].o.GetType().Name); + Assert.Equal("Two", s1[1].o.GetType().Name); + Assert.Equal("Three", s1[2].o.GetType().Name); + Assert.Equal("Four", s1[3].o.GetType().Name); + } + + [Fact] + public static void UseOnStack() + { + Console.WriteLine($"{nameof(UseOnStack)}..."); + + Arr1 s = default; + MakeGarbage(); + + s = Initialize(s); + + // use as a byval argument + PassByValueDoGcAndValidate(s); + + // refs must be separate and alive + Assert.Equal("One", s[0].o.GetType().Name); + Assert.Equal("Two", s[1].o.GetType().Name); + Assert.Equal("Three", s[2].o.GetType().Name); + Assert.Equal("Four", s[3].o.GetType().Name); + + // should copy by value + Arr1 s1 = s; + Assert.Equal("One", s1[0].o.GetType().Name); + Assert.Equal("Two", s1[1].o.GetType().Name); + Assert.Equal("Three", s1[2].o.GetType().Name); + Assert.Equal("Four", s1[3].o.GetType().Name); + } + + // ====================== MixObjectsAndValuetypes ============================================= + [InlineArray(Length)] + struct ObjShortArr + { + public const int Length = 100; + public (object, short) element; + + [UnscopedRef] + public ref (object o, short s) this[int i] => ref Unsafe.Add(ref element, i); + } + + [Fact] + public static void MixObjectsAndValuetypes() + { + Console.WriteLine($"{nameof(MixObjectsAndValuetypes)}..."); + Assert.Equal(ObjShortArr.Length * sizeof(nint) * 2, sizeof(ObjShortArr)); + + var arr = new ObjShortArr(); + for (short i = 0; i < ObjShortArr.Length; i++) + { + arr[i].o = i; + arr[i].s = (short)(i + 1); + } + + GC.Collect(2, GCCollectionMode.Forced, true, true); + + for (short i = 0; i < ObjShortArr.Length; i++) + { + Assert.Equal(i, arr[i].o); + Assert.Equal(i + 1, arr[i].s); + } + } + + // ====================== RefLikeOuter ======================================================== + [InlineArray(Length)] + ref struct ObjShortArrRef + { + public const int Length = 100; + public (object, short) element; + + [UnscopedRef] + public ref (object o, short s) this[int i] => ref Unsafe.Add(ref element, i); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void TestRefLikeOuterMethodArg(ObjShortArrRef arr) + { + GC.Collect(2, GCCollectionMode.Forced, true, true); + + for (short i = 0; i < ObjShortArrRef.Length; i++) + { + Assert.Equal(i * 2, arr[i].o); + Assert.Equal(i * 2 + 1, arr[i].s); + } + } + + [Fact] + public static void RefLikeOuter() + { + Console.WriteLine($"{nameof(RefLikeOuter)}..."); + + var arr = new ObjShortArrRef(); + for (short i = 0; i < ObjShortArrRef.Length; i++) + { + arr[i].o = i; + arr[i].s = (short)(i + 1); + } + + GC.Collect(2, GCCollectionMode.Forced, true, true); + + for (short i = 0; i < ObjShortArrRef.Length; i++) + { + Assert.Equal(i, arr[i].o); + Assert.Equal(i + 1, arr[i].s); + } + + for (short i = 0; i < ObjShortArrRef.Length; i++) + { + arr[i].o = i * 2; + arr[i].s = (short)(i * 2 + 1); + } + + TestRefLikeOuterMethodArg(arr); + } + + // ====================== RefLikeInner ======================================================== + [InlineArray(LengthConst)] + ref struct SpanArr + { + private const int LengthConst = 100; + public Span element; + + public Span* this[int i] + { + get + { + fixed (Span* p = &element) + { + return p + i; + } + } + } + + public int Length => LengthConst; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void TestRefLikeInnerMethodArg(SpanArr arr) + { + GC.Collect(2, GCCollectionMode.Forced, true, true); + + for (int i = 1; i < arr.Length; i++) + { + Assert.Equal(i, arr[i]->Length); + Assert.Equal(i, (*arr[i])[0]); + } + } + + [Fact] + public static void RefLikeInner() + { + Console.WriteLine($"{nameof(RefLikeInner)}..."); + + SpanArr arr = default; + for (int i = 1; i < arr.Length; i++) + { + var objArr = new object[i]; + objArr[0] = i; + *arr[i] = objArr; + } + + TestRefLikeInnerMethodArg(arr); + } + + // ====================== Nested ============================================================== + + struct IntObj + { + public int i; + public object o; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void NestedMethodArg(ref MyArray> nestedArray) + { + GC.Collect(2, GCCollectionMode.Forced, true, true); + + for (int i = 0; i < nestedArray.Length; i++) + { + for (int j = 0; j < nestedArray[i].Length; j++) + { + Assert.Equal(i + j, nestedArray[i][j].o); + Assert.Equal(i * j, nestedArray[i][j].i); + } + } + } + + [Fact] + public static void Nested() + { + Console.WriteLine($"{nameof(Nested)}..."); + + MyArray> nestedArray = default; + + for(int i = 0; i < nestedArray.Length; i++) + { + for (int j = 0; j < nestedArray[i].Length; j++) + { + nestedArray[i][j].o = i + j; + nestedArray[i][j].i = i * j; + } + } + } + + // ====================== Boxed =============================================================== + + [MethodImpl(MethodImplOptions.NoInlining)] + static void BoxedMethodArg(IEnumerable arr) + { + GC.Collect(2, GCCollectionMode.Forced, true, true); + + int i = 0; + foreach(object obj in arr) + { + Assert.Equal(i++, obj); + } + } + + [Fact] + public static void Boxed() + { + Console.WriteLine($"{nameof(Boxed)}..."); + + MyArray arr = default; + for (int i = 0; i < arr.Length; i++) + { + arr[i] = i; + } + + BoxedMethodArg(arr); + } } From 295caf1aed637efaaf1fc47594cf58cd09cb6e75 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 3 Mar 2023 16:25:56 -0800 Subject: [PATCH 16/30] limit the max size of array instance to 1MiB --- .../Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 5bb0cd320f3904..b0ca692c01ed0d 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -468,9 +468,8 @@ private static void AdjustForInlineArray(MetadataType type, ref SizeAndAlignment long size = instanceByteSizeAndAlignment.Size.AsInt; size *= repeat; - // matching the max size validation in MethodTableBuilder - // see: FIELD_OFFSET_LAST_REAL_OFFSET - const int maxSize = ((1 << 27) - 1) - 6; + // limit the max size of array instance to 1MiB + const int maxSize = 1024 * 1024; if (size > maxSize) { ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadValueClassTooLarge, type); From e7738ea3b6bf5df81169a44dd38f35684b31f632 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Fri, 3 Mar 2023 18:50:26 -0800 Subject: [PATCH 17/30] fix an assert in importercalls.cpp --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index f9354f947865ff..9b18d14a12f333 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2893,7 +2893,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, GenTree* indexClone = nullptr; GenTree* ptrToSpanClone = nullptr; assert(genActualType(index) == TYP_INT); - assert(ptrToSpan->TypeGet() == TYP_BYREF); + assert(ptrToSpan->TypeGet() == TYP_BYREF || ptrToSpan->TypeGet() == TYP_I_IMPL); #if defined(DEBUG) if (verbose) From 065467bb898c49956691933be7f63b1227e29836 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 4 Mar 2023 09:34:24 -0800 Subject: [PATCH 18/30] "result" in GC layout should track the pointer count, not the size --- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 5 +++-- src/coreclr/vm/jitinterface.cpp | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 5d266b8c64d162..81c82ade4e4b4a 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2294,13 +2294,14 @@ private int GatherClassGCLayout(TypeDesc type, byte* gcPtrs) Debug.Assert(field.Offset.AsInt == 0); int totalLayoutSize = type.GetElementSize().AsInt / PointerSize; int elementLayoutSize = fieldType.GetElementSize().AsInt / PointerSize; + int gcPointersInElement = result; for (int offset = elementLayoutSize; offset < totalLayoutSize; offset += elementLayoutSize) { Buffer.MemoryCopy(gcPtrs, gcPtrs + offset, elementLayoutSize, elementLayoutSize); - result += elementLayoutSize; + result += gcPointersInElement; } - // value array has only one element field + // inline array has only one element field break; } } diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 7b13040a0711dd..10ad83e628028f 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -2194,13 +2194,14 @@ static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs) _ASSERTE(pFD->GetOffset() == 0); DWORD totalLayoutSize = pMT->GetNumInstanceFieldBytes() / TARGET_POINTER_SIZE; DWORD elementLayoutSize = pFD->GetSize() / TARGET_POINTER_SIZE; + DWORD gcPointersInElement = result; for (DWORD offset = elementLayoutSize; offset < totalLayoutSize; offset += elementLayoutSize) { memcpy(gcPtrs + offset, gcPtrs, elementLayoutSize); - result += elementLayoutSize; + result += gcPointersInElement; } - // value array has only one element field + // inline array has only one element field break; } } From 62b20c8d0c37d9313098698c8da0af2446b1fc00 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 7 Mar 2023 16:51:42 -0800 Subject: [PATCH 19/30] error messages --- src/coreclr/dlls/mscorrc/mscorrc.rc | 4 ++++ src/coreclr/dlls/mscorrc/resource.h | 4 ++++ .../Runtime/TypeLoaderExceptionHelper.cs | 6 ++++++ .../TypeSystem/Common/ExceptionStringID.cs | 4 ++++ .../Common/MetadataFieldLayoutAlgorithm.cs | 19 ++++++++++++++----- src/coreclr/vm/methodtablebuilder.cpp | 9 +++------ .../src/Resources/Strings.resx | 9 +++++++++ 7 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/coreclr/dlls/mscorrc/mscorrc.rc b/src/coreclr/dlls/mscorrc/mscorrc.rc index 62c867c3565403..21784fce2e8cf1 100644 --- a/src/coreclr/dlls/mscorrc/mscorrc.rc +++ b/src/coreclr/dlls/mscorrc/mscorrc.rc @@ -310,6 +310,10 @@ BEGIN IDS_CLASSLOAD_GENERICTYPE_RECURSIVE "Could not load type '%1' from assembly '%2' because it has recursive generic definition." IDS_CLASSLOAD_TOOMANYGENERICARGS "Could not load type '%1' from assembly '%2'. Internal limitation: Too many generic arguments." + IDS_CLASSLOAD_INLINE_ARRAY_FIELD_COUNT "InlineArrayAttribute requires that the target type has a single instance field. Type: '%1'. Assembly: '%2'." + IDS_CLASSLOAD_INLINE_ARRAY_LENGTH "InlineArrayAttribute requires that the length argument is greater than 0. Type: '%1'. Assembly: '%2'." + IDS_CLASSLOAD_INLINE_ARRAY_EXPLICIT "InlineArrayAttribute cannot be applied to a type with explicit layout. Type: '%1'. Assembly: '%2'." + #if FEATURE_COMINTEROP IDS_EE_CANNOTCAST_NOMARSHAL "The Windows Runtime Object can only be used in the threading context where it was created, because it implements INoMarshal or has MarshalingBehaviorAttribute(MarshalingType.None) set." #endif diff --git a/src/coreclr/dlls/mscorrc/resource.h b/src/coreclr/dlls/mscorrc/resource.h index 1903a00a7fac98..b65318ca02ad08 100644 --- a/src/coreclr/dlls/mscorrc/resource.h +++ b/src/coreclr/dlls/mscorrc/resource.h @@ -168,6 +168,10 @@ #define IDS_CLASSLOAD_TOOMANYGENERICARGS 0x17ab +#define IDS_CLASSLOAD_INLINE_ARRAY_FIELD_COUNT 0x17ac +#define IDS_CLASSLOAD_INLINE_ARRAY_LENGTH 0x17ad +#define IDS_CLASSLOAD_INLINE_ARRAY_EXPLICIT 0x17ae + #define IDS_DEBUG_USERBREAKPOINT 0x17b6 #define IDS_PERFORMANCEMON_FUNCNOTFOUND 0x17bb diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/TypeLoaderExceptionHelper.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/TypeLoaderExceptionHelper.cs index adedfbf710e2fe..76b714c11a3946 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/TypeLoaderExceptionHelper.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/TypeLoaderExceptionHelper.cs @@ -80,6 +80,12 @@ private static string GetFormatString(ExceptionStringID id) return SR.ClassLoad_ExplicitLayout; case ExceptionStringID.ClassLoadRankTooLarge: return SR.ClassLoad_RankTooLarge; + case ExceptionStringID.ClassLoadInlineArrayFieldCount: + return SR.ClassLoad_InlineArrayFieldCount; + case ExceptionStringID.ClassLoadInlineArrayLength: + return SR.ClassLoad_InlineArrayLength; + case ExceptionStringID.ClassLoadInlineArrayExplicit: + return SR.ClassLoad_InlineArrayExplicit; case ExceptionStringID.InvalidProgramDefault: return SR.InvalidProgram_Default; case ExceptionStringID.InvalidProgramSpecific: diff --git a/src/coreclr/tools/Common/TypeSystem/Common/ExceptionStringID.cs b/src/coreclr/tools/Common/TypeSystem/Common/ExceptionStringID.cs index d5824a3d2a58f0..4c55874d7999fb 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/ExceptionStringID.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/ExceptionStringID.cs @@ -16,6 +16,10 @@ public enum ExceptionStringID ClassLoadValueClassTooLarge, ClassLoadRankTooLarge, + ClassLoadInlineArrayFieldCount, + ClassLoadInlineArrayLength, + ClassLoadInlineArrayExplicit, + // MissingMethodException MissingMethod, diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index b0ca692c01ed0d..1b3b259782fe86 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -360,7 +360,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty // value array cannot have explicit layout if (type.IsInlineArray) { - ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadInlineArrayExplicit, type); } ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout @@ -436,7 +436,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType if (type.IsInlineArray) { - AdjustForInlineArray(type, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); + AdjustForInlineArray(type, numInstanceFields, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); } ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout @@ -454,13 +454,22 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType return computedLayout; } - private static void AdjustForInlineArray(MetadataType type, ref SizeAndAlignment instanceByteSizeAndAlignment, ref SizeAndAlignment instanceSizeAndAlignment) + private static void AdjustForInlineArray( + MetadataType type, + int instanceFieldCount, + ref SizeAndAlignment instanceByteSizeAndAlignment, + ref SizeAndAlignment instanceSizeAndAlignment) { int repeat = type.GetInlineArrayLength(); if (repeat <= 0) { - ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadInlineArrayLength, type); + } + + if (instanceFieldCount != 1) + { + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadInlineArrayFieldCount, type); } if (!instanceByteSizeAndAlignment.Size.IsIndeterminate) @@ -788,7 +797,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, if (type.IsInlineArray) { - AdjustForInlineArray(type, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); + AdjustForInlineArray(type, numInstanceFields, ref instanceByteSizeAndAlignment, ref instanceSizeAndAlignment); } ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 4505bca531b371..56baa9e83a60fe 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1706,8 +1706,7 @@ MethodTableBuilder::BuildMethodTableThrowing( { if (bmtEnumFields->dwNumInstanceFields != 1) { - // TODO: diagnostics, must have one field. - BuildMethodTableThrowException(IDS_CLASSLOAD_GENERAL); + BuildMethodTableThrowException(IDS_CLASSLOAD_INLINE_ARRAY_FIELD_COUNT); } if (cbVal >= (sizeof(INT32) + 2)) @@ -1720,14 +1719,12 @@ MethodTableBuilder::BuildMethodTableThrowing( } else { - // TODO: diagnostics, repeat must be > 0 - BuildMethodTableThrowException(IDS_CLASSLOAD_GENERAL); + BuildMethodTableThrowException(IDS_CLASSLOAD_INLINE_ARRAY_LENGTH); } if (HasExplicitFieldOffsetLayout()) { - // TODO: diagnostics, must not have explicit offsets - BuildMethodTableThrowException(IDS_CLASSLOAD_GENERAL); + BuildMethodTableThrowException(IDS_CLASSLOAD_INLINE_ARRAY_EXPLICIT); } } } diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 4d2a3582ddd773..e7dc54f16d1940 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -3893,6 +3893,15 @@ '{0}' from assembly '{1}' has too many dimensions. + + InlineArrayAttribute requires that the target type has a single instance field. Type: '{0}'. Assembly: '{1}'. + + + InlineArrayAttribute requires that the length argument is greater than 0. Type: '{0}'. Assembly: '{1}'. + + + InlineArrayAttribute cannot be applied to a type with explicit layout. Type: '{0}'. Assembly: '{1}'. + Could not load type '{0}' from assembly '{1}' because generic types cannot have explicit layout. From 01e2e177e560fc793e62226097acfc6dc85ef242 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 7 Mar 2023 17:37:08 -0800 Subject: [PATCH 20/30] fixed uses of "value array" in comments. --- src/coreclr/inc/corinfo.h | 2 +- src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs | 2 +- .../Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs | 2 +- src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs | 2 +- .../Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index b405a6edfaa17c..d1edff70ea24d8 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -852,7 +852,7 @@ enum CorInfoFlag CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently) CORINFO_FLG_OVERLAPPING_FIELDS = 0x00100000, // struct or class has fields that overlap (aka union) CORINFO_FLG_INTERFACE = 0x00200000, // it is an interface - CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't ask field info (used for types outside of AOT compilation version bubble and value arrays) + CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't ask field info (used for types outside of AOT compilation version bubble and inline arrays) CORINFO_FLG_CUSTOMLAYOUT = 0x00800000, // does this struct have custom layout? CORINFO_FLG_CONTAINS_GC_PTR = 0x01000000, // does the class contain a gc ptr ? CORINFO_FLG_DELEGATE = 0x02000000, // is this a subclass of delegate or multicast delegate ? diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index f1b57271461b50..d3e0ef50b34372 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -602,7 +602,7 @@ public enum CorInfoFlag : uint CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently) CORINFO_FLG_OVERLAPPING_FIELDS = 0x00100000, // struct or class has fields that overlap (aka union) CORINFO_FLG_INTERFACE = 0x00200000, // it is an interface - CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't try to ask about fields outside of AOT compilation version bubble or value arrays + CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't try to ask about fields outside of AOT compilation version bubble or inline arrays CORINFO_FLG_CUSTOMLAYOUT = 0x00800000, // does this struct have custom layout? CORINFO_FLG_CONTAINS_GC_PTR = 0x01000000, // does the class contain a gc ptr ? CORINFO_FLG_DELEGATE = 0x02000000, // is this a subclass of delegate or multicast delegate ? diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 1b3b259782fe86..0fc9064fb00a81 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -357,7 +357,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty layoutMetadata.Size, out instanceByteSizeAndAlignment); - // value array cannot have explicit layout + // inline array cannot have explicit layout if (type.IsInlineArray) { ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadInlineArrayExplicit, type); diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs index 77f474667a5b33..e0caede6be791a 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs @@ -677,7 +677,7 @@ public bool IsByRefLike } /// - /// Gets a value indicating whether this is a value array type + /// Gets a value indicating whether this is an inline array type /// public bool IsInlineArray { diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs index 7d919b67272784..eb3097783a9d6a 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs @@ -297,7 +297,7 @@ private void GcScanValueType(TypeDesc type, in ArgDestination argDest, int delta GcScanRoots(field.FieldType, in argDest, delta + offset, frame, topLevel: false); } - // there is only one formal instance field in a value array + // there is only one formal instance field in an inline array Debug.Assert(field.Offset.AsInt == 0); break; } From f8447726ead7aa89fb8b4a6a1996f989707a5d79 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 7 Mar 2023 18:23:52 -0800 Subject: [PATCH 21/30] PR feedback on StackAllocedArguments --- .../Reflection/Emit/DynamicMethod.CoreCLR.cs | 4 ++-- .../Reflection/RuntimeMethodInfo.CoreCLR.cs | 4 ++-- .../src/System/Reflection/DynamicInvokeInfo.cs | 14 ++------------ .../src/System/Reflection/MethodBase.cs | 18 ++++++++++++------ .../Reflection/RuntimeConstructorInfo.cs | 8 ++++---- .../src/System/Reflection/RuntimeMethodInfo.cs | 4 ++-- 6 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs index 9633a784519a33..dc764971c2604d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.CoreCLR.cs @@ -150,8 +150,8 @@ Signature LazyCreateSignature() { Debug.Assert(parameters != null); StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0._arg0, argCount); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, argCount); + Span copyOfParameters = argStorage._args.AsSpan(argCount); + Span shouldCopyBackParameters = argStorage._copyBacks.AsSpan(argCount); StackAllocatedByRefs byrefStorage = default; #pragma warning disable CS8500 diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs index 5afcf0d87fddea..0d25286564e343 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs @@ -311,9 +311,9 @@ public override MethodImplAttributes GetMethodImplementationFlags() unsafe { StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0._arg0, 1); + Span copyOfParameters = argStorage._args.AsSpan(1); ReadOnlySpan parameters = new(in parameter); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, 1); + Span shouldCopyBackParameters = argStorage._copyBacks.AsSpan(1); StackAllocatedByRefs byrefStorage = default; #pragma warning disable 8500 diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs index 4f0106980837d4..bd260497b6e585 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs @@ -482,27 +482,17 @@ private unsafe object ReturnTransform(ref byte byref, bool wrapInTargetInvocatio // and pass it to CheckArguments(). // For argument count > MaxStackAllocArgCount, do a stackalloc of void* pointers along with // GCReportingRegistration to safely track references. - [StructLayout(LayoutKind.Sequential)] + [InlineArray(MaxStackAllocArgCount)] private ref struct StackAllocedArguments { internal object? _arg0; -#pragma warning disable CA1823, CS0169, IDE0051 // accessed via 'CheckArguments' ref arithmetic - private object? _arg1; - private object? _arg2; - private object? _arg3; -#pragma warning restore CA1823, CS0169, IDE0051 } // Helper struct to avoid intermediate IntPtr[] allocation and RegisterForGCReporting in calls to the native reflection stack. - [StructLayout(LayoutKind.Sequential)] + [InlineArray(MaxStackAllocArgCount)] private ref struct StackAllocatedByRefs { internal ref byte _arg0; -#pragma warning disable CA1823, CS0169, IDE0051 // accessed via 'CheckArguments' ref arithmetic - private ref byte _arg1; - private ref byte _arg2; - private ref byte _arg3; -#pragma warning restore CA1823, CS0169, IDE0051 } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs index 82a5ff83c10467..a43181e65b59fb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs @@ -279,14 +279,20 @@ BindingFlags invokeAttr #endif private protected struct ArgumentData { - internal T _arg0; + private T _arg0; #if !CORECLR #pragma warning disable CA1823, CS0169, IDE0051, IDE0044 // accessed via 'CheckArguments' ref arithmetic - private object? _arg1; - private object? _arg2; - private object? _arg3; + private T _arg1; + private T _arg2; + private T _arg3; #pragma warning restore CA1823, CS0169, IDE0051, IDE0044 #endif + [UnscopedRef] + public Span AsSpan(int length) + { + Debug.Assert((uint)length <= (uint) MaxStackAllocArgCount); + return new Span(ref _arg0, length); + } } // Helper struct to avoid intermediate object[] allocation in calls to the native reflection stack. @@ -297,8 +303,8 @@ private protected struct ArgumentData [StructLayout(LayoutKind.Sequential)] private protected ref struct StackAllocedArguments { - internal ArgumentData _arg0; - internal ArgumentData _copyBack0; + internal ArgumentData _args; + internal ArgumentData _copyBacks; } // Helper struct to avoid intermediate IntPtr[] allocation and RegisterForGCReporting in calls to the native reflection stack. diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs index e096965c66b23c..d9b5ae564bafe4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -142,8 +142,8 @@ internal void ThrowNoInvokeException() { Debug.Assert(parameters != null); StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0._arg0, argCount); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, argCount); + Span copyOfParameters = argStorage._args.AsSpan(argCount); + Span shouldCopyBackParameters = argStorage._copyBacks.AsSpan(argCount); StackAllocatedByRefs byrefStorage = default; #pragma warning disable 8500 @@ -289,8 +289,8 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] { Debug.Assert(parameters != null); StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0._arg0, argCount); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, argCount); + Span copyOfParameters = argStorage._args.AsSpan(argCount); + Span shouldCopyBackParameters = argStorage._copyBacks.AsSpan(argCount); StackAllocatedByRefs byrefStorage = default; #pragma warning disable 8500 diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs index cb87a6969ca483..62e62b1550c9ad 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs @@ -139,8 +139,8 @@ private void ThrowNoInvokeException() { Debug.Assert(parameters != null); StackAllocedArguments argStorage = default; - Span copyOfParameters = new(ref argStorage._arg0._arg0, argCount); - Span shouldCopyBackParameters = new(ref argStorage._copyBack0._arg0, argCount); + Span copyOfParameters = argStorage._args.AsSpan(argCount); + Span shouldCopyBackParameters = argStorage._copyBacks.AsSpan(argCount); StackAllocatedByRefs byrefStorage = default; #pragma warning disable 8500 From 1b5f608b4f50868473e64cfd67944266a407cea1 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Tue, 7 Mar 2023 18:30:02 -0800 Subject: [PATCH 22/30] use the same size limit for inline arrays in the typeloader --- src/coreclr/vm/methodtablebuilder.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 56baa9e83a60fe..8d89af2ae71236 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -8362,7 +8362,10 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable ** pByValueClassCach if (bmtFP->NumInlineArrayElements > 1) { INT64 extendedSize = (INT64)dwNumInstanceFieldBytes * (INT64)bmtFP->NumInlineArrayElements; - if (extendedSize > FIELD_OFFSET_LAST_REAL_OFFSET) { + // limit the max size of array instance to 1MiB + const INT64 maxSize = 1024 * 1024; + if (extendedSize > maxSize) + { BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); } From bf676e4d19644134ef73bbece091740799438747 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 8 Mar 2023 12:30:08 -0800 Subject: [PATCH 23/30] more PR feedback --- src/coreclr/vm/class.h | 4 ++-- src/coreclr/vm/jitinterface.cpp | 4 ++-- src/coreclr/vm/methodtablebuilder.cpp | 8 ++++--- src/coreclr/vm/siginfo.cpp | 4 ++-- .../CompilerServices/InlineArrayAttribute.cs | 3 +++ .../InlineArray/InlineArrayInvalid.cs | 23 +++++++++++++++++++ src/tests/issues.targets | 2 +- 7 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 74554d12d34274..dd9ce6f2d4af01 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -1384,12 +1384,12 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! LIMITED_METHOD_CONTRACT; m_VMFlags |= (DWORD)VMFLAG_HAS_FIELDS_WHICH_MUST_BE_INITED; } - BOOL HasInlineArrayFlagSet() + DWORD IsInlineArray() { LIMITED_METHOD_CONTRACT; return (m_VMFlags & VMFLAG_INLINE_ARRAY); } - void SetInlineArrayFlag() + void SetIsInlineArray() { LIMITED_METHOD_CONTRACT; m_VMFlags |= (DWORD)VMFLAG_INLINE_ARRAY; diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 10ad83e628028f..38ca24f967da36 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -2169,7 +2169,7 @@ static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs) _ASSERTE(pMT->IsValueType()); unsigned result = 0; - bool isInlineArray = pMT->GetClass()->HasInlineArrayFlagSet(); + bool isInlineArray = pMT->GetClass()->IsInlineArray(); ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); for (FieldDesc *pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) { @@ -3569,7 +3569,7 @@ uint32_t CEEInfo::getClassAttribsInternal (CORINFO_CLASS_HANDLE clsHnd) if (pClass->HasExplicitFieldOffsetLayout() && pClass->HasOverlaidField()) ret |= CORINFO_FLG_OVERLAPPING_FIELDS; - if (pClass->HasInlineArrayFlagSet()) + if (pClass->IsInlineArray()) ret |= CORINFO_FLG_DONT_DIG_FIELDS; if (VMClsHnd.IsCanonicalSubtype()) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 8d89af2ae71236..93200f13d9396d 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1715,7 +1715,7 @@ MethodTableBuilder::BuildMethodTableThrowing( if (repeat > 0) { bmtFP->NumInlineArrayElements = repeat; - GetHalfBakedClass()->SetInlineArrayFlag(); + GetHalfBakedClass()->SetIsInlineArray(); } else { @@ -1749,7 +1749,7 @@ MethodTableBuilder::BuildMethodTableThrowing( _ASSERTE(HasLayout()); - if (bmtFP->NumInlineArrayElements) + if (bmtFP->NumInlineArrayElements != 0) { GetLayoutInfo()->m_cbManagedSize *= bmtFP->NumInlineArrayElements; } @@ -11556,7 +11556,6 @@ VOID MethodTableBuilder::HandleGCForValueClasses(MethodTable ** pByValueClassCac DWORD repeat = 1; if (bmtFP->NumInlineArrayElements > 1) { - _ASSERTE(bmtEnumFields->dwNumInstanceFields == 1); repeat = bmtFP->NumInlineArrayElements; } @@ -11583,6 +11582,9 @@ VOID MethodTableBuilder::HandleGCForValueClasses(MethodTable ** pByValueClassCac DWORD dwCurrentOffset = pFieldDescList[i].GetOffset_NoLogging(); DWORD dwElementSize = pByValueMT->GetBaseSize() - OBJECT_BASESIZE; + // if we have an inline array, we will have only one formal instance field, + // but will have to replicate the layout "repeat" times. + // otherwise every field will be matched with 1 serie. for (DWORD r = 0; r < repeat; r++) { // The by value class may have more than one pointer series diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index d333cfee9e249a..2efb208d1bc8bc 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -4862,11 +4862,11 @@ class ByRefPointerOffsetsReporter _ASSERTE(pMT != nullptr); _ASSERTE(pMT->IsByRefLike()); - bool isValArray = pMT->GetClass()->HasInlineArrayFlagSet(); + bool isValArray = pMT->GetClass()->IsInlineArray(); ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS); for (FieldDesc* pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next()) { - if(isValArray) + if (isValArray) { _ASSERTE(pFD->GetOffset() == 0); DWORD elementSize = pFD->GetSize(); diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs index eb966ca090d5b0..9872c61e191ffa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs @@ -5,6 +5,9 @@ namespace System.Runtime.CompilerServices { + /// + /// Indicates that the instance's storage is sequentially replicated "length" times. + /// [EditorBrowsable(EditorBrowsableState.Never)] [AttributeUsage(AttributeTargets.Struct, AllowMultiple = false)] public sealed class InlineArrayAttribute : Attribute diff --git a/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs b/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs index 09bff64d16b995..6704199bee115a 100644 --- a/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs +++ b/src/tests/Loader/classloader/InlineArray/InlineArrayInvalid.cs @@ -74,6 +74,29 @@ public static void TooLarge_Fails() }); } + [InlineArray(-1)] + private struct NegativeLength + { + public long field; + } + + [Fact] + public static void NegativeLength_Fails() + { + Console.WriteLine($"{nameof(NegativeLength_Fails)}..."); + Assert.Throws(() => { var t = typeof(NegativeLength); }); + + Assert.Throws(() => + { + var t = new NegativeLength() + { + field = 1 + }; + return t; + }); + } + + [InlineArray(123)] private struct NoFields { diff --git a/src/tests/issues.targets b/src/tests/issues.targets index f477910a6496b1..5a18053d5ce17f 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1530,7 +1530,7 @@ - InlineArray support is not yet implemented in the Mono runtime. + https://github.com/dotnet/runtime/issues/80798 From 7d42fbc6c73461385fa301e5e6ed10eaa547cbda Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Wed, 8 Mar 2023 12:37:51 -0800 Subject: [PATCH 24/30] remove SetCannotBeBlittedByObjectCloner --- src/coreclr/vm/class.h | 4 ---- src/coreclr/vm/methodtablebuilder.cpp | 5 ----- src/coreclr/vm/methodtablebuilder.h | 1 - 3 files changed, 10 deletions(-) diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index dd9ce6f2d4af01..827212f0ac391f 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -1394,10 +1394,6 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! LIMITED_METHOD_CONTRACT; m_VMFlags |= (DWORD)VMFLAG_INLINE_ARRAY; } - void SetCannotBeBlittedByObjectCloner() - { - /* no op */ - } DWORD HasNonPublicFields() { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 93200f13d9396d..1fbb50eb825cdd 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -3805,9 +3805,6 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, if (!IsFdPublic(dwMemberAttrs)) SetHasNonPublicFields(); - if (IsFdNotSerialized(dwMemberAttrs)) - SetCannotBeBlittedByObjectCloner(); - IfFailThrow(pInternalImport->GetSigOfFieldDef(bmtMetaData->pFields[i], &cMemberSignature, &pMemberSignature)); // Signature validation IfFailThrow(validateTokenSig(bmtMetaData->pFields[i],pMemberSignature,cMemberSignature,dwMemberAttrs,pInternalImport)); @@ -3997,8 +3994,6 @@ VOID MethodTableBuilder::InitializeFieldDescs(FieldDesc *pFieldDescList, if (!fIsStatic) { SetHasFieldsWhichMustBeInited(); - if (ElementType != ELEMENT_TYPE_STRING) - SetCannotBeBlittedByObjectCloner(); } else { // EnumerateFieldDescs already counted the total number of static vs. instance diff --git a/src/coreclr/vm/methodtablebuilder.h b/src/coreclr/vm/methodtablebuilder.h index 0064be068fcfd9..f8cc5b86aab529 100644 --- a/src/coreclr/vm/methodtablebuilder.h +++ b/src/coreclr/vm/methodtablebuilder.h @@ -218,7 +218,6 @@ class MethodTableBuilder // we create the EEClass object, and thus set the flags immediately at the point // we create that object. void SetUnsafeValueClass() { WRAPPER_NO_CONTRACT; GetHalfBakedClass()->SetUnsafeValueClass(); } - void SetCannotBeBlittedByObjectCloner() { WRAPPER_NO_CONTRACT; GetHalfBakedClass()->SetCannotBeBlittedByObjectCloner(); } void SetHasFieldsWhichMustBeInited() { WRAPPER_NO_CONTRACT; GetHalfBakedClass()->SetHasFieldsWhichMustBeInited(); } void SetHasNonPublicFields() { WRAPPER_NO_CONTRACT; GetHalfBakedClass()->SetHasNonPublicFields(); } void SetModuleDynamicID(DWORD x) { WRAPPER_NO_CONTRACT; GetHalfBakedClass()->SetModuleDynamicID(x); } From 7fff2be0621cef2e5566dbfaad177dd200caa942 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 9 Mar 2023 16:29:40 -0800 Subject: [PATCH 25/30] moving GetInlineArrayLength to MetadataType and related changes --- .../Common/Internal/Runtime/GCDescEncoder.cs | 8 ++++---- .../tools/Common/JitInterface/CorInfoImpl.cs | 12 ++++++------ .../TypeSystem/Canon/CanonTypes.Metadata.cs | 6 ++++++ .../TypeSystem/CodeGen/TypeDesc.CodeGen.cs | 6 ++++++ .../Common/TypeSystem/Common/InstantiatedType.cs | 7 +++++-- .../Common/TypeSystem/Common/MetadataType.cs | 13 ++++++++++--- .../tools/Common/TypeSystem/Common/TypeDesc.cs | 11 ----------- .../Common/Utilities/GCPointerMap.Algorithm.cs | 16 ++++++++-------- .../TypeSystem/Interop/IL/InlineArrayType.cs | 12 ++++++++---- .../TypeSystem/Interop/IL/NativeStructType.cs | 6 ++++++ .../Interop/IL/PInvokeDelegateWrapper.cs | 6 ++++++ .../CompilerTypeSystemContext.BoxedTypes.cs | 6 ++++++ ...ompilerTypeSystemContext.GeneratedAssembly.cs | 6 ++++++ .../DependencyAnalysis/NativeLayoutVertexNode.cs | 4 ++-- .../ReadyToRun/GCRefMapBuilder.cs | 10 +++++----- .../TypeRefTypeSystem/TypeRefTypeSystemType.cs | 7 +++++++ 16 files changed, 91 insertions(+), 45 deletions(-) diff --git a/src/coreclr/tools/Common/Internal/Runtime/GCDescEncoder.cs b/src/coreclr/tools/Common/Internal/Runtime/GCDescEncoder.cs index 207f67e1522cbf..ae148d0f2298ea 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/GCDescEncoder.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/GCDescEncoder.cs @@ -28,7 +28,7 @@ public static int GetGCDescSize(TypeDesc type) } else if (elementType.IsDefType) { - var defType = (DefType)elementType; + var defType = (MetadataType)elementType; if (defType.ContainsGCPointers) { GCPointerMap pointerMap = GCPointerMap.FromInstanceLayout(defType); @@ -48,7 +48,7 @@ public static int GetGCDescSize(TypeDesc type) } else { - var defType = (DefType)type; + var defType = (MetadataType)type; if (defType.ContainsGCPointers) { int numSeries = GCPointerMap.FromInstanceLayout(defType).NumSeries; @@ -84,7 +84,7 @@ public static void EncodeGCDesc(ref T builder, TypeDesc type) } else if (elementType.IsDefType) { - var elementDefType = (DefType)elementType; + var elementDefType = (MetadataType)elementType; if (elementDefType.ContainsGCPointers) { GCPointerMap pointerMap = GCPointerMap.FromInstanceLayout(elementDefType); @@ -101,7 +101,7 @@ public static void EncodeGCDesc(ref T builder, TypeDesc type) } else { - var defType = (DefType)type; + var defType = (MetadataType)type; if (defType.ContainsGCPointers) { // Computing the layout for the boxed version if this is a value type. diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 81c82ade4e4b4a..3807ce72c54bc0 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2010,6 +2010,9 @@ private uint getClassAttribsInternal(TypeDesc type) if (metadataType.IsUnsafeValueType) result |= CorInfoFlag.CORINFO_FLG_UNSAFE_VALUECLASS; + + if (metadataType.IsInlineArray) + result |= CorInfoFlag.CORINFO_FLG_DONT_DIG_FIELDS; } if (type.IsCanonicalSubtype(CanonicalFormKind.Any)) @@ -2027,9 +2030,6 @@ private uint getClassAttribsInternal(TypeDesc type) if (type.IsIntrinsic) result |= CorInfoFlag.CORINFO_FLG_INTRINSIC_TYPE; - if (type.IsInlineArray) - result |= CorInfoFlag.CORINFO_FLG_DONT_DIG_FIELDS; - if (metadataType != null) { if (metadataType.ContainsGCPointers) @@ -2243,7 +2243,7 @@ private int MarkGcField(byte* gcPtrs, CorInfoGCType gcType) } } - private int GatherClassGCLayout(TypeDesc type, byte* gcPtrs) + private int GatherClassGCLayout(MetadataType type, byte* gcPtrs) { int result = 0; bool isInlineArray = type.IsInlineArray; @@ -2282,7 +2282,7 @@ private int GatherClassGCLayout(TypeDesc type, byte* gcPtrs) if (gcType == CorInfoGCType.TYPE_GC_OTHER) { - result += GatherClassGCLayout(fieldType, fieldGcPtrs); + result += GatherClassGCLayout((MetadataType)fieldType, fieldGcPtrs); } else { @@ -2312,7 +2312,7 @@ private uint getClassGClayout(CORINFO_CLASS_STRUCT_* cls, byte* gcPtrs) { uint result = 0; - DefType type = (DefType)HandleToObject(cls); + MetadataType type = (MetadataType)HandleToObject(cls); int pointerSize = PointerSize; diff --git a/src/coreclr/tools/Common/TypeSystem/Canon/CanonTypes.Metadata.cs b/src/coreclr/tools/Common/TypeSystem/Canon/CanonTypes.Metadata.cs index 9e657c1e6546f4..320491b70319aa 100644 --- a/src/coreclr/tools/Common/TypeSystem/Canon/CanonTypes.Metadata.cs +++ b/src/coreclr/tools/Common/TypeSystem/Canon/CanonTypes.Metadata.cs @@ -57,6 +57,12 @@ public override bool HasCustomAttribute(string attributeNamespace, string attrib { return false; } + + public override int GetInlineArrayLength() + { + Debug.Fail("if this can be an inline array, implement GetInlineArrayLength"); + throw new InvalidOperationException(); + } } internal sealed partial class CanonType diff --git a/src/coreclr/tools/Common/TypeSystem/CodeGen/TypeDesc.CodeGen.cs b/src/coreclr/tools/Common/TypeSystem/CodeGen/TypeDesc.CodeGen.cs index 860285c10f2341..c14150dc6a28e9 100644 --- a/src/coreclr/tools/Common/TypeSystem/CodeGen/TypeDesc.CodeGen.cs +++ b/src/coreclr/tools/Common/TypeSystem/CodeGen/TypeDesc.CodeGen.cs @@ -25,5 +25,11 @@ partial void AddComputedIntrinsicFlag(ref TypeFlags flags) if (_typeDef.IsIntrinsic) flags |= TypeFlags.IsIntrinsic; } + + partial void AddComputedInlineArrayFlag(ref TypeFlags flags) + { + if (((MetadataType)_typeDef).IsInlineArray) + flags |= TypeFlags.IsInlineArray; + } } } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs index 7dc5e9ee3c3282..d6fb23db52449b 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs @@ -74,6 +74,10 @@ public override DefType BaseType // will provide an implementation that adds the flag if necessary. partial void AddComputedIntrinsicFlag(ref TypeFlags flags); + // Type system implementations that support the notion of inline arrays + // will provide an implementation that adds the flag if necessary. + partial void AddComputedInlineArrayFlag(ref TypeFlags flags); + protected override TypeFlags ComputeTypeFlags(TypeFlags mask) { TypeFlags flags = 0; @@ -106,8 +110,7 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask) if (_typeDef.IsByRefLike) flags |= TypeFlags.IsByRefLike; - if (_typeDef.IsInlineArray) - flags |= TypeFlags.IsInlineArray; + AddComputedInlineArrayFlag(ref flags); AddComputedIntrinsicFlag(ref flags); } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs index 98ab4cecb4cfdd..1d05990af76a5b 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs @@ -92,11 +92,18 @@ public virtual bool IsModuleType /// public abstract MetadataType GetNestedType(string name); - public virtual int GetInlineArrayLength() + /// + /// Gets a value indicating whether this is an inline array type + /// + public virtual bool IsInlineArray { - Debug.Assert(this.IsInlineArray); - return 1; + get + { + return (GetTypeFlags(TypeFlags.IsInlineArray | TypeFlags.AttributeCacheComputed) & TypeFlags.IsInlineArray) != 0; + } } + + public abstract int GetInlineArrayLength(); } public struct ClassLayoutMetadata diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs index e0caede6be791a..ae83551e2bd0e3 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs @@ -676,17 +676,6 @@ public bool IsByRefLike } } - /// - /// Gets a value indicating whether this is an inline array type - /// - public bool IsInlineArray - { - get - { - return (GetTypeFlags(TypeFlags.IsInlineArray | TypeFlags.AttributeCacheComputed) & TypeFlags.IsInlineArray) != 0; - } - } - /// /// Gets a value indicating whether this type implements IDynamicInterfaceCastable /// diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs index f1c794d1455555..90c08433f35b2b 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs @@ -10,7 +10,7 @@ public partial struct GCPointerMap /// /// Computes the GC pointer map for the instance fields of . /// - public static GCPointerMap FromInstanceLayout(DefType type) + public static GCPointerMap FromInstanceLayout(MetadataType type) { Debug.Assert(type.ContainsGCPointers); @@ -21,11 +21,11 @@ public static GCPointerMap FromInstanceLayout(DefType type) return builder.ToGCMap(); } - private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, DefType type, int pointerSize) + private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, MetadataType type, int pointerSize) { if (!type.IsValueType && type.HasBaseType) { - DefType baseType = type.BaseType; + MetadataType baseType = (MetadataType)type.BaseType; GCPointerMapBuilder baseLayoutBuilder = builder.GetInnerBuilder(0, baseType.InstanceByteCount.AsInt); FromInstanceLayoutHelper(ref baseLayoutBuilder, baseType, pointerSize); } @@ -51,7 +51,7 @@ private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, De } else if (fieldType.IsValueType) { - var fieldDefType = (DefType)fieldType; + var fieldDefType = (MetadataType)fieldType; if (fieldDefType.ContainsGCPointers) { for (int i = 0; i < repeat; i++) @@ -69,7 +69,7 @@ private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, De /// /// Computes the GC pointer map of the GC static region of the type. /// - public static GCPointerMap FromStaticLayout(DefType type) + public static GCPointerMap FromStaticLayout(MetadataType type) { GCPointerMapBuilder builder = new GCPointerMapBuilder(type.GCStaticFieldSize.AsInt, type.Context.Target.PointerSize); @@ -87,7 +87,7 @@ public static GCPointerMap FromStaticLayout(DefType type) else { Debug.Assert(fieldType.IsValueType); - var fieldDefType = (DefType)fieldType; + var fieldDefType = (MetadataType)fieldType; if (fieldDefType.ContainsGCPointers) { int pointerSize = type.Context.Target.PointerSize; @@ -105,7 +105,7 @@ public static GCPointerMap FromStaticLayout(DefType type) /// /// Computes the GC pointer map of the thread static region of the type. /// - public static GCPointerMap FromThreadStaticLayout(DefType type) + public static GCPointerMap FromThreadStaticLayout(MetadataType type) { GCPointerMapBuilder builder = new GCPointerMapBuilder(type.ThreadGcStaticFieldSize.AsInt, type.Context.Target.PointerSize); @@ -121,7 +121,7 @@ public static GCPointerMap FromThreadStaticLayout(DefType type) } else if (fieldType.IsValueType) { - var fieldDefType = (DefType)fieldType; + var fieldDefType = (MetadataType)fieldType; if (fieldDefType.ContainsGCPointers) { int pointerSize = type.Context.Target.PointerSize; diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs index 512cad4be34eff..e49b33ef7f3983 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs @@ -91,6 +91,13 @@ public override bool IsSequentialLayout } } + public override bool IsInlineArray => Length > 0; + + public override int GetInlineArrayLength() + { + return (int)Length; + } + public override bool IsBeforeFieldInit { get @@ -173,10 +180,7 @@ public InlineArrayType(ModuleDesc owningModule, MetadataType elementType, uint l public override ClassLayoutMetadata GetClassLayout() { - ClassLayoutMetadata result = default(ClassLayoutMetadata); - result.PackingSize = 0; - result.Size = checked((int)Length * ElementType.GetElementSize().AsInt); - return result; + return default(ClassLayoutMetadata); } public override bool HasCustomAttribute(string attributeNamespace, string attributeName) diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/NativeStructType.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/NativeStructType.cs index 9f5ee9edcb96fd..fcfee2dbab9981 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/NativeStructType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/NativeStructType.cs @@ -68,6 +68,12 @@ public override bool IsExplicitLayout } } + public override int GetInlineArrayLength() + { + Debug.Fail("if this can be an inline array, implement GetInlineArrayLength"); + throw new InvalidOperationException(); + } + public override bool IsSequentialLayout { get diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/PInvokeDelegateWrapper.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/PInvokeDelegateWrapper.cs index c7577641566001..3708dff16c7d17 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/PInvokeDelegateWrapper.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/PInvokeDelegateWrapper.cs @@ -231,6 +231,12 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask) return flags; } + public override int GetInlineArrayLength() + { + Debug.Fail("if this can be an inline array, implement GetInlineArrayLength"); + throw new InvalidOperationException(); + } + private MethodDesc[] _methods; private void InitializeMethods() diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.BoxedTypes.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.BoxedTypes.cs index afc348ef9c89d6..9236e10d2c35a5 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.BoxedTypes.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.BoxedTypes.cs @@ -273,6 +273,12 @@ private sealed partial class BoxedValueType : MetadataType, INonEmittableType public override DefType[] ExplicitlyImplementedInterfaces => Array.Empty(); public override TypeSystemContext Context => ValueTypeRepresented.Context; + public override int GetInlineArrayLength() + { + Debug.Fail("if this can be an inline array, implement GetInlineArrayLength"); + throw new InvalidOperationException(); + } + public BoxedValueType(ModuleDesc owningModule, MetadataType valuetype) { // BoxedValueType has the same genericness as the valuetype it's wrapping. diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.GeneratedAssembly.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.GeneratedAssembly.cs index d18c44ec8706cc..aa2da8a6cd17b6 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.GeneratedAssembly.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.GeneratedAssembly.cs @@ -213,6 +213,12 @@ public override bool IsSequentialLayout } } + public override int GetInlineArrayLength() + { + Debug.Fail("if this can be an inline array, implement GetInlineArrayLength"); + throw new InvalidOperationException(); + } + public override bool IsBeforeFieldInit { get diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs index 262467b79c6ade..62480431a8b375 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs @@ -961,7 +961,7 @@ private static TypeDesc GetActualTemplateTypeForType(NodeFactory factory, TypeDe private ISymbolNode GetStaticsNode(NodeFactory context, out BagElementKind staticsBagKind) { - DefType closestCanonDefType = (DefType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); + MetadataType closestCanonDefType = (MetadataType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); ISymbolNode symbol = context.GCStaticEEType(GCPointerMap.FromStaticLayout(closestCanonDefType)); staticsBagKind = BagElementKind.GcStaticDesc; @@ -970,7 +970,7 @@ private ISymbolNode GetStaticsNode(NodeFactory context, out BagElementKind stati private ISymbolNode GetThreadStaticsNode(NodeFactory context, out BagElementKind staticsBagKind) { - DefType closestCanonDefType = (DefType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); + MetadataType closestCanonDefType = (MetadataType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); ISymbolNode symbol = context.GCStaticEEType(GCPointerMap.FromThreadStaticLayout(closestCanonDefType)); staticsBagKind = BagElementKind.ThreadStaticDesc; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs index eb3097783a9d6a..ba8468c37d737c 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GCRefMapBuilder.cs @@ -279,10 +279,10 @@ private void GcScanValueType(TypeDesc type, in ArgDestination argDest, int delta } } - Debug.Assert(type is DefType); - DefType defType = (DefType)type; - bool isInlineArray = defType.IsInlineArray; - foreach (FieldDesc field in defType.GetFields()) + Debug.Assert(type is MetadataType); + MetadataType structType = (MetadataType)type; + bool isInlineArray = structType.IsInlineArray; + foreach (FieldDesc field in structType.GetFields()) { if (field.IsStatic) continue; @@ -290,7 +290,7 @@ private void GcScanValueType(TypeDesc type, in ArgDestination argDest, int delta if (isInlineArray) { var elementSize = field.FieldType.GetElementSize().AsInt; - var totalSize = defType.InstanceFieldSize.AsInt; + var totalSize = structType.InstanceFieldSize.AsInt; for (int offset = 0; offset < totalSize; offset += elementSize) { diff --git a/src/coreclr/tools/dotnet-pgo/TypeRefTypeSystem/TypeRefTypeSystemType.cs b/src/coreclr/tools/dotnet-pgo/TypeRefTypeSystem/TypeRefTypeSystemType.cs index 40fff461bdfaa9..e1929cf06b4e80 100644 --- a/src/coreclr/tools/dotnet-pgo/TypeRefTypeSystem/TypeRefTypeSystemType.cs +++ b/src/coreclr/tools/dotnet-pgo/TypeRefTypeSystem/TypeRefTypeSystemType.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics; using System.Collections.Generic; using System.Linq; using System.Text; @@ -250,5 +251,11 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask) } protected override MethodImplRecord[] ComputeVirtualMethodImplsForType() => throw new NotImplementedException(); + + public override int GetInlineArrayLength() + { + Debug.Fail("if this can be an inline array, implement GetInlineArrayLength"); + throw new InvalidOperationException(); + } } } From 2d403ee3fc7a879fbb0c8cf1bade78626d45d8bc Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 9 Mar 2023 16:34:37 -0800 Subject: [PATCH 26/30] use type.Context.Target.PointerSize --- .../Utilities/GCPointerMap.Algorithm.cs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs index 90c08433f35b2b..a03addd6632b3a 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/Utilities/GCPointerMap.Algorithm.cs @@ -14,20 +14,19 @@ public static GCPointerMap FromInstanceLayout(MetadataType type) { Debug.Assert(type.ContainsGCPointers); - int pointerSize = type.Context.Target.PointerSize; - GCPointerMapBuilder builder = new GCPointerMapBuilder(type.InstanceByteCount.AsInt, pointerSize); - FromInstanceLayoutHelper(ref builder, type, pointerSize); + GCPointerMapBuilder builder = new GCPointerMapBuilder(type.InstanceByteCount.AsInt, type.Context.Target.PointerSize); + FromInstanceLayoutHelper(ref builder, type); return builder.ToGCMap(); } - private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, MetadataType type, int pointerSize) + private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, MetadataType type) { if (!type.IsValueType && type.HasBaseType) { MetadataType baseType = (MetadataType)type.BaseType; GCPointerMapBuilder baseLayoutBuilder = builder.GetInnerBuilder(0, baseType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref baseLayoutBuilder, baseType, pointerSize); + FromInstanceLayoutHelper(ref baseLayoutBuilder, baseType); } int repeat = 1; @@ -46,7 +45,7 @@ private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, Me { for (int i = 0; i < repeat; i++) { - builder.MarkGCPointer(field.Offset.AsInt + pointerSize * i); + builder.MarkGCPointer(field.Offset.AsInt + type.Context.Target.PointerSize * i); } } else if (fieldType.IsValueType) @@ -59,7 +58,7 @@ private static void FromInstanceLayoutHelper(ref GCPointerMapBuilder builder, Me int fieldSize = fieldDefType.InstanceByteCount.AsInt; GCPointerMapBuilder innerBuilder = builder.GetInnerBuilder(field.Offset.AsInt + fieldSize * i, fieldSize); - FromInstanceLayoutHelper(ref innerBuilder, fieldDefType, pointerSize); + FromInstanceLayoutHelper(ref innerBuilder, fieldDefType); } } } @@ -90,10 +89,9 @@ public static GCPointerMap FromStaticLayout(MetadataType type) var fieldDefType = (MetadataType)fieldType; if (fieldDefType.ContainsGCPointers) { - int pointerSize = type.Context.Target.PointerSize; GCPointerMapBuilder innerBuilder = builder.GetInnerBuilder(field.Offset.AsInt, fieldDefType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref innerBuilder, fieldDefType, pointerSize); + FromInstanceLayoutHelper(ref innerBuilder, fieldDefType); } } } @@ -124,10 +122,9 @@ public static GCPointerMap FromThreadStaticLayout(MetadataType type) var fieldDefType = (MetadataType)fieldType; if (fieldDefType.ContainsGCPointers) { - int pointerSize = type.Context.Target.PointerSize; GCPointerMapBuilder innerBuilder = builder.GetInnerBuilder(field.Offset.AsInt, fieldDefType.InstanceByteCount.AsInt); - FromInstanceLayoutHelper(ref innerBuilder, fieldDefType, pointerSize); + FromInstanceLayoutHelper(ref innerBuilder, fieldDefType); } } } From f6f6acc8783862768aef3c41a86e5eeb376d87d3 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Thu, 9 Mar 2023 18:22:26 -0800 Subject: [PATCH 27/30] lost resources change --- .../Common/TypeSystem/Common/Properties/Resources.resx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/Properties/Resources.resx b/src/coreclr/tools/Common/TypeSystem/Common/Properties/Resources.resx index 6abeabd7b0ea1d..6a70612fc391bf 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/Properties/Resources.resx +++ b/src/coreclr/tools/Common/TypeSystem/Common/Properties/Resources.resx @@ -129,6 +129,15 @@ Failed to load type '{0}' from assembly '{1}' because of field offset '{2}' + + InlineArrayAttribute requires that the target type has a single instance field. Type: '{0}'. Assembly: '{1}'. + + + InlineArrayAttribute requires that the length argument is greater than 0. Type: '{0}'. Assembly: '{1}'. + + + InlineArrayAttribute cannot be applied to a type with explicit layout. Type: '{0}'. Assembly: '{1}'.' + Array of type '{0}' from assembly '{1}' cannot be created because base value type is too large From d68cfa2b07e3ce08698b21552629410038bbb1a1 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Sat, 11 Mar 2023 20:24:48 -0800 Subject: [PATCH 28/30] fix for x86 --- .../tools/Common/JitInterface/CorInfoImpl.cs | 17 ++++++++++------- src/coreclr/vm/jitinterface.cpp | 19 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 3807ce72c54bc0..d8f9a37857d479 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2291,14 +2291,17 @@ private int GatherClassGCLayout(MetadataType type, byte* gcPtrs) if (isInlineArray) { - Debug.Assert(field.Offset.AsInt == 0); - int totalLayoutSize = type.GetElementSize().AsInt / PointerSize; - int elementLayoutSize = fieldType.GetElementSize().AsInt / PointerSize; - int gcPointersInElement = result; - for (int offset = elementLayoutSize; offset < totalLayoutSize; offset += elementLayoutSize) + if (result > 0) { - Buffer.MemoryCopy(gcPtrs, gcPtrs + offset, elementLayoutSize, elementLayoutSize); - result += gcPointersInElement; + Debug.Assert(field.Offset.AsInt == 0); + int totalLayoutSize = type.GetElementSize().AsInt / PointerSize; + int elementLayoutSize = fieldType.GetElementSize().AsInt / PointerSize; + int gcPointersInElement = result; + for (int offset = elementLayoutSize; offset < totalLayoutSize; offset += elementLayoutSize) + { + Buffer.MemoryCopy(gcPtrs, gcPtrs + offset, elementLayoutSize, elementLayoutSize); + result += gcPointersInElement; + } } // inline array has only one element field diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 38ca24f967da36..487a01be1a1dc4 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -2191,14 +2191,17 @@ static unsigned ComputeGCLayout(MethodTable* pMT, BYTE* gcPtrs) if (isInlineArray) { - _ASSERTE(pFD->GetOffset() == 0); - DWORD totalLayoutSize = pMT->GetNumInstanceFieldBytes() / TARGET_POINTER_SIZE; - DWORD elementLayoutSize = pFD->GetSize() / TARGET_POINTER_SIZE; - DWORD gcPointersInElement = result; - for (DWORD offset = elementLayoutSize; offset < totalLayoutSize; offset += elementLayoutSize) - { - memcpy(gcPtrs + offset, gcPtrs, elementLayoutSize); - result += gcPointersInElement; + if (result > 0) + { + _ASSERTE(pFD->GetOffset() == 0); + DWORD totalLayoutSize = pMT->GetNumInstanceFieldBytes() / TARGET_POINTER_SIZE; + DWORD elementLayoutSize = pFD->GetSize() / TARGET_POINTER_SIZE; + DWORD gcPointersInElement = result; + for (DWORD offset = elementLayoutSize; offset < totalLayoutSize; offset += elementLayoutSize) + { + memcpy(gcPtrs + offset, gcPtrs, elementLayoutSize); + result += gcPointersInElement; + } } // inline array has only one element field From ebc698bfcdf9d543b1948226c85c1e27faab23c6 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 13 Mar 2023 10:34:57 -0700 Subject: [PATCH 29/30] Do not make InlineArrayType an inline array just yet. --- .../Common/TypeSystem/Common/MetadataType.cs | 2 +- .../TypeSystem/Interop/IL/InlineArrayType.cs | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs index 1d05990af76a5b..722aa66a878f01 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataType.cs @@ -95,7 +95,7 @@ public virtual bool IsModuleType /// /// Gets a value indicating whether this is an inline array type /// - public virtual bool IsInlineArray + public bool IsInlineArray { get { diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs index e49b33ef7f3983..78fd7bc25a4d2c 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/InlineArrayType.cs @@ -91,13 +91,6 @@ public override bool IsSequentialLayout } } - public override bool IsInlineArray => Length > 0; - - public override int GetInlineArrayLength() - { - return (int)Length; - } - public override bool IsBeforeFieldInit { get @@ -180,7 +173,10 @@ public InlineArrayType(ModuleDesc owningModule, MetadataType elementType, uint l public override ClassLayoutMetadata GetClassLayout() { - return default(ClassLayoutMetadata); + ClassLayoutMetadata result = default(ClassLayoutMetadata); + result.PackingSize = 0; + result.Size = checked((int)Length * ElementType.GetElementSize().AsInt); + return result; } public override bool HasCustomAttribute(string attributeNamespace, string attributeName) @@ -252,6 +248,12 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask) return flags; } + public override int GetInlineArrayLength() + { + Debug.Fail("when this is backed by an actual inline array, implement GetInlineArrayLength"); + throw new InvalidOperationException(); + } + private void InitializeMethods() { MethodDesc[] methods = new MethodDesc[] { From 9373c775eb9b27a15b0a2d8fc46e896f75ecf370 Mon Sep 17 00:00:00 2001 From: vsadov <8218165+VSadov@users.noreply.github.com> Date: Mon, 13 Mar 2023 11:16:12 -0700 Subject: [PATCH 30/30] CORINFO_FLG_INDEXABLE_FIELDS --- src/coreclr/inc/corinfo.h | 4 ++-- src/coreclr/jit/compiler.hpp | 5 +++++ src/coreclr/jit/lclvars.cpp | 5 +++++ src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 2 +- src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs | 4 ++-- src/coreclr/vm/jitinterface.cpp | 2 +- 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index d1edff70ea24d8..58fed256c87245 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -852,11 +852,11 @@ enum CorInfoFlag CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently) CORINFO_FLG_OVERLAPPING_FIELDS = 0x00100000, // struct or class has fields that overlap (aka union) CORINFO_FLG_INTERFACE = 0x00200000, // it is an interface - CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't ask field info (used for types outside of AOT compilation version bubble and inline arrays) + CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't ask field info (used for types outside of AOT compilation version bubble) CORINFO_FLG_CUSTOMLAYOUT = 0x00800000, // does this struct have custom layout? CORINFO_FLG_CONTAINS_GC_PTR = 0x01000000, // does the class contain a gc ptr ? CORINFO_FLG_DELEGATE = 0x02000000, // is this a subclass of delegate or multicast delegate ? - // CORINFO_FLG_UNUSED = 0x04000000, + CORINFO_FLG_INDEXABLE_FIELDS = 0x04000000, // struct fields may be accessed via indexing (used for inline arrays) CORINFO_FLG_BYREF_LIKE = 0x08000000, // it is byref-like value type CORINFO_FLG_VARIANCE = 0x10000000, // MethodTable::HasVariance (sealed does *not* mean uncast-able) CORINFO_FLG_BEFOREFIELDINIT = 0x20000000, // Additional flexibility for when to run .cctor (see code:#ClassConstructionFlags) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 94dcf70963a413..711675ee7038db 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4369,6 +4369,11 @@ inline static bool StructHasDontDigFieldsFlagSet(DWORD attribs) return ((attribs & CORINFO_FLG_DONT_DIG_FIELDS) != 0); } +inline static bool StructHasIndexableFields(DWORD attribs) +{ + return ((attribs & CORINFO_FLG_INDEXABLE_FIELDS) != 0); +} + //------------------------------------------------------------------------------ // DEBUG_DESTROY_NODE: sets value of tree to garbage to catch extra references // diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 7bca65eda06b80..bb97eaf8d988a5 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -1772,6 +1772,11 @@ bool Compiler::StructPromotionHelper::CanPromoteStructType(CORINFO_CLASS_HANDLE return false; } + if (StructHasIndexableFields(typeFlags)) + { + return false; + } + // Don't struct promote if we have an CUSTOMLAYOUT flag on an HFA type if (StructHasCustomLayout(typeFlags) && compiler->IsHfa(typeHnd)) { diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index d8f9a37857d479..49468f41facbef 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2012,7 +2012,7 @@ private uint getClassAttribsInternal(TypeDesc type) result |= CorInfoFlag.CORINFO_FLG_UNSAFE_VALUECLASS; if (metadataType.IsInlineArray) - result |= CorInfoFlag.CORINFO_FLG_DONT_DIG_FIELDS; + result |= CorInfoFlag.CORINFO_FLG_INDEXABLE_FIELDS; } if (type.IsCanonicalSubtype(CanonicalFormKind.Any)) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index d3e0ef50b34372..b2b57e7970a5ed 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -602,11 +602,11 @@ public enum CorInfoFlag : uint CORINFO_FLG_ARRAY = 0x00080000, // class is an array class (initialized differently) CORINFO_FLG_OVERLAPPING_FIELDS = 0x00100000, // struct or class has fields that overlap (aka union) CORINFO_FLG_INTERFACE = 0x00200000, // it is an interface - CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't try to ask about fields outside of AOT compilation version bubble or inline arrays + CORINFO_FLG_DONT_DIG_FIELDS = 0x00400000, // don't try to ask about fields outside of AOT compilation version bubble CORINFO_FLG_CUSTOMLAYOUT = 0x00800000, // does this struct have custom layout? CORINFO_FLG_CONTAINS_GC_PTR = 0x01000000, // does the class contain a gc ptr ? CORINFO_FLG_DELEGATE = 0x02000000, // is this a subclass of delegate or multicast delegate ? - // CORINFO_FLG_UNUSED = 0x04000000, + CORINFO_FLG_INDEXABLE_FIELDS = 0x04000000, // struct fields may be accessed via indexing (used for inline arrays) CORINFO_FLG_BYREF_LIKE = 0x08000000, // it is byref-like value type CORINFO_FLG_VARIANCE = 0x10000000, // MethodTable::HasVariance (sealed does *not* mean uncast-able) CORINFO_FLG_BEFOREFIELDINIT = 0x20000000, // Additional flexibility for when to run .cctor (see code:#ClassConstructionFlags) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 487a01be1a1dc4..3e60391f4ddc0d 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -3573,7 +3573,7 @@ uint32_t CEEInfo::getClassAttribsInternal (CORINFO_CLASS_HANDLE clsHnd) ret |= CORINFO_FLG_OVERLAPPING_FIELDS; if (pClass->IsInlineArray()) - ret |= CORINFO_FLG_DONT_DIG_FIELDS; + ret |= CORINFO_FLG_INDEXABLE_FIELDS; if (VMClsHnd.IsCanonicalSubtype()) ret |= CORINFO_FLG_SHAREDINST;