Skip to content

Commit 39b3b90

Browse files
github-actions[bot]Copilotjkoritzinsky
authored
[release/10.0-preview6] Record struct packing even when falling back to auto layout (#116770)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com> Co-authored-by: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
1 parent 9b493d2 commit 39b3b90

File tree

4 files changed

+113
-37
lines changed

4 files changed

+113
-37
lines changed

‎src/coreclr/vm/class.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,12 @@ class EEClassLayoutInfo
460460
m_ManagedLargestAlignmentRequirementOfAllMembers = alignment;
461461
}
462462

463+
void SetPackingSize(BYTE cbPackingSize)
464+
{
465+
LIMITED_METHOD_CONTRACT;
466+
m_cbPackingSize = cbPackingSize;
467+
}
468+
463469
ULONG InitializeSequentialFieldLayout(
464470
FieldDesc* pFields,
465471
MethodTable** pByValueClassCache,
@@ -488,12 +494,6 @@ class EEClassLayoutInfo
488494
: (m_bFlags & ~e_ZERO_SIZED);
489495
}
490496

491-
void SetPackingSize(BYTE cbPackingSize)
492-
{
493-
LIMITED_METHOD_CONTRACT;
494-
m_cbPackingSize = cbPackingSize;
495-
}
496-
497497
UINT32 SetInstanceBytesSize(UINT32 size)
498498
{
499499
LIMITED_METHOD_CONTRACT;

‎src/coreclr/vm/classlayoutinfo.cpp‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -891,25 +891,37 @@ EEClassNativeLayoutInfo* EEClassNativeLayoutInfo::CollectNativeLayoutFieldMetada
891891

892892
pNativeLayoutInfo->m_numFields = numTotalInstanceFields;
893893

894-
BYTE parentAlignmentRequirement = 0;
894+
// If there's no parent, pretend that the parent alignment requirement is 1.
895+
BYTE parentAlignmentRequirement = 1;
895896
if (pParentLayoutInfo != nullptr)
896897
{
897898
parentAlignmentRequirement = pParentLayoutInfo->GetLargestAlignmentRequirement();
898899
}
899900

901+
BYTE packingSize = pMT->GetLayoutInfo()->GetPackingSize();
902+
if (packingSize == 0)
903+
{
904+
packingSize = DEFAULT_PACKING_SIZE;
905+
}
906+
900907
BYTE fieldAlignmentRequirement = 0;
901908
// Now compute the native size of each field
902909
for (LayoutRawFieldInfo* pfwalk = pInfoArray; pfwalk->m_token != mdFieldDefNil; pfwalk++)
903910
{
904911
pfwalk->m_placement.m_size = pfwalk->m_nfd.NativeSize();
905-
pfwalk->m_placement.m_alignment = pfwalk->m_nfd.AlignmentRequirement();
912+
// Allow the packing size to override a looser alignment requirement.
913+
pfwalk->m_placement.m_alignment = min(packingSize, (BYTE)pfwalk->m_nfd.AlignmentRequirement());
906914
if (pfwalk->m_placement.m_alignment > fieldAlignmentRequirement)
907915
{
908916
fieldAlignmentRequirement = (BYTE)pfwalk->m_placement.m_alignment;
909917
}
910918
}
911919

912-
pNativeLayoutInfo->m_alignmentRequirement = max(max<BYTE>(1, parentAlignmentRequirement), fieldAlignmentRequirement);
920+
// Allow the packing size to require less alignment than the parent's alignment requirement.
921+
BYTE initialAlignmentRequirement = min(packingSize, parentAlignmentRequirement);
922+
923+
// The alignment of the native layout is the stricter of the initial alignment requirement or the alignment requirements of the fields.
924+
pNativeLayoutInfo->m_alignmentRequirement = max(initialAlignmentRequirement, fieldAlignmentRequirement);
913925

914926
BOOL fExplicitOffsets = pMT->GetClass()->HasExplicitFieldOffsetLayout();
915927

@@ -920,11 +932,6 @@ EEClassNativeLayoutInfo* EEClassNativeLayoutInfo::CollectNativeLayoutFieldMetada
920932
}
921933
else
922934
{
923-
BYTE packingSize = pMT->GetLayoutInfo()->GetPackingSize();
924-
if (packingSize == 0)
925-
{
926-
packingSize = DEFAULT_PACKING_SIZE;
927-
}
928935
lastFieldEnd = CalculateOffsetsForSequentialLayout(pInfoArray, cInstanceFields, cbAdjustedParentLayoutNativeSize, packingSize);
929936
}
930937

‎src/coreclr/vm/methodtablebuilder.cpp‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8219,16 +8219,12 @@ VOID MethodTableBuilder::PlaceInstanceFields(MethodTable** pByValueClassCache)
82198219

82208220
if (bmtLayout->layoutType == EEClassLayoutInfo::LayoutType::Sequential)
82218221
{
8222-
if (hasNonTrivialParent && !pParentMT->IsManagedSequential())
8222+
// If the parent type is not Object, ValueType, or Sequential, or if this type has GC fields,
8223+
// we will use Auto layout instead of Sequential layout and set the packing size.
8224+
if ((hasNonTrivialParent && !pParentMT->IsManagedSequential()) || hasGCFields)
82238225
{
8224-
// If the parent type is not Object, ValueType or Sequential, then we need to use Auto layout.
8225-
bmtLayout->layoutType = EEClassLayoutInfo::LayoutType::Auto;
8226-
}
8227-
8228-
if (hasGCFields)
8229-
{
8230-
// If this type has GC fields, we will use Auto layout instead of Sequential layout.
82318226
bmtLayout->layoutType = EEClassLayoutInfo::LayoutType::Auto;
8227+
pLayoutInfo->SetPackingSize(bmtLayout->packingSize);
82328228
}
82338229
}
82348230

‎src/tests/Interop/StructPacking/StructPacking.cs‎

Lines changed: 88 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ struct DefaultLayoutDefaultPacking<T> : ITestStructure
3030
public T _value;
3131

3232
public int Size => Unsafe.SizeOf<DefaultLayoutDefaultPacking<T>>();
33-
public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
34-
public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
33+
public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
34+
public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
3535
}
3636

3737
[StructLayout(LayoutKind.Sequential)]
@@ -41,8 +41,8 @@ struct SequentialLayoutDefaultPacking<T> : ITestStructure
4141
public T _value;
4242

4343
public int Size => Unsafe.SizeOf<SequentialLayoutDefaultPacking<T>>();
44-
public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
45-
public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
44+
public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
45+
public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
4646
}
4747

4848
[StructLayout(LayoutKind.Sequential, Pack = 1)]
@@ -52,8 +52,8 @@ struct SequentialLayoutMinPacking<T> : ITestStructure
5252
public T _value;
5353

5454
public int Size => Unsafe.SizeOf<SequentialLayoutMinPacking<T>>();
55-
public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
56-
public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
55+
public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
56+
public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
5757
}
5858

5959
[StructLayout(LayoutKind.Sequential, Pack = 128)]
@@ -63,8 +63,8 @@ struct SequentialLayoutMaxPacking<T> : ITestStructure
6363
public T _value;
6464

6565
public int Size => Unsafe.SizeOf<SequentialLayoutMaxPacking<T>>();
66-
public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
67-
public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
66+
public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
67+
public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
6868
}
6969

7070
[StructLayout(LayoutKind.Auto)]
@@ -74,8 +74,8 @@ struct AutoLayoutDefaultPacking<T> : ITestStructure
7474
public T _value;
7575

7676
public int Size => Unsafe.SizeOf<AutoLayoutDefaultPacking<T>>();
77-
public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
78-
public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
77+
public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
78+
public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
7979
}
8080

8181
[StructLayout(LayoutKind.Auto, Pack = 1)]
@@ -85,8 +85,8 @@ struct AutoLayoutMinPacking<T> : ITestStructure
8585
public T _value;
8686

8787
public int Size => Unsafe.SizeOf<AutoLayoutMinPacking<T>>();
88-
public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
89-
public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
88+
public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
89+
public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
9090
}
9191

9292
[StructLayout(LayoutKind.Auto, Pack = 128)]
@@ -96,11 +96,22 @@ struct AutoLayoutMaxPacking<T> : ITestStructure
9696
public T _value;
9797

9898
public int Size => Unsafe.SizeOf<AutoLayoutMaxPacking<T>>();
99-
public int OffsetOfByte => Program.OffsetOf(ref this, ref _byte);
100-
public int OffsetOfValue => Program.OffsetOf(ref this, ref _value);
99+
public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
100+
public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
101101
}
102102

103-
public unsafe partial class Program
103+
[StructLayout(LayoutKind.Sequential, Pack = 1)]
104+
struct ManagedAutoUnmanagedSequentialLayoutMinPacking : ITestStructure
105+
{
106+
public Action _value;
107+
public byte _byte;
108+
109+
public int Size => Unsafe.SizeOf<ManagedAutoUnmanagedSequentialLayoutMinPacking>();
110+
public int OffsetOfByte => StructPacking.OffsetOf(ref this, ref _byte);
111+
public int OffsetOfValue => StructPacking.OffsetOf(ref this, ref _value);
112+
}
113+
114+
public unsafe partial class StructPacking
104115
{
105116
const int Pass = 100;
106117
const int Fail = 0;
@@ -136,6 +147,9 @@ public static int TestEntryPoint()
136147
succeeded &= TestMyVector128();
137148
succeeded &= TestMyVector256();
138149

150+
// Test data types that invalidate managed sequential layout
151+
succeeded &= TestAction();
152+
139153
return succeeded ? Pass : Fail;
140154
}
141155

@@ -1629,6 +1643,45 @@ static bool TestMyVector256()
16291643

16301644
return succeeded;
16311645
}
1646+
static bool TestAction()
1647+
{
1648+
bool succeeded = true;
1649+
1650+
if (Environment.Is64BitProcess)
1651+
{
1652+
succeeded &= Test<ManagedAutoUnmanagedSequentialLayoutMinPacking>(
1653+
expectedSize: 16,
1654+
expectedOffsetByte: 8,
1655+
expectedOffsetValue: 0,
1656+
expectedNativeSize: 9
1657+
);
1658+
1659+
succeeded &= Test<ManagedAutoUnmanagedSequentialLayoutMinPacking>(
1660+
expectedSize: 16,
1661+
expectedOffsetByte: 8,
1662+
expectedOffsetValue: 0,
1663+
expectedNativeSize: 9
1664+
);
1665+
}
1666+
else
1667+
{
1668+
succeeded &= Test<ManagedAutoUnmanagedSequentialLayoutMinPacking>(
1669+
expectedSize: 8,
1670+
expectedOffsetByte: 4,
1671+
expectedOffsetValue: 0,
1672+
expectedNativeSize: 5
1673+
);
1674+
1675+
succeeded &= Test<ManagedAutoUnmanagedSequentialLayoutMinPacking>(
1676+
expectedSize: 8,
1677+
expectedOffsetByte: 4,
1678+
expectedOffsetValue: 0,
1679+
expectedNativeSize: 5
1680+
);
1681+
}
1682+
1683+
return succeeded;
1684+
}
16321685

16331686
static bool Test<T>(int expectedSize, int expectedOffsetByte, int expectedOffsetValue) where T : ITestStructure
16341687
{
@@ -1667,6 +1720,26 @@ static bool Test<T>(int expectedSize, int expectedOffsetByte, int expectedOffset
16671720
return succeeded;
16681721
}
16691722

1723+
static bool Test<T>(int expectedSize, int expectedOffsetByte, int expectedOffsetValue, int expectedNativeSize) where T : ITestStructure
1724+
{
1725+
bool succeeded = Test<T>(expectedSize, expectedOffsetByte, expectedOffsetValue);
1726+
1727+
int nativeSize = Marshal.SizeOf<T>();
1728+
if (nativeSize != expectedNativeSize)
1729+
{
1730+
Console.WriteLine($"Unexpected Native Size for {typeof(T)}.");
1731+
Console.WriteLine($" Expected: {expectedNativeSize}; Actual: {nativeSize}");
1732+
succeeded = false;
1733+
}
1734+
1735+
if (!succeeded)
1736+
{
1737+
Console.WriteLine();
1738+
}
1739+
1740+
return succeeded;
1741+
}
1742+
16701743
internal static int OffsetOf<T, U>(ref T origin, ref U target)
16711744
{
16721745
return Unsafe.ByteOffset(ref origin, ref Unsafe.As<U, T>(ref target)).ToInt32();

0 commit comments

Comments
 (0)