From 9024f6c7c304c106be3e49558d4ad61b72e36b3c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 8 Aug 2022 17:55:49 -0700 Subject: [PATCH 1/9] Add ABI tests for Int128 covering interesting scenarios --- .../Interop/PInvoke/Int128/Int128Native.cpp | 190 ++++++++++++++++++ .../Interop/PInvoke/Int128/Int128Test.cs | 84 ++++++++ 2 files changed, 274 insertions(+) diff --git a/src/tests/Interop/PInvoke/Int128/Int128Native.cpp b/src/tests/Interop/PInvoke/Int128/Int128Native.cpp index 28f70bca06fabd..0ce4a0923aa3fc 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Native.cpp +++ b/src/tests/Interop/PInvoke/Int128/Int128Native.cpp @@ -19,6 +19,12 @@ static Int128 Int128Value = { }; +struct StructWithInt128 +{ + int64_t messUpPadding; + Int128 value; +}; + extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE GetInt128(uint64_t upper, uint64_t lower) { Int128 result; @@ -62,6 +68,190 @@ extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128(Int128 lhs, Int128 rhs) return result; } +extern "C" DLL_EXPORT StructWithInt128 STDMETHODCALLTYPE AddStructWithInt128(StructWithInt128 lhs, StructWithInt128 rhs) +{ + StructWithInt128 result = {}; + result.messUpPadding = lhs.messUpPadding; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result.value = lhs.value + rhs.value; +#else + result.value.lower = lhs.value.lower + rhs.value.lower; + uint64_t carry = (result.value.lower < lhs.value.lower) ? 1 : 0; + result.value.upper = lhs.value.upper + rhs.value.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT StructWithInt128 STDMETHODCALLTYPE AddStructWithInt128_1(int64_t dummy1, StructWithInt128 lhs, StructWithInt128 rhs) +{ + StructWithInt128 result = {}; + result.messUpPadding = lhs.messUpPadding; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result.value = lhs.value + rhs.value; +#else + result.value.lower = lhs.value.lower + rhs.value.lower; + uint64_t carry = (result.value.lower < lhs.value.lower) ? 1 : 0; + result.value.upper = lhs.value.upper + rhs.value.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT StructWithInt128 STDMETHODCALLTYPE AddStructWithInt128_9(int64_t dummy1, int64_t dummy2, int64_t dummy3, int64_t dummy4, int64_t dummy5, int64_t dummy6, int64_t dummy7, int64_t dummy8, int64_t dummy9, StructWithInt128 lhs, StructWithInt128 rhs) +{ + StructWithInt128 result = {}; + result.messUpPadding = lhs.messUpPadding; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result.value = lhs.value + rhs.value; +#else + result.value.lower = lhs.value.lower + rhs.value.lower; + uint64_t carry = (result.value.lower < lhs.value.lower) ? 1 : 0; + result.value.upper = lhs.value.upper + rhs.value.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_1(int64_t dummy1, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_2(int64_t dummy1, int64_t dummy2, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_3(int64_t dummy1, int64_t dummy2, int64_t dummy3, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_4(int64_t dummy1, int64_t dummy2, int64_t dummy3, int64_t dummy4, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_5(int64_t dummy1, int64_t dummy2, int64_t dummy3, int64_t dummy4, int64_t dummy5, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_6(int64_t dummy1, int64_t dummy2, int64_t dummy3, int64_t dummy4, int64_t dummy5, int64_t dummy6, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_7(int64_t dummy1, int64_t dummy2, int64_t dummy3, int64_t dummy4, int64_t dummy5, int64_t dummy6, int64_t dummy7, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_8(int64_t dummy1, int64_t dummy2, int64_t dummy3, int64_t dummy4, int64_t dummy5, int64_t dummy6, int64_t dummy7, int64_t dummy8, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + +extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128_9(int64_t dummy1, int64_t dummy2, int64_t dummy3, int64_t dummy4, int64_t dummy5, int64_t dummy6, int64_t dummy7, int64_t dummy8, int64_t dummy9, Int128 lhs, Int128 rhs) +{ + Int128 result; + +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + result = lhs + rhs; +#else + result.lower = lhs.lower + rhs.lower; + uint64_t carry = (result.lower < lhs.lower) ? 1 : 0; + result.upper = lhs.upper + rhs.upper + carry; +#endif + + return result; +} + + extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128s(const Int128* pValues, uint32_t count) { Int128 result = {}; diff --git a/src/tests/Interop/PInvoke/Int128/Int128Test.cs b/src/tests/Interop/PInvoke/Int128/Int128Test.cs index 5a9ddd5bb2135d..95de72ba36d53f 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Test.cs +++ b/src/tests/Interop/PInvoke/Int128/Int128Test.cs @@ -5,6 +5,13 @@ using System.Runtime.InteropServices; using Xunit; +struct StructWithInt128 +{ + public StructWithInt128(Int128 val) { value = val; messUpPadding = 0x101010; } + public long messUpPadding; + public Int128 value; +}; + unsafe partial class Int128Native { [DllImport(nameof(Int128Native))] @@ -25,6 +32,43 @@ unsafe partial class Int128Native [DllImport(nameof(Int128Native))] public static extern Int128 AddInt128(Int128 lhs, Int128 rhs); + [DllImport(nameof(Int128Native))] + public static extern StructWithInt128 AddStructWithInt128(StructWithInt128 lhs, StructWithInt128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern StructWithInt128 AddStructWithInt128_1(long dummy1, StructWithInt128 lhs, StructWithInt128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern StructWithInt128 AddStructWithInt128_9(long dummy1, long dummy2, long dummy3, long dummy4, long dummy5, long dummy6, long dummy7, long dummy8, long dummy9, StructWithInt128 lhs, StructWithInt128 rhs); + + // Test alignment and proper register usage for Int128 with various amounts of other registers in use. These tests are designed to stress the calling convention of Arm64 and Unix X64. + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_1(long dummy1, Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_2(long dummy1, long dummy2, Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_3(long dummy1, long dummy2, long dummy3, Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_4(long dummy1, long dummy2, long dummy3, long dummy4, Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_5(long dummy1, long dummy2, long dummy3, long dummy4, long dummy5, Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_6(long dummy1, long dummy2, long dummy3, long dummy4, long dummy5, long dummy6, Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_7(long dummy1, long dummy2, long dummy3, long dummy4, long dummy5, long dummy6, long dummy7, Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_8(long dummy1, long dummy2, long dummy3, long dummy4, long dummy5, long dummy6, long dummy7, long dummy8, Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern Int128 AddInt128_9(long dummy1, long dummy2, long dummy3, long dummy4, long dummy5, long dummy6, long dummy7, long dummy8, long dummy9, Int128 lhs, Int128 rhs); + [DllImport(nameof(Int128Native))] public static extern Int128 AddInt128s(Int128* pValues, int count); @@ -77,5 +121,45 @@ private static void TestInt128() Int128 value9 = Int128Native.AddInt128s(in values[0], values.Length); Assert.Equal(new Int128(95, 100), value9); + + // Test ABI alignment issues on Arm64 and Unix X64, both enregistered and while spilled to the stack + Int128 value10 = Int128Native.AddInt128_1(1, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value10); + + Int128 value11 = Int128Native.AddInt128_2(1, 2, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value11); + + Int128 value12 = Int128Native.AddInt128_3(1, 2, 3, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value12); + + Int128 value13 = Int128Native.AddInt128_4(1, 2, 3, 4, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value13); + + Int128 value14 = Int128Native.AddInt128_5(1, 2, 3, 4, 5, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value14); + + Int128 value15 = Int128Native.AddInt128_6(1, 2, 3, 4, 5, 6, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value15); + + Int128 value16 = Int128Native.AddInt128_7(1, 2, 3, 4, 5, 6, 7, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value16); + + Int128 value17 = Int128Native.AddInt128_8(1, 2, 3, 4, 5, 6, 7, 8, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value17); + + Int128 value18 = Int128Native.AddInt128_9(1, 2, 3, 4, 5, 6, 7, 8, 9, new Int128(25, 26), new Int128(27, 28)); + Assert.Equal(new Int128(52, 54), value18); + + // Test alignment within a structure + StructWithInt128 value19 = Int128Native.AddStructWithInt128(new StructWithInt128(new Int128(29, 30)), new StructWithInt128(new Int128(31, 32))); + Assert.Equal(new StructWithInt128(new Int128(60, 62)), value19); + + // Test abi register alignment within a structure + StructWithInt128 value20 = Int128Native.AddStructWithInt128_1(1, new StructWithInt128(new Int128(29, 30)), new StructWithInt128(new Int128(31, 32))); + Assert.Equal(new StructWithInt128(new Int128(60, 62)), value20); + + // Test abi alignment when spilled to the stack + StructWithInt128 value21 = Int128Native.AddStructWithInt128_9(1, 2, 3, 4, 5, 6, 7, 8, 9, new StructWithInt128(new Int128(29, 30)), new StructWithInt128(new Int128(31, 32))); + Assert.Equal(new StructWithInt128(new Int128(60, 62)), value21); } } From 74935a10e343cc02e0b869615919d265bbe884a5 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 10 Aug 2022 14:03:02 -0700 Subject: [PATCH 2/9] New test --- .../fieldlayout/InstanceFieldLayout.cs | 414 ++++++++++++++++++ .../readytorun/fieldlayout/fieldlayout.csproj | 16 + .../fieldlayout/fieldlayouttests.cs | 10 + 3 files changed, 440 insertions(+) create mode 100644 src/tests/readytorun/fieldlayout/InstanceFieldLayout.cs create mode 100644 src/tests/readytorun/fieldlayout/fieldlayout.csproj create mode 100644 src/tests/readytorun/fieldlayout/fieldlayouttests.cs diff --git a/src/tests/readytorun/fieldlayout/InstanceFieldLayout.cs b/src/tests/readytorun/fieldlayout/InstanceFieldLayout.cs new file mode 100644 index 00000000000000..9c8416224e0a5a --- /dev/null +++ b/src/tests/readytorun/fieldlayout/InstanceFieldLayout.cs @@ -0,0 +1,414 @@ +// 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.Runtime.InteropServices; +using System.Runtime.Intrinsics; + +#pragma warning disable 169 + +namespace ContainsGCPointers +{ + struct NoPointers + { + public int int1; + public byte byte1; + public char char1; + } + + struct StillNoPointers + { + public NoPointers noPointers1; + public bool bool1; + } + + class ClassNoPointers + { + public char char1; + } + + struct HasPointers + { + public string string1; + } + + struct FieldHasPointers + { + public HasPointers hasPointers1; + } + + class ClassHasPointers + { + public ClassHasPointers classHasPointers1; + } + + class BaseClassHasPointers : ClassHasPointers + { + } + + public class ClassHasIntArray + { + public int[] intArrayField; + } + + public class ClassHasArrayOfClassType + { + public ClassNoPointers[] classTypeArray; + } +} + +namespace Explicit +{ + [StructLayout(LayoutKind.Explicit)] + class Class1 + { + public static int Stat; + [FieldOffset(4)] + public bool Bar; + [FieldOffset(10)] + public char Baz; + } + + [StructLayout(LayoutKind.Explicit)] + class Class2 : Class1 + { + [FieldOffset(0)] + public int Lol; + [FieldOffset(20)] + public byte Omg; + } + + [StructLayout(LayoutKind.Explicit, Size = 40)] + class ExplicitSize + { + [FieldOffset(0)] + public int Lol; + [FieldOffset(20)] + public byte Omg; + } + + [StructLayout(LayoutKind.Explicit)] + public class ExplicitEmptyClass + { + } + + [StructLayout(LayoutKind.Explicit, Size = 0)] + public class ExplicitEmptyClassSize0 + { + } + + [StructLayout(LayoutKind.Explicit)] + public struct ExplicitEmptyStruct + { + } + + [StructLayout(LayoutKind.Explicit)] + ref struct MisalignedPointer + { + [FieldOffset(2)] + public object O; + } + + [StructLayout(LayoutKind.Explicit)] + ref struct MisalignedByRef + { + [FieldOffset(2)] + public ByRefStruct O; + } + + ref struct ByRefStruct + { + } +} + +namespace Sequential +{ + [StructLayout(LayoutKind.Sequential)] + class Class1 + { + public int MyInt; + public bool MyBool; + public char MyChar; + public string MyString; + public byte[] MyByteArray; + public Class1 MyClass1SelfRef; + } + + [StructLayout(LayoutKind.Sequential)] + class Class2 : Class1 + { + public int MyInt2; + } + + // [StructLayout(LayoutKind.Sequential)] is applied by default by the C# compiler + struct Struct0 + { + public bool b1; + public bool b2; + public bool b3; + public int i1; + public string s1; + } + + // [StructLayout(LayoutKind.Sequential)] is applied by default by the C# compiler + struct Struct1 + { + public Struct0 MyStruct0; + public bool MyBool; + } + + [StructLayout(LayoutKind.Sequential)] + public class ClassDoubleBool + { + public double double1; + public bool bool1; + } + + [StructLayout(LayoutKind.Sequential)] + public class ClassBoolDoubleBool + { + public bool bool1; + public double double1; + public bool bool2; + } +} + +namespace Auto +{ + [StructLayout(LayoutKind.Auto)] + struct StructWithBool + { + public bool MyStructBool; + } + + [StructLayout(LayoutKind.Auto)] + struct StructWithIntChar + { + public char MyStructChar; + public int MyStructInt; + } + + [StructLayout(LayoutKind.Auto)] + struct StructWithChar + { + public char MyStructChar; + } + + class ClassContainingStructs + { + public static int MyStaticInt; + + public StructWithBool MyStructWithBool; + public bool MyBool1; + public char MyChar1; + public int MyInt; + public double MyDouble; + public long MyLong; + public byte[] MyByteArray; + public string MyString1; + public bool MyBool2; + public StructWithIntChar MyStructWithIntChar; + public StructWithChar MyStructWithChar; + } + + class BaseClass7BytesRemaining + { + public bool MyBool1; + public double MyDouble1; + public long MyLong1; + public byte[] MyByteArray1; + public string MyString1; + } + + class BaseClass4BytesRemaining + { + public long MyLong1; + public uint MyUint1; + } + + class BaseClass3BytesRemaining + { + public int MyInt1; + public string MyString1; + public bool MyBool1; + } + + class OptimizePartial : BaseClass7BytesRemaining + { + public bool OptBool; + public char OptChar; + public long NoOptLong; + public string NoOptString; + } + + class Optimize7Bools : BaseClass7BytesRemaining + { + public bool OptBool1; + public bool OptBool2; + public bool OptBool3; + public bool OptBool4; + public bool OptBool5; + public bool OptBool6; + public bool OptBool7; + public bool NoOptBool8; + public string NoOptString; + } + + class OptimizeAlignedFields : BaseClass7BytesRemaining + { + public bool OptBool1; + public bool OptBool2; + public bool OptBool3; + public bool NoOptBool4; + public char OptChar1; + public char OptChar2; + public string NoOptString; + } + + class OptimizeLargestField : BaseClass4BytesRemaining + { + public bool NoOptBool; + public char NoOptChar; + public int OptInt; + public string NoOptString; + } + + class NoOptimizeMisaligned : BaseClass3BytesRemaining + { + public char NoOptChar; + public int NoOptInt; + public string NoOptString; + } + + class NoOptimizeCharAtSize2Alignment : BaseClass3BytesRemaining + { + public char NoOptChar; + } + + [StructLayout(LayoutKind.Auto, Pack = 1)] + struct MinPacking + { + public byte _byte; + public T _value; + } + + [StructLayout(LayoutKind.Auto)] + struct int8x16x2 + { + public Vector128 _0; + public Vector128 _1; + } + + struct Wrapper_int8x16x2 + { + public int8x16x2 fld; + } +} + +namespace IsByRefLike +{ + public ref struct ByRefLikeStruct + { + public ref object ByRef; + } + + public struct NotByRefLike + { + public int X; + } +} + +namespace EnumAlignment +{ + public enum ByteEnum : byte {} + public enum ShortEnum : short {} + public enum IntEnum : int {} + public enum LongEnum : long {} + + public struct LongIntEnumStruct + { + public LongEnum _1; + public IntEnum _2; + public LongEnum _3; + public IntEnum _4; + } + + public struct LongIntEnumStructFieldStruct + { + public byte _0; + public LongIntEnumStruct _struct; + } + + public struct IntShortEnumStruct + { + public IntEnum _1; + public ShortEnum _2; + public IntEnum _3; + public ShortEnum _4; + } + + public struct IntShortEnumStructFieldStruct + { + public byte _0; + public IntShortEnumStruct _struct; + } + + public struct ShortByteEnumStruct + { + public ShortEnum _1; + public ByteEnum _2; + public ShortEnum _3; + public ByteEnum _4; + } + + public struct ShortByteEnumStructFieldStruct + { + public byte _0; + public ShortByteEnumStruct _struct; + } + + [StructLayout(LayoutKind.Auto)] + public struct LongIntEnumStructAuto + { + public LongEnum _1; + public IntEnum _2; + public LongEnum _3; + public IntEnum _4; + } + + public struct LongIntEnumStructAutoFieldStruct + { + public byte _0; + public LongIntEnumStructAuto _struct; + } + + [StructLayout(LayoutKind.Auto)] + public struct IntShortEnumStructAuto + { + public IntEnum _1; + public ShortEnum _2; + public IntEnum _3; + public ShortEnum _4; + } + + public struct IntShortEnumStructAutoFieldStruct + { + public byte _0; + public IntShortEnumStructAuto _struct; + } + + [StructLayout(LayoutKind.Auto)] + public struct ShortByteEnumStructAuto + { + public ShortEnum _1; + public ByteEnum _2; + public ShortEnum _3; + public ByteEnum _4; + } + + public struct ShortByteEnumStructAutoFieldStruct + { + public byte _0; + public ShortByteEnumStructAuto _struct; + } +} diff --git a/src/tests/readytorun/fieldlayout/fieldlayout.csproj b/src/tests/readytorun/fieldlayout/fieldlayout.csproj new file mode 100644 index 00000000000000..969e9b6b493b26 --- /dev/null +++ b/src/tests/readytorun/fieldlayout/fieldlayout.csproj @@ -0,0 +1,16 @@ + + + + exe + BuildAndRun + true + true + true + + + + + + + + \ No newline at end of file diff --git a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs new file mode 100644 index 00000000000000..a78ce61424dffc --- /dev/null +++ b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs @@ -0,0 +1,10 @@ +using System; + +class FieldLayoutOffsetsTest +{ + static ContainsGCPointers.NoPointers _fld1; + static ContainsGCPointers.StillNoPointers _fld2; + static ContainsGCPointers.ClassNoPointers _fld3 = new ContainsGCPointers.ClassNoPointers(); + static ContainsGCPointers.HasPointers _fld4; + static ContainsGCPointers.FieldHasPointers _fld5; +} From be730bcfb9296ec2760ae691ae55811780503f2d Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 10 Aug 2022 17:29:09 -0700 Subject: [PATCH 3/9] Fix auto layout algorithm to compute structure alignment correctly - In particular: 1. The alignment requirement imposed by of a non-primitive, non-enum valuetype field is the alignment of that field 2. The alignment requirement imposed by a primitive is the pointer size of the target platform, unless running on Arm32, in which case if the primitive or enum is 8 bytes in size, the alignment requirement is 8. - The previous implementation produced an alignment of pointer size, unless running on Arm32 and one of the fields had an alignment requirement of 8 (in which case the alignment requirement computed for the structure would be 8) In addition, add a test which verifies that the instance field layout test types are actually producing R2R compatible results at runtime. - This test shows that we have some issues around explicit layout, so I was forced to disable that portion of the test for now. Fixes #65281 --- .../Common/MetadataFieldLayoutAlgorithm.cs | 36 ++- .../CoreTestAssembly/InstanceFieldLayout.cs | 215 +++++++------- .../CoreTestAssembly/Platform.cs | 20 ++ .../ILCompiler.TypeSystem.Tests.csproj | 1 + .../InstanceFieldLayoutTests.cs | 20 ++ .../TestTypeSystemContext.cs | 7 + .../readytorun/fieldlayout/fieldlayout.csproj | 6 +- .../fieldlayout/fieldlayouttests.cs | 273 +++++++++++++++++- 8 files changed, 466 insertions(+), 112 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 8da028a5190fb5..975e5a9d0fcd7a 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -520,25 +520,41 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, if (!fieldLayoutAbiStable) layoutAbiStable = false; - largestAlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequired); - if (IsByValueClass(fieldType)) { + largestAlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequired); instanceValueClassFieldsArr[instanceValueClassFieldCount++] = field; } - else if (fieldType.IsGCPointer) - { - instanceGCPointerFieldsArr[instanceGCPointerFieldsCount++] = field; - } else { - int log2size = CalculateLog2(fieldSizeAndAlignment.Size.AsInt); - instanceNonGCPointerFieldsArr[log2size][instanceNonGCPointerFieldsCount[log2size]++] = field; + // non-value-type fields always require pointer alignment + // This does not account for types that are marked IsAlign8Candidate due to 8-byte fields + // but that is explicitly handled when we calculate the final alignment for the type. + + // This behavior is extremely strange for primitive types, as it makes a struct with a single byte in it + // have 8 byte alignment, but that is the current implementation. + + largestAlignmentRequired = LayoutInt.Max(new LayoutInt(context.Target.PointerSize), largestAlignmentRequired); + + if (fieldType.IsGCPointer) + { + instanceGCPointerFieldsArr[instanceGCPointerFieldsCount++] = field; + } + else + { + int log2size = CalculateLog2(fieldSizeAndAlignment.Size.AsInt); + instanceNonGCPointerFieldsArr[log2size][instanceNonGCPointerFieldsCount[log2size]++] = field; + + if (fieldType.IsPrimitive || fieldType.IsEnum) + { + // Handle alignment of long/ulong/double on ARM32 + largestAlignmentRequired = LayoutInt.Max(context.Target.GetObjectAlignment(fieldSizeAndAlignment.Size), largestAlignmentRequired); + } + } } } - largestAlignmentRequired = context.Target.GetObjectAlignment(largestAlignmentRequired); - bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && largestAlignmentRequired.AsInt > 4; + bool requiresAlign8 = !largestAlignmentRequired.IsIndeterminate && context.Target.PointerSize == 4 && context.Target.GetObjectAlignment(largestAlignmentRequired).AsInt > 4 && context.Target.PointerSize == 4; // For types inheriting from another type, field offsets continue on from where they left off // Base alignment is not always required, it's only applied when there's a version bubble boundary diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs index b49dd4ff729309..cf9fed79b5b60a 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs @@ -3,6 +3,7 @@ using System; using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; #pragma warning disable 169 @@ -10,35 +11,35 @@ namespace ContainsGCPointers { struct NoPointers { - int int1; - byte byte1; - char char1; + public int int1; + public byte byte1; + public char char1; } struct StillNoPointers { - NoPointers noPointers1; - bool bool1; + public NoPointers noPointers1; + public bool bool1; } class ClassNoPointers { - char char1; + public char char1; } struct HasPointers { - string string1; + public string string1; } struct FieldHasPointers { - HasPointers hasPointers1; + public HasPointers hasPointers1; } class ClassHasPointers { - ClassHasPointers classHasPointers1; + public ClassHasPointers classHasPointers1; } class BaseClassHasPointers : ClassHasPointers @@ -47,12 +48,12 @@ class BaseClassHasPointers : ClassHasPointers public class ClassHasIntArray { - int[] intArrayField; + public int[] intArrayField; } - public class ClassHasArrayOfClassType + class ClassHasArrayOfClassType { - ClassNoPointers[] classTypeArray; + public ClassNoPointers[] classTypeArray; } } @@ -61,29 +62,29 @@ namespace Explicit [StructLayout(LayoutKind.Explicit)] class Class1 { - static int Stat; + public static int Stat; [FieldOffset(4)] - bool Bar; + public bool Bar; [FieldOffset(10)] - char Baz; + public char Baz; } [StructLayout(LayoutKind.Explicit)] class Class2 : Class1 { [FieldOffset(0)] - int Lol; + public int Lol; [FieldOffset(20)] - byte Omg; + public byte Omg; } [StructLayout(LayoutKind.Explicit, Size = 40)] class ExplicitSize { [FieldOffset(0)] - int Lol; + public int Lol; [FieldOffset(20)] - byte Omg; + public byte Omg; } [StructLayout(LayoutKind.Explicit)] @@ -125,50 +126,50 @@ namespace Sequential [StructLayout(LayoutKind.Sequential)] class Class1 { - int MyInt; - bool MyBool; - char MyChar; - string MyString; - byte[] MyByteArray; - Class1 MyClass1SelfRef; + public int MyInt; + public bool MyBool; + public char MyChar; + public string MyString; + public byte[] MyByteArray; + public Class1 MyClass1SelfRef; } [StructLayout(LayoutKind.Sequential)] class Class2 : Class1 { - int MyInt2; + public int MyInt2; } // [StructLayout(LayoutKind.Sequential)] is applied by default by the C# compiler struct Struct0 { - bool b1; - bool b2; - bool b3; - int i1; - string s1; + public bool b1; + public bool b2; + public bool b3; + public int i1; + public string s1; } // [StructLayout(LayoutKind.Sequential)] is applied by default by the C# compiler struct Struct1 { - Struct0 MyStruct0; - bool MyBool; + public Struct0 MyStruct0; + public bool MyBool; } [StructLayout(LayoutKind.Sequential)] public class ClassDoubleBool { - double double1; - bool bool1; + public double double1; + public bool bool1; } [StructLayout(LayoutKind.Sequential)] public class ClassBoolDoubleBool { - bool bool1; - double double1; - bool bool2; + public bool bool1; + public double double1; + public bool bool2; } } @@ -177,111 +178,111 @@ namespace Auto [StructLayout(LayoutKind.Auto)] struct StructWithBool { - bool MyStructBool; + public bool MyStructBool; } [StructLayout(LayoutKind.Auto)] struct StructWithIntChar { - char MyStructChar; - int MyStructInt; + public char MyStructChar; + public int MyStructInt; } [StructLayout(LayoutKind.Auto)] struct StructWithChar { - char MyStructChar; + public char MyStructChar; } class ClassContainingStructs { - static int MyStaticInt; + public static int MyStaticInt; - StructWithBool MyStructWithBool; - bool MyBool1; - char MyChar1; - int MyInt; - double MyDouble; - long MyLong; - byte[] MyByteArray; - string MyString1; - bool MyBool2; - StructWithIntChar MyStructWithIntChar; - StructWithChar MyStructWithChar; + public StructWithBool MyStructWithBool; + public bool MyBool1; + public char MyChar1; + public int MyInt; + public double MyDouble; + public long MyLong; + public byte[] MyByteArray; + public string MyString1; + public bool MyBool2; + public StructWithIntChar MyStructWithIntChar; + public StructWithChar MyStructWithChar; } class BaseClass7BytesRemaining { - bool MyBool1; - double MyDouble1; - long MyLong1; - byte[] MyByteArray1; - string MyString1; + public bool MyBool1; + public double MyDouble1; + public long MyLong1; + public byte[] MyByteArray1; + public string MyString1; } class BaseClass4BytesRemaining { - long MyLong1; - uint MyUint1; + public long MyLong1; + public uint MyUint1; } class BaseClass3BytesRemaining { - int MyInt1; - string MyString1; - bool MyBool1; + public int MyInt1; + public string MyString1; + public bool MyBool1; } class OptimizePartial : BaseClass7BytesRemaining { - bool OptBool; - char OptChar; - long NoOptLong; - string NoOptString; + public bool OptBool; + public char OptChar; + public long NoOptLong; + public string NoOptString; } class Optimize7Bools : BaseClass7BytesRemaining { - bool OptBool1; - bool OptBool2; - bool OptBool3; - bool OptBool4; - bool OptBool5; - bool OptBool6; - bool OptBool7; - bool NoOptBool8; - string NoOptString; + public bool OptBool1; + public bool OptBool2; + public bool OptBool3; + public bool OptBool4; + public bool OptBool5; + public bool OptBool6; + public bool OptBool7; + public bool NoOptBool8; + public string NoOptString; } class OptimizeAlignedFields : BaseClass7BytesRemaining { - bool OptBool1; - bool OptBool2; - bool OptBool3; - bool NoOptBool4; - char OptChar1; - char OptChar2; - string NoOptString; + public bool OptBool1; + public bool OptBool2; + public bool OptBool3; + public bool NoOptBool4; + public char OptChar1; + public char OptChar2; + public string NoOptString; } class OptimizeLargestField : BaseClass4BytesRemaining { - bool NoOptBool; - char NoOptChar; - int OptInt; - string NoOptString; + public bool NoOptBool; + public char NoOptChar; + public int OptInt; + public string NoOptString; } class NoOptimizeMisaligned : BaseClass3BytesRemaining { - char NoOptChar; - int NoOptInt; - string NoOptString; + public char NoOptChar; + public int NoOptInt; + public string NoOptString; } class NoOptimizeCharAtSize2Alignment : BaseClass3BytesRemaining { - char NoOptChar; + public char NoOptChar; } [StructLayout(LayoutKind.Auto, Pack = 1)] @@ -290,27 +291,45 @@ struct MinPacking public byte _byte; public T _value; } + + [StructLayout(LayoutKind.Auto)] + struct int8x16x2 + { + public Vector128 _0; + public Vector128 _1; + } + + struct Wrapper_int8x16x2 + { + public int8x16x2 fld; + } + + struct Wrapper_int8x16x2_2 + { + public bool fld1; + public int8x16x2 fld2; + } } namespace IsByRefLike { public ref struct ByRefLikeStruct { - ref object ByRef; + public ref object ByRef; } public struct NotByRefLike { - int X; + public int X; } } namespace EnumAlignment { - public enum ByteEnum : byte {} - public enum ShortEnum : short {} - public enum IntEnum : int {} - public enum LongEnum : long {} + public enum ByteEnum : byte { Val } + public enum ShortEnum : short { Val } + public enum IntEnum : int { Val } + public enum LongEnum : long { Val } public struct LongIntEnumStruct { diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs index 886f7aa791fd69..f10a3bd9d18d90 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs @@ -1,5 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; #pragma warning disable 649 #pragma warning disable 169 @@ -135,4 +137,22 @@ public static class RuntimeFeature { public const string VirtualStaticsInInterfaces = nameof(VirtualStaticsInInterfaces); } + + internal sealed class IntrinsicAttribute : Attribute + { + } +} + +namespace System.Runtime.Intrinsics +{ + [Intrinsic] + [StructLayout(LayoutKind.Sequential, Size = 16)] + public readonly struct Vector128 + where T : struct + { + // These fields exist to ensure the alignment is 8, rather than 1. + // This also allows the debug view to work https://github.com/dotnet/runtime/issues/9495) + private readonly ulong _00; + private readonly ulong _01; + } } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj index a9e0faf8ac8177..444ce230f3f834 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj @@ -46,6 +46,7 @@ + diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/InstanceFieldLayoutTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/InstanceFieldLayoutTests.cs index 0a60a72a4a640b..4b02f805f3535b 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/InstanceFieldLayoutTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/InstanceFieldLayoutTests.cs @@ -850,5 +850,25 @@ public void TestInvalidByRefLikeTypes() Assert.Throws(() => type.ComputeInstanceLayout(InstanceLayoutKind.TypeAndFields)); } } + + [Fact] + public void TestWrapperAroundVectorTypes() + { + { + MetadataType type = (MetadataType)_testModule.GetType("System.Runtime.Intrinsics", "Vector128`1"); + MetadataType instantiatedType = type.MakeInstantiatedType(_context.GetWellKnownType(WellKnownType.Byte)); + Assert.Equal(16, instantiatedType.InstanceFieldAlignment.AsInt); + } + + { + DefType type = _testModule.GetType("Auto", "int8x16x2"); + Assert.Equal(16, type.InstanceFieldAlignment.AsInt); + } + + { + DefType type = _testModule.GetType("Auto", "Wrapper_int8x16x2"); + Assert.Equal(16, type.InstanceFieldAlignment.AsInt); + } + } } } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs index 0fbd0b6a35329e..4842df691dfa9c 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; +using ILCompiler; using Internal.TypeSystem; using System.Reflection; using System.Reflection.PortableExecutable; @@ -22,6 +23,7 @@ class TestTypeSystemContext : MetadataTypeSystemContext { Dictionary _modules = new Dictionary(StringComparer.OrdinalIgnoreCase); + private VectorFieldLayoutAlgorithm _vectorFieldLayoutAlgorithm; MetadataFieldLayoutAlgorithm _metadataFieldLayout = new TestMetadataFieldLayoutAlgorithm(); MetadataRuntimeInterfacesAlgorithm _metadataRuntimeInterfacesAlgorithm = new MetadataRuntimeInterfacesAlgorithm(); ArrayOfTRuntimeInterfacesAlgorithm _arrayOfTRuntimeInterfacesAlgorithm; @@ -32,6 +34,7 @@ class TestTypeSystemContext : MetadataTypeSystemContext public TestTypeSystemContext(TargetArchitecture arch) : base(new TargetDetails(arch, TargetOS.Unknown, TargetAbi.Unknown)) { + _vectorFieldLayoutAlgorithm = new VectorFieldLayoutAlgorithm(_metadataFieldLayout, true); } public ModuleDesc GetModuleForSimpleName(string simpleName) @@ -67,6 +70,10 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) { if (type == UniversalCanonType) return UniversalCanonLayoutAlgorithm.Instance; + else if (VectorFieldLayoutAlgorithm.IsVectorType(type)) + { + return _vectorFieldLayoutAlgorithm; + } return _metadataFieldLayout; } diff --git a/src/tests/readytorun/fieldlayout/fieldlayout.csproj b/src/tests/readytorun/fieldlayout/fieldlayout.csproj index 969e9b6b493b26..a9902bca098f35 100644 --- a/src/tests/readytorun/fieldlayout/fieldlayout.csproj +++ b/src/tests/readytorun/fieldlayout/fieldlayout.csproj @@ -1,5 +1,5 @@ - + exe BuildAndRun @@ -11,6 +11,6 @@ - + - \ No newline at end of file + diff --git a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs index a78ce61424dffc..ae688cdd9caee2 100644 --- a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs +++ b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs @@ -1,10 +1,281 @@ using System; +using System.Runtime.Intrinsics; -class FieldLayoutOffsetsTest +class Test +{ + // This test uses the same set of types as the type system unittests use, and attempts to validate that the R2R usage of said types works well. + // This is done by touching the various types, and then relying on the verification logic in R2R images to detect failures. + static int Main() + { + ContainsGCPointersFieldsTest.Test(); +// ExplicitTest.Test(); // Explicit layout is known to not quite match the runtime, and if enabled this set of tests will fail. + SequentialTest.Test(); + AutoTest.Test(); + EnumAlignmentTest.Test(); + AutoTestWithVector.Test(); + return 100; + } +} + +class EnumAlignmentTest +{ + static EnumAlignment.LongIntEnumStruct _fld1; + static EnumAlignment.LongIntEnumStructFieldStruct _fld2; + static EnumAlignment.IntShortEnumStruct _fld3; + static EnumAlignment.IntShortEnumStructFieldStruct _fld4; + static EnumAlignment.ShortByteEnumStruct _fld5; + static EnumAlignment.ShortByteEnumStructFieldStruct _fld6; + static EnumAlignment.LongIntEnumStructAuto _fld7; + static EnumAlignment.LongIntEnumStructAutoFieldStruct _fld8; + static EnumAlignment.IntShortEnumStructAuto _fld9; + static EnumAlignment.IntShortEnumStructAutoFieldStruct _fld10; + static EnumAlignment.ShortByteEnumStructAuto _fld11; + static EnumAlignment.ShortByteEnumStructAutoFieldStruct _fld12; + + public static void Test() + { + _fld1._1 = EnumAlignment.LongEnum.Val; + _fld1._2 = EnumAlignment.IntEnum.Val; + _fld1._3 = EnumAlignment.LongEnum.Val; + _fld1._4 = EnumAlignment.IntEnum.Val; + + _fld2._0 = 0; + _fld2._struct = _fld1; + + _fld3._1 = EnumAlignment.IntEnum.Val; + _fld3._2 = EnumAlignment.ShortEnum.Val; + _fld3._3 = EnumAlignment.IntEnum.Val; + _fld3._4 = EnumAlignment.ShortEnum.Val; + + _fld4._0 = 1; + _fld4._struct = _fld3; + + _fld5._1 = EnumAlignment.ShortEnum.Val; + _fld5._2 = EnumAlignment.ByteEnum.Val; + _fld5._3 = EnumAlignment.ShortEnum.Val; + _fld5._4 = EnumAlignment.ByteEnum.Val; + + _fld6._0 = 2; + _fld6._struct = _fld5; + + _fld7._1 = EnumAlignment.LongEnum.Val; + _fld7._2 = EnumAlignment.IntEnum.Val; + _fld7._3 = EnumAlignment.LongEnum.Val; + _fld7._4 = EnumAlignment.IntEnum.Val; + + _fld8._0 = 3; + _fld8._struct = _fld7; + + _fld9._1 = EnumAlignment.IntEnum.Val; + _fld9._2 = EnumAlignment.ShortEnum.Val; + _fld9._3 = EnumAlignment.IntEnum.Val; + _fld9._4 = EnumAlignment.ShortEnum.Val; + + _fld10._0 = 4; + _fld10._struct = _fld9; + + _fld11._1 = EnumAlignment.ShortEnum.Val; + _fld11._2 = EnumAlignment.ByteEnum.Val; + _fld11._3 = EnumAlignment.ShortEnum.Val; + _fld11._4 = EnumAlignment.ByteEnum.Val; + + _fld12._0 = 5; + _fld12._struct = _fld11; + } +} + +class AutoTest +{ + static Auto.StructWithBool _fld1; + static Auto.StructWithIntChar _fld2; + static Auto.StructWithChar _fld3; + static Auto.ClassContainingStructs _fld4 = new Auto.ClassContainingStructs(); + static Auto.BaseClass7BytesRemaining _fld5 = new Auto.BaseClass7BytesRemaining(); + static Auto.BaseClass4BytesRemaining _fld6 = new Auto.BaseClass4BytesRemaining(); + static Auto.BaseClass3BytesRemaining _fld7 = new Auto.BaseClass3BytesRemaining(); + static Auto.OptimizePartial _fld8 = new Auto.OptimizePartial(); + static Auto.Optimize7Bools _fld9 = new Auto.Optimize7Bools(); + static Auto.OptimizeAlignedFields _fld10 = new Auto.OptimizeAlignedFields(); + static Auto.OptimizeLargestField _fld11 = new Auto.OptimizeLargestField(); + static Auto.NoOptimizeMisaligned _fld12 = new Auto.NoOptimizeMisaligned(); + static Auto.NoOptimizeCharAtSize2Alignment _fld13 = new Auto.NoOptimizeCharAtSize2Alignment(); + static Auto.MinPacking _fld14 = new Auto.MinPacking(); + + public static void Test() + { + _fld1.MyStructBool = true; + + _fld2.MyStructInt = 1; + _fld2.MyStructChar = 'A'; + + _fld3.MyStructChar = 'B'; + + _fld4.MyStructWithChar = _fld3; + _fld4.MyStructWithIntChar = _fld2; + _fld4.MyStructWithBool = _fld1; + _fld4.MyString1 = "Str"; + _fld4.MyBool1 = false; + _fld4.MyBool2 = true; + + _fld5.MyBool1 = false; + _fld5.MyLong1 = 2; + _fld5.MyString1 = "Str2"; + _fld5.MyDouble1 = 1.0; + _fld5.MyByteArray1 = new byte[3]; + + _fld6.MyLong1 = 3; + _fld6.MyUint1 = 4; + + _fld7.MyBool1 = true; + _fld7.MyInt1 = 5; + _fld7.MyString1 = "str3"; + + _fld8.OptBool = false; + _fld8.OptChar = 'B'; + _fld8.NoOptLong = 6; + _fld8.NoOptString = "STR4"; + + _fld9.OptBool1 = true; + _fld9.OptBool2 = false; + _fld9.OptBool3 = true; + _fld9.OptBool4 = true; + _fld9.OptBool5 = false; + _fld9.OptBool6 = true; + _fld9.OptBool7 = false; + _fld9.NoOptBool8 = true; + _fld9.NoOptString = "STR5"; + + _fld10.OptBool1 = false; + _fld10.OptBool2 = true; + _fld10.OptBool3 = false; + _fld10.NoOptBool4 = true; + _fld10.OptChar1 = 'C'; + _fld10.OptChar2 = 'D'; + _fld10.NoOptString = "STR6"; + + _fld13.NoOptChar = 'E'; + + _fld14._value = 7; + _fld14._byte = 8; + } +} + +class AutoTestWithVector +{ + static Auto.int8x16x2 _fld1 = new Auto.int8x16x2(); + static Auto.Wrapper_int8x16x2 _fld2 = new Auto.Wrapper_int8x16x2(); + static Auto.Wrapper_int8x16x2_2 _fld3 = new Auto.Wrapper_int8x16x2_2(); + + public static void Test() + { + _fld1._0 = new Vector128(); + _fld1._1 = new Vector128(); + + _fld2.fld = _fld1; + + _fld3.fld1 = true; + _fld3.fld2 = _fld1; + } +} + +class SequentialTest +{ + static Sequential.Class1 _fld1 = new Sequential.Class1(); + static Sequential.Class2 _fld2 = new Sequential.Class2(); + static Sequential.Struct0 _fld3; + static Sequential.Struct1 _fld4; + static Sequential.ClassDoubleBool _fld5 = new Sequential.ClassDoubleBool(); + static Sequential.ClassBoolDoubleBool _fld6 = new Sequential.ClassBoolDoubleBool(); + + public static void Test() + { + _fld1.MyClass1SelfRef = _fld1; + _fld1.MyChar = 'A'; + _fld1.MyInt = 1; + _fld1.MyString = "STR"; + _fld1.MyBool = true; + + _fld2.MyClass1SelfRef = _fld1; + _fld2.MyChar = 'B'; + _fld2.MyInt = 2; + _fld2.MyString = "STR2"; + _fld2.MyBool = false; + _fld2.MyInt2 = 3; + + _fld3.b1 = true; + _fld3.b2 = false; + _fld3.b3 = true; + _fld3.i1 = 4; + _fld3.s1 = "str"; + + _fld4.MyStruct0 = _fld3; + _fld4.MyBool = false; + + _fld5.bool1 = true; + _fld5.double1 = 1.0; + + _fld6.bool1 = false; + _fld6.bool2 = true; + _fld6.double1 = 2.0; + } +} + +class ExplicitTest +{ + static Explicit.Class1 _fld1 = new Explicit.Class1(); + static Explicit.Class2 _fld2 = new Explicit.Class2(); + static Explicit.ExplicitSize _fld3 = new Explicit.ExplicitSize(); + static Explicit.ExplicitEmptyClass _fld4 = new Explicit.ExplicitEmptyClass(); + static Explicit.ExplicitEmptyClassSize0 _fld5 = new Explicit.ExplicitEmptyClassSize0(); + static Explicit.ExplicitEmptyStruct _fld6 = new Explicit.ExplicitEmptyStruct(); + + public static void Test() + { + _fld1.Bar = true; + _fld1.Baz = 'A'; + + _fld2.Baz = 'B'; + _fld2.Bar = false; + _fld2.Lol = 1; + _fld2.Omg = 2; + + _fld3.Omg = 3; + _fld3.Lol = 4; + } +} +class ContainsGCPointersFieldsTest { static ContainsGCPointers.NoPointers _fld1; static ContainsGCPointers.StillNoPointers _fld2; static ContainsGCPointers.ClassNoPointers _fld3 = new ContainsGCPointers.ClassNoPointers(); static ContainsGCPointers.HasPointers _fld4; static ContainsGCPointers.FieldHasPointers _fld5; + static ContainsGCPointers.ClassHasPointers _fld6 = new ContainsGCPointers.ClassHasPointers(); + static ContainsGCPointers.BaseClassHasPointers _fld7 = new ContainsGCPointers.BaseClassHasPointers(); + static ContainsGCPointers.ClassHasIntArray _fld8 = new ContainsGCPointers.ClassHasIntArray(); + static ContainsGCPointers.ClassHasArrayOfClassType _fld9 = new ContainsGCPointers.ClassHasArrayOfClassType(); + + public static void Test() + { + _fld1.int1 = 1; + _fld1.byte1 = 2; + _fld1.char1 = '0'; + _fld2.bool1 = true; + + _fld2.noPointers1 = _fld1; + + _fld3.char1 = '2'; + + _fld4.string1 = "STR"; + + _fld5.hasPointers1.string1 = "STR2"; + + _fld6.classHasPointers1 = new ContainsGCPointers.ClassHasPointers(); + + _fld7.classHasPointers1 = new ContainsGCPointers.ClassHasPointers(); + + _fld8.intArrayField = new int[1]; + + _fld9.classTypeArray = new ContainsGCPointers.ClassNoPointers[1]; + } } From e5995746ca753a4e7c6ae646b4cb7f5e02d3431a Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 10 Aug 2022 17:33:39 -0700 Subject: [PATCH 4/9] Re-enable disabled test --- src/tests/issues.targets | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 66b2d741706d88..9a4aceb941aa5a 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -916,12 +916,6 @@ https://github.com/dotnet/runtime/issues/62881 - - https://github.com/dotnet/runtime/issues/63856 - - - https://github.com/dotnet/runtime/issues/63856 - From 3ef8ee4a100d60f2e5077afea80749cb0187a897 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 10 Aug 2022 17:35:06 -0700 Subject: [PATCH 5/9] Remove file that shouldn't be added as part of the new test --- .../fieldlayout/InstanceFieldLayout.cs | 414 ------------------ 1 file changed, 414 deletions(-) delete mode 100644 src/tests/readytorun/fieldlayout/InstanceFieldLayout.cs diff --git a/src/tests/readytorun/fieldlayout/InstanceFieldLayout.cs b/src/tests/readytorun/fieldlayout/InstanceFieldLayout.cs deleted file mode 100644 index 9c8416224e0a5a..00000000000000 --- a/src/tests/readytorun/fieldlayout/InstanceFieldLayout.cs +++ /dev/null @@ -1,414 +0,0 @@ -// 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.Runtime.InteropServices; -using System.Runtime.Intrinsics; - -#pragma warning disable 169 - -namespace ContainsGCPointers -{ - struct NoPointers - { - public int int1; - public byte byte1; - public char char1; - } - - struct StillNoPointers - { - public NoPointers noPointers1; - public bool bool1; - } - - class ClassNoPointers - { - public char char1; - } - - struct HasPointers - { - public string string1; - } - - struct FieldHasPointers - { - public HasPointers hasPointers1; - } - - class ClassHasPointers - { - public ClassHasPointers classHasPointers1; - } - - class BaseClassHasPointers : ClassHasPointers - { - } - - public class ClassHasIntArray - { - public int[] intArrayField; - } - - public class ClassHasArrayOfClassType - { - public ClassNoPointers[] classTypeArray; - } -} - -namespace Explicit -{ - [StructLayout(LayoutKind.Explicit)] - class Class1 - { - public static int Stat; - [FieldOffset(4)] - public bool Bar; - [FieldOffset(10)] - public char Baz; - } - - [StructLayout(LayoutKind.Explicit)] - class Class2 : Class1 - { - [FieldOffset(0)] - public int Lol; - [FieldOffset(20)] - public byte Omg; - } - - [StructLayout(LayoutKind.Explicit, Size = 40)] - class ExplicitSize - { - [FieldOffset(0)] - public int Lol; - [FieldOffset(20)] - public byte Omg; - } - - [StructLayout(LayoutKind.Explicit)] - public class ExplicitEmptyClass - { - } - - [StructLayout(LayoutKind.Explicit, Size = 0)] - public class ExplicitEmptyClassSize0 - { - } - - [StructLayout(LayoutKind.Explicit)] - public struct ExplicitEmptyStruct - { - } - - [StructLayout(LayoutKind.Explicit)] - ref struct MisalignedPointer - { - [FieldOffset(2)] - public object O; - } - - [StructLayout(LayoutKind.Explicit)] - ref struct MisalignedByRef - { - [FieldOffset(2)] - public ByRefStruct O; - } - - ref struct ByRefStruct - { - } -} - -namespace Sequential -{ - [StructLayout(LayoutKind.Sequential)] - class Class1 - { - public int MyInt; - public bool MyBool; - public char MyChar; - public string MyString; - public byte[] MyByteArray; - public Class1 MyClass1SelfRef; - } - - [StructLayout(LayoutKind.Sequential)] - class Class2 : Class1 - { - public int MyInt2; - } - - // [StructLayout(LayoutKind.Sequential)] is applied by default by the C# compiler - struct Struct0 - { - public bool b1; - public bool b2; - public bool b3; - public int i1; - public string s1; - } - - // [StructLayout(LayoutKind.Sequential)] is applied by default by the C# compiler - struct Struct1 - { - public Struct0 MyStruct0; - public bool MyBool; - } - - [StructLayout(LayoutKind.Sequential)] - public class ClassDoubleBool - { - public double double1; - public bool bool1; - } - - [StructLayout(LayoutKind.Sequential)] - public class ClassBoolDoubleBool - { - public bool bool1; - public double double1; - public bool bool2; - } -} - -namespace Auto -{ - [StructLayout(LayoutKind.Auto)] - struct StructWithBool - { - public bool MyStructBool; - } - - [StructLayout(LayoutKind.Auto)] - struct StructWithIntChar - { - public char MyStructChar; - public int MyStructInt; - } - - [StructLayout(LayoutKind.Auto)] - struct StructWithChar - { - public char MyStructChar; - } - - class ClassContainingStructs - { - public static int MyStaticInt; - - public StructWithBool MyStructWithBool; - public bool MyBool1; - public char MyChar1; - public int MyInt; - public double MyDouble; - public long MyLong; - public byte[] MyByteArray; - public string MyString1; - public bool MyBool2; - public StructWithIntChar MyStructWithIntChar; - public StructWithChar MyStructWithChar; - } - - class BaseClass7BytesRemaining - { - public bool MyBool1; - public double MyDouble1; - public long MyLong1; - public byte[] MyByteArray1; - public string MyString1; - } - - class BaseClass4BytesRemaining - { - public long MyLong1; - public uint MyUint1; - } - - class BaseClass3BytesRemaining - { - public int MyInt1; - public string MyString1; - public bool MyBool1; - } - - class OptimizePartial : BaseClass7BytesRemaining - { - public bool OptBool; - public char OptChar; - public long NoOptLong; - public string NoOptString; - } - - class Optimize7Bools : BaseClass7BytesRemaining - { - public bool OptBool1; - public bool OptBool2; - public bool OptBool3; - public bool OptBool4; - public bool OptBool5; - public bool OptBool6; - public bool OptBool7; - public bool NoOptBool8; - public string NoOptString; - } - - class OptimizeAlignedFields : BaseClass7BytesRemaining - { - public bool OptBool1; - public bool OptBool2; - public bool OptBool3; - public bool NoOptBool4; - public char OptChar1; - public char OptChar2; - public string NoOptString; - } - - class OptimizeLargestField : BaseClass4BytesRemaining - { - public bool NoOptBool; - public char NoOptChar; - public int OptInt; - public string NoOptString; - } - - class NoOptimizeMisaligned : BaseClass3BytesRemaining - { - public char NoOptChar; - public int NoOptInt; - public string NoOptString; - } - - class NoOptimizeCharAtSize2Alignment : BaseClass3BytesRemaining - { - public char NoOptChar; - } - - [StructLayout(LayoutKind.Auto, Pack = 1)] - struct MinPacking - { - public byte _byte; - public T _value; - } - - [StructLayout(LayoutKind.Auto)] - struct int8x16x2 - { - public Vector128 _0; - public Vector128 _1; - } - - struct Wrapper_int8x16x2 - { - public int8x16x2 fld; - } -} - -namespace IsByRefLike -{ - public ref struct ByRefLikeStruct - { - public ref object ByRef; - } - - public struct NotByRefLike - { - public int X; - } -} - -namespace EnumAlignment -{ - public enum ByteEnum : byte {} - public enum ShortEnum : short {} - public enum IntEnum : int {} - public enum LongEnum : long {} - - public struct LongIntEnumStruct - { - public LongEnum _1; - public IntEnum _2; - public LongEnum _3; - public IntEnum _4; - } - - public struct LongIntEnumStructFieldStruct - { - public byte _0; - public LongIntEnumStruct _struct; - } - - public struct IntShortEnumStruct - { - public IntEnum _1; - public ShortEnum _2; - public IntEnum _3; - public ShortEnum _4; - } - - public struct IntShortEnumStructFieldStruct - { - public byte _0; - public IntShortEnumStruct _struct; - } - - public struct ShortByteEnumStruct - { - public ShortEnum _1; - public ByteEnum _2; - public ShortEnum _3; - public ByteEnum _4; - } - - public struct ShortByteEnumStructFieldStruct - { - public byte _0; - public ShortByteEnumStruct _struct; - } - - [StructLayout(LayoutKind.Auto)] - public struct LongIntEnumStructAuto - { - public LongEnum _1; - public IntEnum _2; - public LongEnum _3; - public IntEnum _4; - } - - public struct LongIntEnumStructAutoFieldStruct - { - public byte _0; - public LongIntEnumStructAuto _struct; - } - - [StructLayout(LayoutKind.Auto)] - public struct IntShortEnumStructAuto - { - public IntEnum _1; - public ShortEnum _2; - public IntEnum _3; - public ShortEnum _4; - } - - public struct IntShortEnumStructAutoFieldStruct - { - public byte _0; - public IntShortEnumStructAuto _struct; - } - - [StructLayout(LayoutKind.Auto)] - public struct ShortByteEnumStructAuto - { - public ShortEnum _1; - public ByteEnum _2; - public ShortEnum _3; - public ByteEnum _4; - } - - public struct ShortByteEnumStructAutoFieldStruct - { - public byte _0; - public ShortByteEnumStructAuto _struct; - } -} From 0cdf40bd4d3ae5f5e1c2f7ca242c38a5203e3bf7 Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Thu, 11 Aug 2022 22:26:14 +0200 Subject: [PATCH 6/9] Make a few test types public to silence unassigned field errors --- .../CoreTestAssembly/InstanceFieldLayout.cs | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs index cf9fed79b5b60a..8c16f4811ee91c 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs @@ -9,40 +9,40 @@ namespace ContainsGCPointers { - struct NoPointers + public struct NoPointers { public int int1; public byte byte1; public char char1; } - struct StillNoPointers + public struct StillNoPointers { public NoPointers noPointers1; public bool bool1; } - class ClassNoPointers + public class ClassNoPointers { public char char1; } - struct HasPointers + public struct HasPointers { public string string1; } - struct FieldHasPointers + public struct FieldHasPointers { public HasPointers hasPointers1; } - class ClassHasPointers + public class ClassHasPointers { public ClassHasPointers classHasPointers1; } - class BaseClassHasPointers : ClassHasPointers + public class BaseClassHasPointers : ClassHasPointers { } @@ -51,7 +51,7 @@ public class ClassHasIntArray public int[] intArrayField; } - class ClassHasArrayOfClassType + public class ClassHasArrayOfClassType { public ClassNoPointers[] classTypeArray; } @@ -124,7 +124,7 @@ ref struct ByRefStruct namespace Sequential { [StructLayout(LayoutKind.Sequential)] - class Class1 + public class Class1 { public int MyInt; public bool MyBool; @@ -135,13 +135,13 @@ class Class1 } [StructLayout(LayoutKind.Sequential)] - class Class2 : Class1 + public class Class2 : Class1 { public int MyInt2; } // [StructLayout(LayoutKind.Sequential)] is applied by default by the C# compiler - struct Struct0 + public struct Struct0 { public bool b1; public bool b2; @@ -151,7 +151,7 @@ struct Struct0 } // [StructLayout(LayoutKind.Sequential)] is applied by default by the C# compiler - struct Struct1 + public struct Struct1 { public Struct0 MyStruct0; public bool MyBool; @@ -176,25 +176,25 @@ public class ClassBoolDoubleBool namespace Auto { [StructLayout(LayoutKind.Auto)] - struct StructWithBool + public struct StructWithBool { public bool MyStructBool; } [StructLayout(LayoutKind.Auto)] - struct StructWithIntChar + public struct StructWithIntChar { public char MyStructChar; public int MyStructInt; } [StructLayout(LayoutKind.Auto)] - struct StructWithChar + public struct StructWithChar { public char MyStructChar; } - class ClassContainingStructs + public class ClassContainingStructs { public static int MyStaticInt; @@ -211,7 +211,7 @@ class ClassContainingStructs public StructWithChar MyStructWithChar; } - class BaseClass7BytesRemaining + public class BaseClass7BytesRemaining { public bool MyBool1; public double MyDouble1; @@ -220,20 +220,20 @@ class BaseClass7BytesRemaining public string MyString1; } - class BaseClass4BytesRemaining + public class BaseClass4BytesRemaining { public long MyLong1; public uint MyUint1; } - class BaseClass3BytesRemaining + public class BaseClass3BytesRemaining { public int MyInt1; public string MyString1; public bool MyBool1; } - class OptimizePartial : BaseClass7BytesRemaining + public class OptimizePartial : BaseClass7BytesRemaining { public bool OptBool; public char OptChar; @@ -241,7 +241,7 @@ class OptimizePartial : BaseClass7BytesRemaining public string NoOptString; } - class Optimize7Bools : BaseClass7BytesRemaining + public class Optimize7Bools : BaseClass7BytesRemaining { public bool OptBool1; public bool OptBool2; @@ -254,7 +254,7 @@ class Optimize7Bools : BaseClass7BytesRemaining public string NoOptString; } - class OptimizeAlignedFields : BaseClass7BytesRemaining + public class OptimizeAlignedFields : BaseClass7BytesRemaining { public bool OptBool1; public bool OptBool2; @@ -265,7 +265,7 @@ class OptimizeAlignedFields : BaseClass7BytesRemaining public string NoOptString; } - class OptimizeLargestField : BaseClass4BytesRemaining + public class OptimizeLargestField : BaseClass4BytesRemaining { public bool NoOptBool; public char NoOptChar; @@ -273,38 +273,38 @@ class OptimizeLargestField : BaseClass4BytesRemaining public string NoOptString; } - class NoOptimizeMisaligned : BaseClass3BytesRemaining + public class NoOptimizeMisaligned : BaseClass3BytesRemaining { public char NoOptChar; public int NoOptInt; public string NoOptString; } - class NoOptimizeCharAtSize2Alignment : BaseClass3BytesRemaining + public class NoOptimizeCharAtSize2Alignment : BaseClass3BytesRemaining { public char NoOptChar; } [StructLayout(LayoutKind.Auto, Pack = 1)] - struct MinPacking + public struct MinPacking { public byte _byte; public T _value; } [StructLayout(LayoutKind.Auto)] - struct int8x16x2 + public struct int8x16x2 { public Vector128 _0; public Vector128 _1; } - struct Wrapper_int8x16x2 + public struct Wrapper_int8x16x2 { public int8x16x2 fld; } - struct Wrapper_int8x16x2_2 + public struct Wrapper_int8x16x2_2 { public bool fld1; public int8x16x2 fld2; From 174590f37963ab56750e208446d9ab961bdf5c5a Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 16 Aug 2022 12:09:44 -0700 Subject: [PATCH 7/9] Update comments and add more testing --- .../Common/MetadataFieldLayoutAlgorithm.cs | 9 +- .../ArchitectureSpecificFieldLayoutTests.cs | 62 ++++++++++ .../CoreTestAssembly/InstanceFieldLayout.cs | 108 ++++++++++++++++++ .../fieldlayout/fieldlayouttests.cs | 15 +++ 4 files changed, 193 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 975e5a9d0fcd7a..70b529a60601ce 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -469,6 +469,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, if (IsByValueClass(fieldType)) { + // Valuetypes which are not primitives or enums instanceValueClassFieldCount++; } else if (fieldType.IsGCPointer) @@ -522,12 +523,17 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, if (IsByValueClass(fieldType)) { + // This block handles valuetypes which are not primitives or enums, it only has a meaningful effect, if the + // type has an alignment greater than pointer size. largestAlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequired); instanceValueClassFieldsArr[instanceValueClassFieldCount++] = field; } else { - // non-value-type fields always require pointer alignment + // non-value-type (and primitive type) fields will add an alignment requirement of pointer size + // This alignment requirement will not be significant in the final alignment calculation unlesss the + // type is greater than the size of a single pointer. + // // This does not account for types that are marked IsAlign8Candidate due to 8-byte fields // but that is explicitly handled when we calculate the final alignment for the type. @@ -542,6 +548,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, } else { + Debug.Assert(fieldType.IsPrimitive || fieldType.IsPointer || fieldType.IsFunctionPointer || fieldType.IsEnum || fieldType.IsByRef); int log2size = CalculateLog2(fieldSizeAndAlignment.Size.AsInt); instanceNonGCPointerFieldsArr[log2size][instanceNonGCPointerFieldsCount[log2size]++] = field; diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs index 067486f858c168..a55aa430f2fccb 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs @@ -414,5 +414,67 @@ public void TestAlignmentBehavior_ShortByteEnumStructAuto() Assert.Equal(0x4, tX86FieldStruct.GetField("_struct").Offset.AsInt); Assert.Equal(0x4, tARMFieldStruct.GetField("_struct").Offset.AsInt); } + + [Fact] + public void TestAlignmentBehavior_StructStructByte_StructByteAuto() + { + string _namespace = "Sequential"; + string _type = "StructStructByte_StructByteAuto"; + + MetadataType tX64 = _testModuleX64.GetType(_namespace, _type); + MetadataType tX86 = _testModuleX86.GetType(_namespace, _type); + MetadataType tARM = _testModuleARM.GetType(_namespace, _type); + + Assert.Equal(0x1, tX64.InstanceFieldAlignment.AsInt); + Assert.Equal(0x1, tARM.InstanceFieldAlignment.AsInt); + Assert.Equal(0x1, tX86.InstanceFieldAlignment.AsInt); + + Assert.Equal(0x2, tX64.InstanceFieldSize.AsInt); + Assert.Equal(0x2, tARM.InstanceFieldSize.AsInt); + Assert.Equal(0x2, tX86.InstanceFieldSize.AsInt); + + Assert.Equal(0x0, tX64.GetField("fld1").Offset.AsInt); + Assert.Equal(0x0, tARM.GetField("fld1").Offset.AsInt); + Assert.Equal(0x0, tX86.GetField("fld1").Offset.AsInt); + + Assert.Equal(0x1, tX64.GetField("fld2").Offset.AsInt); + Assert.Equal(0x1, tARM.GetField("fld2").Offset.AsInt); + Assert.Equal(0x1, tX86.GetField("fld2").Offset.AsInt); + } + + [Theory] + [InlineData("StructStructByte_StructByteAuto", new int[]{1,1,1}, new int[]{2,2,2})] + [InlineData("StructStructByte_Struct2BytesAuto", new int[]{2,2,2}, new int[]{4,4,4})] + [InlineData("StructStructByte_Struct3BytesAuto", new int[]{4,4,4}, new int[]{8,8,8})] + [InlineData("StructStructByte_Struct4BytesAuto", new int[]{4,4,4}, new int[]{8,8,8})] + [InlineData("StructStructByte_Struct5BytesAuto", new int[]{8,4,4}, new int[]{16,12,12})] + [InlineData("StructStructByte_Struct8BytesAuto", new int[]{8,4,4}, new int[]{16,12,12})] + [InlineData("StructStructByte_Struct9BytesAuto", new int[]{8,4,4}, new int[]{24,16,16})] + public void TestAlignmentBehavior_AutoAlignmentRules(string wrapperType, int[] alignment, int[] size) + { + string _namespace = "Sequential"; + string _type = wrapperType; + + MetadataType tX64 = _testModuleX64.GetType(_namespace, _type); + MetadataType tX86 = _testModuleX86.GetType(_namespace, _type); + MetadataType tARM = _testModuleARM.GetType(_namespace, _type); + + Assert.Equal(alignment[0], tX64.InstanceFieldAlignment.AsInt); + Assert.Equal(alignment[1], tARM.InstanceFieldAlignment.AsInt); + Assert.Equal(alignment[2], tX86.InstanceFieldAlignment.AsInt); + + Assert.Equal(size[0], tX64.InstanceFieldSize.AsInt); + Assert.Equal(size[1], tARM.InstanceFieldSize.AsInt); + Assert.Equal(size[2], tX86.InstanceFieldSize.AsInt); + + Assert.Equal(0x0, tX64.GetField("fld1").Offset.AsInt); + Assert.Equal(0x0, tARM.GetField("fld1").Offset.AsInt); + Assert.Equal(0x0, tX86.GetField("fld1").Offset.AsInt); + + Assert.Equal(alignment[0], tX64.GetField("fld2").Offset.AsInt); + Assert.Equal(alignment[1], tARM.GetField("fld2").Offset.AsInt); + Assert.Equal(alignment[2], tX86.GetField("fld2").Offset.AsInt); + } + } } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs index 8c16f4811ee91c..c360958375e010 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs @@ -171,6 +171,47 @@ public class ClassBoolDoubleBool public double double1; public bool bool2; } + + public struct StructByte + { + public byte fld1; + } + + public struct StructStructByte_StructByteAuto + { + public StructByte fld1; + public Auto.StructByte fld2; + } + public struct StructStructByte_Struct2BytesAuto + { + public StructByte fld1; + public Auto.Struct2Bytes fld2; + } + public struct StructStructByte_Struct3BytesAuto + { + public StructByte fld1; + public Auto.Struct3Bytes fld2; + } + public struct StructStructByte_Struct4BytesAuto + { + public StructByte fld1; + public Auto.Struct4Bytes fld2; + } + public struct StructStructByte_Struct5BytesAuto + { + public StructByte fld1; + public Auto.Struct5Bytes fld2; + } + public struct StructStructByte_Struct8BytesAuto + { + public StructByte fld1; + public Auto.Struct8Bytes fld2; + } + public struct StructStructByte_Struct9BytesAuto + { + public StructByte fld1; + public Auto.Struct9Bytes fld2; + } } namespace Auto @@ -309,6 +350,73 @@ public struct Wrapper_int8x16x2_2 public bool fld1; public int8x16x2 fld2; } + + [StructLayout(LayoutKind.Auto)] + public struct StructByte + { + public byte fld1; + } + + [StructLayout(LayoutKind.Auto)] + public struct Struct2Bytes + { + public byte fld1; + public byte fld2; + } + + [StructLayout(LayoutKind.Auto)] + public struct Struct3Bytes + { + public byte fld1; + public byte fld2; + public byte fld3; + } + + [StructLayout(LayoutKind.Auto)] + public struct Struct4Bytes + { + public byte fld1; + public byte fld2; + public byte fld3; + public byte fld4; + } + + [StructLayout(LayoutKind.Auto)] + public struct Struct5Bytes + { + public byte fld1; + public byte fld2; + public byte fld3; + public byte fld4; + public byte fld5; + } + + [StructLayout(LayoutKind.Auto)] + public struct Struct8Bytes + { + public byte fld1; + public byte fld2; + public byte fld3; + public byte fld4; + public byte fld5; + public byte fld6; + public byte fld7; + public byte fld8; + } + + [StructLayout(LayoutKind.Auto)] + public struct Struct9Bytes + { + public byte fld1; + public byte fld2; + public byte fld3; + public byte fld4; + public byte fld5; + public byte fld6; + public byte fld7; + public byte fld8; + public byte fld9; + } } namespace IsByRefLike diff --git a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs index ae688cdd9caee2..690f6ff9426d42 100644 --- a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs +++ b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs @@ -186,6 +186,13 @@ class SequentialTest static Sequential.Struct1 _fld4; static Sequential.ClassDoubleBool _fld5 = new Sequential.ClassDoubleBool(); static Sequential.ClassBoolDoubleBool _fld6 = new Sequential.ClassBoolDoubleBool(); + static Sequential.StructStructByte_StructByteAuto _fld7; + static Sequential.StructStructByte_Struct2BytesAuto _fld8; + static Sequential.StructStructByte_Struct3BytesAuto _fld9; + static Sequential.StructStructByte_Struct4BytesAuto _fld10; + static Sequential.StructStructByte_Struct5BytesAuto _fld11; + static Sequential.StructStructByte_Struct8BytesAuto _fld12; + static Sequential.StructStructByte_Struct9BytesAuto _fld13; public static void Test() { @@ -217,6 +224,14 @@ public static void Test() _fld6.bool1 = false; _fld6.bool2 = true; _fld6.double1 = 2.0; + + _fld7.fld2 = default(Auto.StructByte); + _fld8.fld2 = default(Auto.Struct2Bytes); + _fld9.fld2 = default(Auto.Struct3Bytes); + _fld10.fld2 = default(Auto.Struct4Bytes); + _fld11.fld2 = default(Auto.Struct5Bytes); + _fld12.fld2 = default(Auto.Struct8Bytes); + _fld13.fld2 = default(Auto.Struct9Bytes); } } From ffda43dab3f7ef80ab73072c98f3dbd1548f4a5b Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 16 Aug 2022 18:17:11 -0700 Subject: [PATCH 8/9] First stab at support for proper 128bit integer layout and abi --- src/coreclr/jit/lclvars.cpp | 33 ++++++++- src/coreclr/jit/morph.cpp | 22 ++++++ src/coreclr/jit/register_arg_convention.cpp | 6 +- src/coreclr/jit/register_arg_convention.h | 2 +- .../ReadyToRun/ArgIterator.cs | 21 +++++- .../ArchitectureSpecificFieldLayoutTests.cs | 74 +++++++++++++++++++ .../CoreTestAssembly/InstanceFieldLayout.cs | 24 ++++++ .../CoreTestAssembly/Platform.cs | 18 +++++ .../ILCompiler.TypeSystem.Tests.csproj | 1 + .../TestTypeSystemContext.cs | 9 ++- src/coreclr/vm/callingconvention.h | 21 +++++- src/coreclr/vm/methodtablebuilder.cpp | 26 +++---- .../fieldlayout/fieldlayouttests.cs | 5 +- 13 files changed, 234 insertions(+), 28 deletions(-) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 4aa1b1e4d9865d..2c8597e99ae05b 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -717,6 +717,13 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo, unsigned skipArgs, un unsigned cSlotsToEnregister = cSlots; #if defined(TARGET_ARM64) + if (!compMacOsArm64Abi()) + { + if (!info.compIsVarArgs && (cSlots == 2) && varTypeIsStruct(argType) && varDscInfo->canEnreg(argType, cSlots) && hfaType == TYP_UNDEF && info.compCompHnd->getClassAlignmentRequirement(varDsc->GetStructHnd()) == 16) + { + compArgSize += varDscInfo->alignReg(argType, 2) * REGSIZE_BYTES; + } + } if (compFeatureArgSplit()) { @@ -1228,11 +1235,21 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo, unsigned skipArgs, un unsigned argAlignment = cAlign * TARGET_POINTER_SIZE; #else unsigned argAlignment = eeGetArgSizeAlignment(origArgType, (hfaType == TYP_FLOAT)); + +#ifdef TARGET_ARM64 + if (!info.compIsVarArgs && varDscInfo->stackArgSize == 16 && !varTypeUsesFloatReg(argType) && varTypeIsStruct(argType) && info.compCompHnd->getClassAlignmentRequirement(varDsc->GetStructHnd()) == 16) + { + // TODO-Cleanup: use "eeGetArgSizeAlignment" here. See also: https://github.com/dotnet/runtime/issues/46026. + argAlignment = 16; + } +#endif // TARGET_ARM64 + // We expect the following rounding operation to be a noop on all // ABIs except ARM (where we have 8-byte aligned args) and macOS // ARM64 (that allows to pack multiple smaller parameters in a - // single stack slot). - assert(compMacOsArm64Abi() || ((varDscInfo->stackArgSize % argAlignment) == 0)); + // single stack slot), and ARM64 128 bit layout structures passed by value + // that would naturally be passed in integer registers if there was space + assert(compMacOsArm64Abi() || (argAlignment == 16) || ((varDscInfo->stackArgSize % argAlignment) == 0)); #endif varDscInfo->stackArgSize = roundUp(varDscInfo->stackArgSize, argAlignment); @@ -6317,11 +6334,19 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, } #endif // TARGET_ARM const bool isFloatHfa = (varDsc->lvIsHfa() && (varDsc->GetHfaType() == TYP_FLOAT)); - const unsigned argAlignment = eeGetArgSizeAlignment(varDsc->lvType, isFloatHfa); - if (compMacOsArm64Abi()) + unsigned argAlignment = eeGetArgSizeAlignment(varDsc->lvType, isFloatHfa); + +#if TARGET_ARM64 + if (!info.compIsVarArgs && varDsc->lvSize() == 16 && varDsc->lvType == TYP_STRUCT && !isFloatHfa && info.compCompHnd->getClassAlignmentRequirement(varDsc->GetStructHnd()) == 16) + { + // TODO-Cleanup: use "eeGetArgSizeAlignment" here. See also: https://github.com/dotnet/runtime/issues/46026. + argAlignment = 16; + } + if (compMacOsArm64Abi() || (argAlignment == 16)) { argOffs = roundUp(argOffs, argAlignment); } +#endif assert((argSize % argAlignment) == 0); assert((argOffs % argAlignment) == 0); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index fff00343e888ab..1ab32540a8282b 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2510,6 +2510,28 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call } else { + if (!callIsVararg && size == 2) + { + assert(varTypeIsStruct(argSigType)); + if (comp->info.compCompHnd->getClassAlignmentRequirement(argSigClass) == 16) + { + // Implement rule C.11 from the ARM64 Procedure calling standard. + argAlignBytes = 16; + + // Implement rule C.10 from the ARM64 Procedure Calling standard. + // If the argument has an alignment of 16 then the NGRN is rounded up to the next even number. + // + // This rule is not used for Apple platforms. See https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms + if (!compMacOsArm64Abi()) + { + if ((intArgRegNum & 1) && size == 2) + { + intArgRegNum++; + } + } + } + } + // Check if the last register needed is still in the int argument register range. isRegArg = (intArgRegNum + (size - 1)) < maxRegArgs; diff --git a/src/coreclr/jit/register_arg_convention.cpp b/src/coreclr/jit/register_arg_convention.cpp index a90e61c3a59fde..c8e414d8822a43 100644 --- a/src/coreclr/jit/register_arg_convention.cpp +++ b/src/coreclr/jit/register_arg_convention.cpp @@ -71,7 +71,7 @@ bool InitVarDscInfo::enoughAvailRegs(var_types type, unsigned numRegs /* = 1 */) return regArgNum(type) + numRegs - backFillCount <= maxRegArgNum(type); } -#ifdef TARGET_ARM +#if defined(TARGET_ARM) || defined(TARGET_ARM64) unsigned InitVarDscInfo::alignReg(var_types type, unsigned requiredRegAlignment) { assert(requiredRegAlignment > 0); @@ -91,10 +91,12 @@ unsigned InitVarDscInfo::alignReg(var_types type, unsigned requiredRegAlignment) unsigned cAlignSkipped = requiredRegAlignment - alignMask; assert(cAlignSkipped == 1); // Alignment is currently only 1 or 2, so misalignment can only be 1. +#ifdef TARGET_ARM if (varTypeIsFloating(type)) { fltArgSkippedRegMask |= genMapFloatRegArgNumToRegMask(floatRegArgNum); } +#endif assert(regArgNum(type) + cAlignSkipped <= maxRegArgNum(type)); // if equal, then we aligned the last slot, and the // arg can't be enregistered @@ -102,7 +104,7 @@ unsigned InitVarDscInfo::alignReg(var_types type, unsigned requiredRegAlignment) return cAlignSkipped; } -#endif // TARGET_ARM +#endif // TARGET_ARM || TARGET_ARM64 bool InitVarDscInfo::canEnreg(var_types type, unsigned numRegs /* = 1 */) { diff --git a/src/coreclr/jit/register_arg_convention.h b/src/coreclr/jit/register_arg_convention.h index 0085e8ac80a2ea..5537d71c847ede 100644 --- a/src/coreclr/jit/register_arg_convention.h +++ b/src/coreclr/jit/register_arg_convention.h @@ -70,7 +70,7 @@ struct InitVarDscInfo // Returns the first argument register of the allocated set. unsigned allocRegArg(var_types type, unsigned numRegs = 1); -#ifdef TARGET_ARM +#if defined(TARGET_ARM) || defined(TARGET_ARM64) // We are aligning the register to an ABI-required boundary, such as putting // double-precision floats in even-numbered registers, by skipping one register. // "requiredRegAlignment" is the amount to align to: 1 for no alignment (everything diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ArgIterator.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ArgIterator.cs index 73e0a1d4087d75..efd5b866290b57 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ArgIterator.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ArgIterator.cs @@ -1257,6 +1257,16 @@ public int GetNextOffset() Debug.Assert(_transitionBlock.IsAppleArm64ABI || (cbArg % _transitionBlock.PointerSize) == 0); int regSlots = ALIGN_UP(cbArg, _transitionBlock.PointerSize) / _transitionBlock.PointerSize; + + if (!IsVarArg && !_transitionBlock.IsAppleArm64ABI && ((_arm64IdxGenReg & 1) == 1) && regSlots == 2 && ((DefType)_argTypeHandle.GetRuntimeTypeHandle()).InstanceFieldAlignment == new LayoutInt(16)) + { + // Implement rule C.10 from the ARM64 Procedure Calling standard. + // If the argument has an alignment of 16 then the NGRN is rounded up to the next even number. + // + // This rule is not used for Apple platforms. See https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms + _arm64IdxGenReg++; + } + // Only x0-x7 are valid argument registers (x8 is always the return buffer) if (_arm64IdxGenReg + regSlots <= 8) { @@ -1288,9 +1298,9 @@ public int GetNextOffset() } } + int alignment = 1; if (_transitionBlock.IsAppleArm64ABI) { - int alignment; if (!isValueType) { Debug.Assert((cbArg & (cbArg - 1)) == 0); @@ -1304,9 +1314,16 @@ public int GetNextOffset() { alignment = 8; } - _arm64OfsStack = ALIGN_UP(_arm64OfsStack, alignment); } + if (!IsVarArg && cbArg == 16 && ((DefType)_argTypeHandle.GetRuntimeTypeHandle()).InstanceFieldAlignment == new LayoutInt(16)) + { + // Implement rule C.11 from the ARM64 Procedure Calling standard. + alignment = 16; + } + + _arm64OfsStack = ALIGN_UP(_arm64OfsStack, alignment); + argOfs = _transitionBlock.OffsetOfArgs + _arm64OfsStack; _arm64OfsStack += cbArg; return argOfs; diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs index a55aa430f2fccb..7b3be7676c03b0 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs @@ -19,9 +19,16 @@ public class ArchitectureSpecificFieldLayoutTests ModuleDesc _testModuleX86; TestTypeSystemContext _contextX64; ModuleDesc _testModuleX64; + TestTypeSystemContext _contextX64Windows; + ModuleDesc _testModuleX64Windows; + TestTypeSystemContext _contextX64Linux; + ModuleDesc _testModuleX64Linux; TestTypeSystemContext _contextARM; ModuleDesc _testModuleARM; + TestTypeSystemContext _contextARM64; + ModuleDesc _testModuleARM64; + public ArchitectureSpecificFieldLayoutTests() { _contextX64 = new TestTypeSystemContext(TargetArchitecture.X64); @@ -30,6 +37,18 @@ public ArchitectureSpecificFieldLayoutTests() _testModuleX64 = systemModuleX64; + _contextX64Linux = new TestTypeSystemContext(TargetArchitecture.X64, TargetOS.Linux); + var systemModuleX64Linux = _contextX64Linux.CreateModuleForSimpleName("CoreTestAssembly"); + _contextX64Linux.SetSystemModule(systemModuleX64Linux); + + _testModuleX64Linux = systemModuleX64Linux; + + _contextX64Windows = new TestTypeSystemContext(TargetArchitecture.X64, TargetOS.Windows); + var systemModuleX64Windows = _contextX64Windows.CreateModuleForSimpleName("CoreTestAssembly"); + _contextX64Windows.SetSystemModule(systemModuleX64Windows); + + _testModuleX64Windows = systemModuleX64Windows; + _contextARM = new TestTypeSystemContext(TargetArchitecture.ARM); var systemModuleARM = _contextARM.CreateModuleForSimpleName("CoreTestAssembly"); _contextARM.SetSystemModule(systemModuleARM); @@ -41,6 +60,12 @@ public ArchitectureSpecificFieldLayoutTests() _contextX86.SetSystemModule(systemModuleX86); _testModuleX86 = systemModuleX86; + + _contextARM64 = new TestTypeSystemContext(TargetArchitecture.ARM64); + var systemModuleARM64 = _contextARM64.CreateModuleForSimpleName("CoreTestAssembly"); + _contextARM64.SetSystemModule(systemModuleARM64); + + _testModuleARM64 = systemModuleARM64; } [Fact] @@ -476,5 +501,54 @@ public void TestAlignmentBehavior_AutoAlignmentRules(string wrapperType, int[] a Assert.Equal(alignment[2], tX86.GetField("fld2").Offset.AsInt); } + [Theory] + [InlineData("StructStructByte_Int128StructAuto", "ARM64", 16, 32)] + [InlineData("StructStructByte_Int128StructAuto", "ARM", 8, 24)] + [InlineData("StructStructByte_Int128StructAuto", "X86", 8, 24)] + [InlineData("StructStructByte_Int128StructAuto", "X64Linux", 16, 32)] + [InlineData("StructStructByte_Int128StructAuto", "X64Windows", 8, 24)] + [InlineData("StructStructByte_UInt128StructAuto", "ARM64", 16, 32)] + [InlineData("StructStructByte_UInt128StructAuto", "ARM", 8, 24)] + [InlineData("StructStructByte_UInt128StructAuto", "X86", 8, 24)] + [InlineData("StructStructByte_UInt128StructAuto", "X64Linux", 16, 32)] + [InlineData("StructStructByte_UInt128StructAuto", "X64Windows", 8, 24)] + // Variation of TestAlignmentBehavior_AutoAlignmentRules above that is able to deal with os specific behavior + public void TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence(string wrapperType, string osArch, int alignment, int size) + { + ModuleDesc testModule; + switch (osArch) + { + case "ARM64": + testModule = _testModuleARM64; + break; + case "ARM": + testModule = _testModuleARM; + break; + case "X64": + testModule = _testModuleX64; + break; + case "X64Linux": + testModule = _testModuleX64Linux; + break; + case "X64Windows": + testModule = _testModuleX64Windows; + break; + case "X86": + testModule = _testModuleX86; + break; + default: + throw new Exception(); + } + + string _namespace = "Sequential"; + string _type = wrapperType; + + MetadataType type = testModule.GetType(_namespace, _type); + + Assert.Equal(alignment, type.InstanceFieldAlignment.AsInt); + Assert.Equal(size, type.InstanceFieldSize.AsInt); + Assert.Equal(0x0, type.GetField("fld1").Offset.AsInt); + Assert.Equal(alignment, type.GetField("fld2").Offset.AsInt); + } } } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs index c360958375e010..92446b5770ef94 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/InstanceFieldLayout.cs @@ -212,6 +212,18 @@ public struct StructStructByte_Struct9BytesAuto public StructByte fld1; public Auto.Struct9Bytes fld2; } + + public struct StructStructByte_Int128StructAuto + { + public StructByte fld1; + public Auto.Int128Struct fld2; + } + + public struct StructStructByte_UInt128StructAuto + { + public StructByte fld1; + public Auto.UInt128Struct fld2; + } } namespace Auto @@ -417,6 +429,18 @@ public struct Struct9Bytes public byte fld8; public byte fld9; } + + [StructLayout(LayoutKind.Auto)] + public struct UInt128Struct + { + UInt128 fld1; + } + + [StructLayout(LayoutKind.Auto)] + public struct Int128Struct + { + Int128 fld1; + } } namespace IsByRefLike diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs index d9602770b92da5..6935dd75d92c93 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs @@ -73,6 +73,24 @@ public ref struct TypedReference private readonly ref byte _value; private readonly RuntimeTypeHandle _typeHandle; } + + [Intrinsic] + [StructLayout(LayoutKind.Sequential)] + public readonly struct Int128 + { + + private readonly ulong _lower; + private readonly ulong _upper; + } + + [Intrinsic] + [StructLayout(LayoutKind.Sequential)] + public readonly struct UInt128 + { + + private readonly ulong _lower; + private readonly ulong _upper; + } } namespace System.Collections diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj index 444ce230f3f834..1f6b33ff18ba39 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj @@ -47,6 +47,7 @@ + diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs index 4842df691dfa9c..056c8f32941aa6 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs @@ -24,6 +24,8 @@ class TestTypeSystemContext : MetadataTypeSystemContext Dictionary _modules = new Dictionary(StringComparer.OrdinalIgnoreCase); private VectorFieldLayoutAlgorithm _vectorFieldLayoutAlgorithm; + private Int128FieldLayoutAlgorithm _int128FieldLayoutAlgorithm; + MetadataFieldLayoutAlgorithm _metadataFieldLayout = new TestMetadataFieldLayoutAlgorithm(); MetadataRuntimeInterfacesAlgorithm _metadataRuntimeInterfacesAlgorithm = new MetadataRuntimeInterfacesAlgorithm(); ArrayOfTRuntimeInterfacesAlgorithm _arrayOfTRuntimeInterfacesAlgorithm; @@ -31,10 +33,11 @@ class TestTypeSystemContext : MetadataTypeSystemContext public CanonicalizationMode CanonMode { get; set; } = CanonicalizationMode.RuntimeDetermined; - public TestTypeSystemContext(TargetArchitecture arch) + public TestTypeSystemContext(TargetArchitecture arch, TargetOS targetOS = TargetOS.Unknown) : base(new TargetDetails(arch, TargetOS.Unknown, TargetAbi.Unknown)) { _vectorFieldLayoutAlgorithm = new VectorFieldLayoutAlgorithm(_metadataFieldLayout, true); + _int128FieldLayoutAlgorithm = new Int128FieldLayoutAlgorithm(_metadataFieldLayout); } public ModuleDesc GetModuleForSimpleName(string simpleName) @@ -74,6 +77,10 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) { return _vectorFieldLayoutAlgorithm; } + else if (Int128FieldLayoutAlgorithm.IsIntegerType(type)) + { + return _int128FieldLayoutAlgorithm; + } return _metadataFieldLayout; } diff --git a/src/coreclr/vm/callingconvention.h b/src/coreclr/vm/callingconvention.h index a206eecbf04e49..2d14247379da1c 100644 --- a/src/coreclr/vm/callingconvention.h +++ b/src/coreclr/vm/callingconvention.h @@ -1569,6 +1569,19 @@ int ArgIteratorTemplate::GetNextOffset() _ASSERTE((cbArg% TARGET_POINTER_SIZE) == 0); #endif const int regSlots = ALIGN_UP(cbArg, TARGET_POINTER_SIZE) / TARGET_POINTER_SIZE; + +#if !defined(OSX_ARM64_ABI) + // Implement rule C.10 from the ARM64 Procedure Calling standard. + // If the argument has an alignment of 16 then the NGRN is rounded up to the next even number. + // + // This rule is not used for Apple platforms. See https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms + if (!this->IsVarArg() && ((_arm64IdxGenReg & 1) == 1) && regSlots == 2 && thValueType.GetMethodTable()->GetFieldAlignmentRequirement() == 16) + { + m_idxGenReg++; + numRegistersUsed++; + } +#endif + // Only x0-x7 are valid argument registers (x8 is always the return buffer) if (m_idxGenReg + regSlots <= 8) { @@ -1606,8 +1619,8 @@ int ArgIteratorTemplate::GetNextOffset() } } + int alignment = 1; #ifdef OSX_ARM64_ABI - int alignment; if (!isValueType) { _ASSERTE((cbArg & (cbArg - 1)) == 0); @@ -1621,8 +1634,12 @@ int ArgIteratorTemplate::GetNextOffset() { alignment = 8; } - m_ofsStack = (int)ALIGN_UP(m_ofsStack, alignment); #endif // OSX_ARM64_ABI + if (!this->IsVarArg() && cbArg == 16 && thValueType.GetMethodTable()->GetFieldAlignmentRequirement() == 16) + { + alignment = 16; + } + m_ofsStack = (int)ALIGN_UP(m_ofsStack, alignment); int argOfs = TransitionBlock::GetOffsetOfArgs() + m_ofsStack; m_ofsStack += cbArg; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index ecd1e9d22916cb..d9fea5dddd1fce 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -9907,21 +9907,6 @@ void MethodTableBuilder::CheckForSystemTypes() return; } -#if defined(UNIX_AMD64_ABI) || defined(TARGET_ARM64) - else if (strcmp(nameSpace, g_SystemNS) == 0) - { - EEClassLayoutInfo* pLayout = pClass->GetLayoutInfo(); - - // These types correspond to fundamental data types in the underlying ABIs: - // * Int128: __int128 - // * UInt128: unsigned __int128 - - if ((strcmp(name, g_Int128Name) == 0) || (strcmp(name, g_UInt128Name) == 0)) - { - pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = 16; // sizeof(__int128) - } - } -#endif // UNIX_AMD64_ABI || TARGET_ARM64 } if (g_pNullableClass != NULL) @@ -10005,6 +9990,17 @@ void MethodTableBuilder::CheckForSystemTypes() { pMT->SetInternalCorElementType (ELEMENT_TYPE_I); } +#if defined(UNIX_AMD64_ABI) || defined(TARGET_ARM64) + else if ((strcmp(name, g_Int128Name) == 0) || (strcmp(name, g_UInt128Name) == 0)) + { + EEClassLayoutInfo* pLayout = pClass->GetLayoutInfo(); + + // These types correspond to fundamental data types in the underlying ABIs: + // * Int128: __int128 + // * UInt128: unsigned __int128 + pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = 16; // sizeof(__int128) + } +#endif // UNIX_AMD64_ABI || TARGET_ARM64 } else { diff --git a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs index 690f6ff9426d42..be0b1fcc1baa7d 100644 --- a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs +++ b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs @@ -193,7 +193,8 @@ class SequentialTest static Sequential.StructStructByte_Struct5BytesAuto _fld11; static Sequential.StructStructByte_Struct8BytesAuto _fld12; static Sequential.StructStructByte_Struct9BytesAuto _fld13; - + static Sequential.StructStructByte_Int128StructAuto _fld14; + static Sequential.StructStructByte_UInt128StructAuto _fld15; public static void Test() { _fld1.MyClass1SelfRef = _fld1; @@ -232,6 +233,8 @@ public static void Test() _fld11.fld2 = default(Auto.Struct5Bytes); _fld12.fld2 = default(Auto.Struct8Bytes); _fld13.fld2 = default(Auto.Struct9Bytes); + _fld14.fld2 = default(Auto.Int128Struct); + _fld15.fld2 = default(Auto.UInt128Struct); } } From d67513255e5d0b48c0d2170c7c917da6fe1a29b5 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 17 Aug 2022 11:47:01 -0700 Subject: [PATCH 9/9] Fix bugs so that at least Windows Arm64 works --- src/coreclr/jit/lclvars.cpp | 2 +- .../tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs | 4 ++-- .../ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs | 2 +- src/coreclr/vm/callingconvention.h | 3 +-- src/tests/Interop/PInvoke/Int128/Int128Native.cpp | 8 ++++++-- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 2c8597e99ae05b..271c4199d71c6c 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -6337,7 +6337,7 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, unsigned argAlignment = eeGetArgSizeAlignment(varDsc->lvType, isFloatHfa); #if TARGET_ARM64 - if (!info.compIsVarArgs && varDsc->lvSize() == 16 && varDsc->lvType == TYP_STRUCT && !isFloatHfa && info.compCompHnd->getClassAlignmentRequirement(varDsc->GetStructHnd()) == 16) + if (!info.compIsVarArgs && varDsc->lvType == TYP_STRUCT && varDsc->lvSize() == 16 && !isFloatHfa && info.compCompHnd->getClassAlignmentRequirement(varDsc->GetStructHnd()) == 16) { // TODO-Cleanup: use "eeGetArgSizeAlignment" here. See also: https://github.com/dotnet/runtime/issues/46026. argAlignment = 16; diff --git a/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs index 60be29fdce95c9..a78d540b7801cd 100644 --- a/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs @@ -28,7 +28,7 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp ComputedInstanceFieldLayout layoutFromMetadata = _fallbackAlgorithm.ComputeInstanceLayout(defType, layoutKind); - if (defType.Context.Target.IsWindows || (defType.Context.Target.PointerSize == 4)) + if (defType.Context.Target.Architecture != TargetArchitecture.ARM64 && defType.Context.Target.IsWindows || (defType.Context.Target.PointerSize == 4)) { return layoutFromMetadata; } @@ -72,7 +72,7 @@ public override ValueTypeShapeCharacteristics ComputeValueTypeShapeCharacteristi public static bool IsIntegerType(DefType type) { return type.IsIntrinsic - && type.Namespace == "System." + && type.Namespace == "System" && ((type.Name == "Int128") || (type.Name == "UInt128")); } } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs index 056c8f32941aa6..3f963c24104eb1 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs @@ -34,7 +34,7 @@ class TestTypeSystemContext : MetadataTypeSystemContext public CanonicalizationMode CanonMode { get; set; } = CanonicalizationMode.RuntimeDetermined; public TestTypeSystemContext(TargetArchitecture arch, TargetOS targetOS = TargetOS.Unknown) - : base(new TargetDetails(arch, TargetOS.Unknown, TargetAbi.Unknown)) + : base(new TargetDetails(arch, targetOS, TargetAbi.Unknown)) { _vectorFieldLayoutAlgorithm = new VectorFieldLayoutAlgorithm(_metadataFieldLayout, true); _int128FieldLayoutAlgorithm = new Int128FieldLayoutAlgorithm(_metadataFieldLayout); diff --git a/src/coreclr/vm/callingconvention.h b/src/coreclr/vm/callingconvention.h index 2d14247379da1c..a4e525740a479a 100644 --- a/src/coreclr/vm/callingconvention.h +++ b/src/coreclr/vm/callingconvention.h @@ -1575,10 +1575,9 @@ int ArgIteratorTemplate::GetNextOffset() // If the argument has an alignment of 16 then the NGRN is rounded up to the next even number. // // This rule is not used for Apple platforms. See https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms - if (!this->IsVarArg() && ((_arm64IdxGenReg & 1) == 1) && regSlots == 2 && thValueType.GetMethodTable()->GetFieldAlignmentRequirement() == 16) + if (!this->IsVarArg() && ((m_idxGenReg & 1) == 1) && regSlots == 2 && thValueType.GetMethodTable()->GetFieldAlignmentRequirement() == 16) { m_idxGenReg++; - numRegistersUsed++; } #endif diff --git a/src/tests/Interop/PInvoke/Int128/Int128Native.cpp b/src/tests/Interop/PInvoke/Int128/Int128Native.cpp index 0ce4a0923aa3fc..cae08e6b244f5c 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Native.cpp +++ b/src/tests/Interop/PInvoke/Int128/Int128Native.cpp @@ -11,10 +11,14 @@ #elif defined(__SIZEOF_INT128__) typedef __int128 Int128; #else - typedef struct { +struct +#ifdef _M_ARM64 +alignas(16) +#endif +Int128 { uint64_t lower; uint64_t upper; - } Int128; + }; #endif static Int128 Int128Value = { };