Skip to content

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

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
VSadov merged 30 commits into from
Mar 16, 2023
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
parse the attribute in MT builder
  • Loading branch information
@VSadov
VSadov committed Mar 10, 2023
commit 170a306a29aaafa3f9504b7e3f3e89a3454dea5e
61 changes: 41 additions & 20 deletions src/coreclr/vm/methodtablebuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

@mangod9 mangod9 Mar 4, 2023

Choose a reason for hiding this comment

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

should there be a non-generic error here?

Copy link
Member Author

@VSadov VSadov Mar 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

yes, ideally the same in the native loader and AOT compilers

}

if (cbVal >= (sizeof(INT32) + 2))
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT Mar 4, 2023

Choose a reason for hiding this comment

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

Please comment what this is checking.

{
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();
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/vm/siginfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
{
Expand All @@ -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());
}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/wellknownattributes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ enum class WellKnownAttribute : DWORD
WinRTMarshalingBehaviorAttribute,
PreserveBaseOverridesAttribute,
ObjectiveCTrackedTypeAttribute,
InlineArrayAttribute,

CountOfWellKnownAttributes
};
Expand Down Expand Up @@ -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
Expand Down