Skip to content

Commit 876ab61

Browse files
authored
Unify JIT interface getTypeLayout between VM and crossgen2; refine what is considered "significant padding" (#88238)
* Stop considering auto layout types to have significant padding in the VM. This was already the behavior in crossgen2. This fixes one of the points of #71711. * Remove special case where we never considered structs with GC pointers to have significant padding. After the above change this has no additional diffs. * Fix the inline array layout expansion; the numFields of the inline array node was computed incorrectly, and the parent indices of descendants were not updated correctly Example C#: ```csharp private (long, int) _tuple; [MethodImpl(MethodImplOptions.NoInlining)] private void Foo() { _tuple = (5, 10); } ``` Before: ```asm ; V00 this [V00,T00] ( 3, 3 ) ref -> rcx this class-hnd single-def ;# V01 OutArgs [V01 ] ( 1, 1 ) struct ( 0) [rsp+00H] do-not-enreg[XS] addr-exposed "OutgoingArgSpace" ; V02 tmp1 [V02 ] ( 4, 8 ) struct (16) [rsp+08H] do-not-enreg[SB] ld-addr-op "NewObj constructor temp" ; V03 tmp2 [V03,T01] ( 3, 5 ) long -> [rsp+08H] do-not-enreg[] "field V02.Item1 (fldOffset=0x0)" P-DEP ; V04 tmp3 [V04,T02] ( 3, 5 ) int -> [rsp+10H] do-not-enreg[] "field V02.Item2 (fldOffset=0x8)" P-DEP ; ; Lcl frame size = 24 G_M52879_IG01: ;; offset=0000H sub rsp, 24 vzeroupper ;; size=7 bbWeight=1 PerfScore 1.25 G_M52879_IG02: ;; offset=0007H vxorps xmm0, xmm0, xmm0 vmovups xmmword ptr [rsp+08H], xmm0 mov qword ptr [rsp+08H], 5 mov dword ptr [rsp+10H], 10 vmovups xmm0, xmmword ptr [rsp+08H] vmovups xmmword ptr [rcx+08H], xmm0 ;; size=38 bbWeight=1 PerfScore 8.33 G_M52879_IG03: ;; offset=002DH add rsp, 24 ret ;; size=5 bbWeight=1 PerfScore 1.25 ; Total bytes of code 50, prolog size 7, PerfScore 15.83, instruction count 10, allocated bytes for code 50 (MethodHash=1da13170) for method Program:Foo():this (FullOpts) ``` After: ```asm ; V00 this [V00,T00] ( 4, 4 ) ref -> rcx this class-hnd single-def ;# V01 OutArgs [V01 ] ( 1, 1 ) struct ( 0) [rsp+00H] do-not-enreg[XS] addr-exposed "OutgoingArgSpace" ;* V02 tmp1 [V02 ] ( 0, 0 ) struct (16) zero-ref ld-addr-op "NewObj constructor temp" ;* V03 tmp2 [V03,T01] ( 0, 0 ) long -> zero-ref "field V02.Item1 (fldOffset=0x0)" P-INDEP ;* V04 tmp3 [V04,T02] ( 0, 0 ) int -> zero-ref "field V02.Item2 (fldOffset=0x8)" P-INDEP ; ; Lcl frame size = 0 G_M52879_IG01: ;; offset=0000H ;; size=0 bbWeight=1 PerfScore 0.00 G_M52879_IG02: ;; offset=0000H mov qword ptr [rcx+08H], 5 mov dword ptr [rcx+10H], 10 ;; size=15 bbWeight=1 PerfScore 2.00 G_M52879_IG03: ;; offset=000FH ret ;; size=1 bbWeight=1 PerfScore 1.00 ; Total bytes of code 16, prolog size 0, PerfScore 4.60, instruction count 3, allocated bytes for code 16 (MethodHash=1da13170) for method Program:Foo():this (FullOpts) ```
1 parent abedbc9 commit 876ab61

File tree

2 files changed

+62
-32
lines changed

2 files changed

+62
-32
lines changed

src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2368,7 +2368,7 @@ private GetTypeLayoutResult GetTypeLayoutHelper(MetadataType type, uint parentIn
23682368
parNode->size = (uint)type.GetElementSize().AsInt;
23692369
parNode->numFields = 0;
23702370
parNode->type = CorInfoType.CORINFO_TYPE_VALUECLASS;
2371-
parNode->hasSignificantPadding = false;
2371+
parNode->hasSignificantPadding = type.IsExplicitLayout || (type.IsSequentialLayout && type.GetClassLayout().Size != 0);
23722372

23732373
#if READYTORUN
23742374
// The contract of getTypeLayout is carefully crafted to still
@@ -2381,7 +2381,7 @@ private GetTypeLayoutResult GetTypeLayoutHelper(MetadataType type, uint parentIn
23812381
// amenable to the optimizations that this unlocks if they already
23822382
// went through EncodeFieldBaseOffset.
23832383
//
2384-
if (!_compilation.IsLayoutFixedInCurrentVersionBubble(type))
2384+
if (!parNode->hasSignificantPadding && !_compilation.IsLayoutFixedInCurrentVersionBubble(type))
23852385
{
23862386
// For types without fixed layout the JIT is not allowed to
23872387
// rely on padding bits being insignificant, since fields could
@@ -2391,14 +2391,6 @@ private GetTypeLayoutResult GetTypeLayoutHelper(MetadataType type, uint parentIn
23912391
}
23922392
#endif
23932393

2394-
if (type.IsExplicitLayout || (type.IsSequentialLayout && type.GetClassLayout().Size != 0) || type.IsInlineArray)
2395-
{
2396-
if (!type.ContainsGCPointers && !type.IsByRefLike)
2397-
{
2398-
parNode->hasSignificantPadding = true;
2399-
}
2400-
}
2401-
24022394
// The intrinsic SIMD/HW SIMD types have a lot of fields that the JIT does
24032395
// not care about since they are considered primitives by the JIT.
24042396
if (type.IsIntrinsic)
@@ -2472,19 +2464,43 @@ private GetTypeLayoutResult GetTypeLayoutHelper(MetadataType type, uint parentIn
24722464
int elemSize = fieldType.GetElementSize().AsInt;
24732465
int arrSize = type.GetElementSize().AsInt;
24742466

2467+
// Number of fields added for each element, including all
2468+
// subfields. For example, for ValueTuple<int, int>[4]:
2469+
// [ 0]: InlineArray parent = -1
2470+
// [ 1]: ValueTuple<int, int> parent = 0 -
2471+
// [ 2]: int parent = 1 |
2472+
// [ 3]: int parent = 1 |
2473+
// [ 4]: ValueTuple<int, int> parent = 0 - stride = 3
2474+
// [ 5]: int parent = 4
2475+
// [ 6]: int parent = 4
2476+
// [ 7]: ValueTuple<int, int> parent = 0
2477+
// [ 8]: int parent = 7
2478+
// [ 9]: int parent = 7
2479+
// [10]: ValueTuple<int, int> parent = 0
2480+
// [11]: int parent = 10
2481+
// [12]: int parent = 10
2482+
uint elemFieldsStride = (uint)*numTreeNodes - (structNodeIndex + 1);
2483+
2484+
// Now duplicate the fields of the previous entry for each
2485+
// additional element. For each entry we have to update the
2486+
// offset and the parent index.
24752487
for (int elemOffset = elemSize; elemOffset < arrSize; elemOffset += elemSize)
24762488
{
2477-
for (nuint templateTreeNodeIndex = structNodeIndex + 1; templateTreeNodeIndex < treeNodeEnd; templateTreeNodeIndex++)
2489+
nuint prevElemStart = *numTreeNodes - elemFieldsStride;
2490+
for (nuint i = 0; i < elemFieldsStride; i++)
24782491
{
24792492
if (*numTreeNodes >= maxTreeNodes)
24802493
return GetTypeLayoutResult.Partial;
24812494

24822495
CORINFO_TYPE_LAYOUT_NODE* treeNode = &treeNodes[(*numTreeNodes)++];
2483-
*treeNode = treeNodes[templateTreeNodeIndex];
2484-
treeNode->offset += (uint)elemOffset;
2485-
2486-
parNode->numFields++;
2496+
*treeNode = treeNodes[prevElemStart + i];
2497+
treeNode->offset += (uint)elemSize;
2498+
// The first field points back to the inline array
2499+
// and has no bias; the rest of them do.
2500+
treeNode->parent += (i == 0) ? 0 : elemFieldsStride;
24872501
}
2502+
2503+
parNode->numFields++;
24882504
}
24892505
}
24902506
}

src/coreclr/vm/jitinterface.cpp

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2106,20 +2106,9 @@ static GetTypeLayoutResult GetTypeLayoutHelper(
21062106
parNode.size = pMT->GetNumInstanceFieldBytes();
21072107
parNode.numFields = 0;
21082108
parNode.type = CorInfoType::CORINFO_TYPE_VALUECLASS;
2109-
parNode.hasSignificantPadding = false;
21102109

21112110
EEClass* pClass = pMT->GetClass();
2112-
if (pClass->IsNotTightlyPacked() && (!pClass->IsManagedSequential() || pClass->HasExplicitSize() || pClass->IsInlineArray()))
2113-
{
2114-
// Historically on the JIT side we did not consider types with GC
2115-
// pointers to have significant padding, even when they have explicit
2116-
// layout attributes. This retains the more liberal treatment and
2117-
// lets the JIT still optimize these cases.
2118-
if (!pMT->ContainsPointers() && pMT != g_TypedReferenceMT)
2119-
{
2120-
parNode.hasSignificantPadding = true;
2121-
}
2122-
}
2111+
parNode.hasSignificantPadding = pClass->HasExplicitFieldOffsetLayout() || pClass->HasExplicitSize();
21232112

21242113
// The intrinsic SIMD/HW SIMD types have a lot of fields that the JIT does
21252114
// not care about since they are considered primitives by the JIT.
@@ -2181,21 +2170,46 @@ static GetTypeLayoutResult GetTypeLayoutHelper(
21812170
uint32_t elemSize = pFD->GetSize();
21822171
uint32_t arrSize = pMT->GetNumInstanceFieldBytes();
21832172

2173+
// Number of fields added for each element, including all
2174+
// subfields. For example, for ValueTuple<int, int>[4]:
2175+
// [ 0]: InlineArray
2176+
// [ 1]: ValueTuple<int, int> parent = 0 -
2177+
// [ 2]: int parent = 1 |
2178+
// [ 3]: int parent = 1 |
2179+
// [ 4]: ValueTuple<int, int> parent = 0 - stride = 3
2180+
// [ 5]: int parent = 4
2181+
// [ 6]: int parent = 4
2182+
// [ 7]: ValueTuple<int, int> parent = 0
2183+
// [ 8]: int parent = 7
2184+
// [ 9]: int parent = 7
2185+
// [10]: ValueTuple<int, int> parent = 0
2186+
// [11]: int parent = 10
2187+
// [12]: int parent = 10
2188+
2189+
unsigned elemFieldsStride = (unsigned)*numTreeNodes - (structNodeIndex + 1);
2190+
2191+
// Now duplicate the fields of the previous entry for each
2192+
// additional element. For each entry we have to update the offset
2193+
// and the parent index.
21842194
for (uint32_t elemOffset = elemSize; elemOffset < arrSize; elemOffset += elemSize)
21852195
{
2186-
for (size_t templateTreeNodeIndex = structNodeIndex + 1; templateTreeNodeIndex < treeNodeEnd; templateTreeNodeIndex++)
2196+
size_t prevElemStart = *numTreeNodes - elemFieldsStride;
2197+
for (size_t i = 0; i < elemFieldsStride; i++)
21872198
{
21882199
if (*numTreeNodes >= maxTreeNodes)
21892200
{
21902201
return GetTypeLayoutResult::Partial;
21912202
}
21922203

21932204
CORINFO_TYPE_LAYOUT_NODE& treeNode = treeNodes[(*numTreeNodes)++];
2194-
treeNode = treeNodes[templateTreeNodeIndex];
2195-
treeNode.offset += elemOffset;
2196-
2197-
parNode.numFields++;
2205+
treeNode = treeNodes[prevElemStart + i];
2206+
treeNode.offset += elemSize;
2207+
// The first field points back to the inline array and has
2208+
// no bias; the rest of them do.
2209+
treeNode.parent += (i == 0) ? 0 : elemFieldsStride;
21982210
}
2211+
2212+
parNode.numFields++;
21992213
}
22002214
}
22012215
}

0 commit comments

Comments
 (0)