Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

CoreCLR support for InlineArrayAttribute. (struct layout part) #82744

Merged
merged 30 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
da7b00b
initial port from the prototype
VSadov Feb 25, 2023
170a306
parse the attribute in MT builder
VSadov Feb 26, 2023
bb1c3db
validate replicated size
VSadov Feb 26, 2023
0a4cd78
aot changes
VSadov Feb 26, 2023
6eeb887
refmap
VSadov Feb 27, 2023
f0d156b
validate total size in ilc
VSadov Feb 27, 2023
470aee2
add the actual attribute
VSadov Feb 28, 2023
78ab523
Apply suggestions from code review
VSadov Feb 28, 2023
a452be7
add a small use of InlineArray in CoreLib - to get some crossgen cove…
VSadov Feb 28, 2023
2801f50
a few more uses in CorLib
VSadov Mar 1, 2023
e3284d1
fix for sequential layout
VSadov Mar 1, 2023
4ff0a25
Standardize on use of "InlineArray" in the implementation
VSadov Mar 2, 2023
b406dfe
simpler layout replication
VSadov Mar 2, 2023
7f197d6
some initial tests (will add more)
VSadov Mar 3, 2023
1852993
more tests
VSadov Mar 4, 2023
295caf1
limit the max size of array instance to 1MiB
VSadov Mar 4, 2023
e7738ea
fix an assert in importercalls.cpp
VSadov Mar 4, 2023
065467b
"result" in GC layout should track the pointer count, not the size
VSadov Mar 4, 2023
62b20c8
error messages
VSadov Mar 8, 2023
01e2e17
fixed uses of "value array" in comments.
VSadov Mar 8, 2023
f844772
PR feedback on StackAllocedArguments
VSadov Mar 8, 2023
1b5f608
use the same size limit for inline arrays in the typeloader
VSadov Mar 8, 2023
bf676e4
more PR feedback
VSadov Mar 8, 2023
7d42fbc
remove SetCannotBeBlittedByObjectCloner
VSadov Mar 8, 2023
7fff2be
moving GetInlineArrayLength to MetadataType and related changes
VSadov Mar 10, 2023
2d403ee
use type.Context.Target.PointerSize
VSadov Mar 10, 2023
f6f6acc
lost resources change
VSadov Mar 10, 2023
d68cfa2
fix for x86
VSadov Mar 12, 2023
ebc698b
Do not make InlineArrayType an inline array just yet.
VSadov Mar 13, 2023
9373c77
CORINFO_FLG_INDEXABLE_FIELDS
VSadov Mar 13, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ Signature LazyCreateSignature()
{
Debug.Assert(parameters != null);
StackAllocedArguments argStorage = default;
Span<object?> copyOfParameters = new(ref argStorage._arg0, argCount);
Span<ParameterCopyBackAction> shouldCopyBackParameters = new(ref argStorage._copyBack0, argCount);
Span<object?> copyOfParameters = argStorage._args.AsSpan(argCount);
Span<ParameterCopyBackAction> shouldCopyBackParameters = argStorage._copyBacks.AsSpan(argCount);

StackAllocatedByRefs byrefStorage = default;
#pragma warning disable CS8500
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,9 @@ public override MethodImplAttributes GetMethodImplementationFlags()
unsafe
{
StackAllocedArguments argStorage = default;
Span<object?> copyOfParameters = new(ref argStorage._arg0, 1);
Span<object?> copyOfParameters = argStorage._args.AsSpan(1);
ReadOnlySpan<object?> parameters = new(in parameter);
Span<ParameterCopyBackAction> shouldCopyBackParameters = new(ref argStorage._copyBack0, 1);
Span<ParameterCopyBackAction> shouldCopyBackParameters = argStorage._copyBacks.AsSpan(1);

StackAllocatedByRefs byrefStorage = default;
#pragma warning disable 8500
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/dlls/mscorrc/mscorrc.rc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/dlls/mscorrc/resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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, 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)
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)
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
8 changes: 4 additions & 4 deletions src/coreclr/tools/Common/Internal/Runtime/GCDescEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -84,7 +84,7 @@ public static void EncodeGCDesc<T>(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);
Expand All @@ -101,7 +101,7 @@ public static void EncodeGCDesc<T>(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.
Expand Down
29 changes: 26 additions & 3 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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_INDEXABLE_FIELDS;
}

if (type.IsCanonicalSubtype(CanonicalFormKind.Any))
Expand Down Expand Up @@ -2240,9 +2243,10 @@ 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;

foreach (var field in type.GetFields())
{
Expand Down Expand Up @@ -2278,12 +2282,31 @@ private int GatherClassGCLayout(TypeDesc type, byte* gcPtrs)

if (gcType == CorInfoGCType.TYPE_GC_OTHER)
{
result += GatherClassGCLayout(fieldType, fieldGcPtrs);
result += GatherClassGCLayout((MetadataType)fieldType, fieldGcPtrs);
}
else
{
result += MarkGcField(fieldGcPtrs, gcType);
}

if (isInlineArray)
{
if (result > 0)
{
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
break;
}
}
return result;
}
Expand All @@ -2292,7 +2315,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;

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ public enum CorInfoFlag : uint
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,11 @@ public partial class InstantiatedType
if (_typeDef.IsIntrinsic)
flags |= TypeFlags.IsIntrinsic;
}

partial void AddComputedInlineArrayFlag(ref TypeFlags flags)
Copy link
Member

Choose a reason for hiding this comment

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

Technically this should go to a new file TypeDesc.Metadata.cs, but maybe this is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically this should go to a new file TypeDesc.Metadata.cs, but maybe this is fine.

I will not change this part then.
Let me know if you rethink this.

{
if (((MetadataType)_typeDef).IsInlineArray)
flags |= TypeFlags.IsInlineArray;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ public enum ExceptionStringID
ClassLoadValueClassTooLarge,
ClassLoadRankTooLarge,

ClassLoadInlineArrayFieldCount,
ClassLoadInlineArrayLength,
ClassLoadInlineArrayExplicit,

// MissingMethodException
MissingMethod,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public override bool HasCustomAttribute(string attributeNamespace, string attrib
return _typeDef.HasCustomAttribute(attributeNamespace, attributeName);
}

public override int GetInlineArrayLength()
{
return _typeDef.GetInlineArrayLength();
}

public override MetadataType GetNestedType(string name)
{
// Return the result from the typical type definition.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -106,6 +110,8 @@ protected override TypeFlags ComputeTypeFlags(TypeFlags mask)
if (_typeDef.IsByRefLike)
flags |= TypeFlags.IsByRefLike;

AddComputedInlineArrayFlag(ref flags);

AddComputedIntrinsicFlag(ref flags);
}

Expand Down