From 29ea8888a3e6d1e219b72bf2c68039c6162c63ac Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 16 Aug 2022 18:17:11 -0700 Subject: [PATCH 01/11] 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 ca40e03f70da019f0e39966e10ddbf4c70e6b375 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 8 Aug 2022 17:55:49 -0700 Subject: [PATCH 02/11] 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 183a36e5d3fc725e01415ae073d09ffd9c7d0f37 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 17 Aug 2022 11:47:01 -0700 Subject: [PATCH 03/11] 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 = { }; From 823090b81217589c22b110801a64d4bedc4b2445 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 17 Aug 2022 17:13:11 -0700 Subject: [PATCH 04/11] Add more types to the ABI tester, so that we cover the Int128 scenarios --- src/tests/JIT/Stress/ABI/ABIs.cs | 11 ++++++----- src/tests/JIT/Stress/ABI/Gen.cs | 5 +++++ src/tests/JIT/Stress/ABI/Program.cs | 2 ++ src/tests/JIT/Stress/ABI/Types.cs | 5 ++++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/tests/JIT/Stress/ABI/ABIs.cs b/src/tests/JIT/Stress/ABI/ABIs.cs index a9c85f864aa88b..3ad1eed8d7c743 100644 --- a/src/tests/JIT/Stress/ABI/ABIs.cs +++ b/src/tests/JIT/Stress/ABI/ABIs.cs @@ -47,14 +47,14 @@ internal class Win86Abi : IAbi new[] { typeof(byte), typeof(short), typeof(int), typeof(long), - typeof(float), typeof(double), + typeof(float), typeof(double), typeof(Int128), typeof(Vector), typeof(Vector128), typeof(Vector256), typeof(S1P), typeof(S2P), typeof(S2U), typeof(S3U), typeof(S4P), typeof(S4U), typeof(S5U), typeof(S6U), typeof(S7U), typeof(S8P), typeof(S8U), typeof(S9U), typeof(S10U), typeof(S11U), typeof(S12U), typeof(S13U), typeof(S14U), typeof(S15U), typeof(S16U), typeof(S17U), - typeof(S31U), typeof(S32U), + typeof(S31U), typeof(S32U), typeof(I128_1), typeof(I128_2) }; public CallingConvention[] PInvokeConventions { get; } = { CallingConvention.Cdecl, CallingConvention.StdCall, }; @@ -107,12 +107,13 @@ internal class SysVAbi : IAbi typeof(byte), typeof(short), typeof(int), typeof(long), typeof(float), typeof(double), typeof(Vector), typeof(Vector128), typeof(Vector256), + typeof(Int128), typeof(S1P), typeof(S2P), typeof(S2U), typeof(S3U), typeof(S4P), typeof(S4U), typeof(S5U), typeof(S6U), typeof(S7U), typeof(S8P), typeof(S8U), typeof(S9U), typeof(S10U), typeof(S11U), typeof(S12U), typeof(S13U), typeof(S14U), typeof(S15U), typeof(S16U), typeof(S17U), - typeof(S31U), typeof(S32U), + typeof(S31U), typeof(S32U), typeof(I128_1), typeof(I128_2) }; public CallingConvention[] PInvokeConventions { get; } = { CallingConvention.Cdecl }; @@ -135,14 +136,14 @@ internal class Arm64Abi : IAbi new[] { typeof(byte), typeof(short), typeof(int), typeof(long), - typeof(float), typeof(double), + typeof(float), typeof(double), typeof(Int128), typeof(Vector), typeof(Vector128), typeof(Vector256), typeof(S1P), typeof(S2P), typeof(S2U), typeof(S3U), typeof(S4P), typeof(S4U), typeof(S5U), typeof(S6U), typeof(S7U), typeof(S8P), typeof(S8U), typeof(S9U), typeof(S10U), typeof(S11U), typeof(S12U), typeof(S13U), typeof(S14U), typeof(S15U), typeof(S16U), - typeof(Hfa1), typeof(Hfa2), + typeof(Hfa1), typeof(Hfa2), typeof(I128_1) }; public CallingConvention[] PInvokeConventions { get; } = { CallingConvention.Cdecl }; diff --git a/src/tests/JIT/Stress/ABI/Gen.cs b/src/tests/JIT/Stress/ABI/Gen.cs index a7d0d0efbe5435..c4cc8396994b68 100644 --- a/src/tests/JIT/Stress/ABI/Gen.cs +++ b/src/tests/JIT/Stress/ABI/Gen.cs @@ -48,6 +48,9 @@ internal static object GenConstant(Type type, FieldInfo[] fields, Random rand) if (type == typeof(double)) return (double)rand.Next(); + if (type == typeof(Int128)) + return new Int128((ulong)GenConstant(typeof(ulong), null, rand), (ulong)GenConstant(typeof(ulong), null, rand)); + if (type == typeof(Vector)) return GenConstantVector, int>(rand); @@ -173,6 +176,8 @@ internal static void EmitLoadPrimitive(ILGenerator il, object val) il.Emit(OpCodes.Ldc_R4, (float)val); else if (ty == typeof(double)) il.Emit(OpCodes.Ldc_R8, (double)val); + else if (ty == typeof(Int128)) + EmitLoadBlittable(il, (Int128)val); else if (ty == typeof(Vector)) EmitLoadBlittable(il, (Vector)val); else if (ty == typeof(Vector128)) diff --git a/src/tests/JIT/Stress/ABI/Program.cs b/src/tests/JIT/Stress/ABI/Program.cs index 8eab812a5748c3..1c1c00c648309b 100644 --- a/src/tests/JIT/Stress/ABI/Program.cs +++ b/src/tests/JIT/Stress/ABI/Program.cs @@ -344,6 +344,7 @@ public static void EmitDumpValues(string listName, ILGenerator g, IEnumerable), typeof(Vector128), typeof(Vector256), + typeof(Int128), typeof(S1P), typeof(S2P), typeof(S2U), typeof(S3U), typeof(S4P), typeof(S4U), typeof(S5U), typeof(S6U), typeof(S7U), typeof(S8P), typeof(S8U), typeof(S9U), @@ -351,6 +352,7 @@ public static void EmitDumpValues(string listName, ILGenerator g, IEnumerable new TypeEx(t)).ToArray(); private static readonly IAbi s_abi = SelectAbi(); diff --git a/src/tests/JIT/Stress/ABI/Types.cs b/src/tests/JIT/Stress/ABI/Types.cs index a24c47154de6d2..140ae76762b257 100644 --- a/src/tests/JIT/Stress/ABI/Types.cs +++ b/src/tests/JIT/Stress/ABI/Types.cs @@ -61,6 +61,8 @@ public struct S31U { public byte F0, F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F1 public struct S32U { public byte F0, F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12, F13, F14, F15, F16, F17, F18, F19, F20, F21, F22, F23, F24, F25, F26, F27, F28, F29, F30, F31; public S32U(byte f0, byte f1, byte f2, byte f3, byte f4, byte f5, byte f6, byte f7, byte f8, byte f9, byte f10, byte f11, byte f12, byte f13, byte f14, byte f15, byte f16, byte f17, byte f18, byte f19, byte f20, byte f21, byte f22, byte f23, byte f24, byte f25, byte f26, byte f27, byte f28, byte f29, byte f30, byte f31) => (F0, F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12, F13, F14, F15, F16, F17, F18, F19, F20, F21, F22, F23, F24, F25, F26, F27, F28, F29, F30, F31) = (f0, f1, f2, f3, f4, f5, f6, f7, f8, f9, f10, f11, f12, f13, f14, f15, f16, f17, f18, f19, f20, f21, f22, f23, f24, f25, f26, f27, f28, f29, f30, f31); } public struct Hfa1 { public float F0, F1; public Hfa1(float f0, float f1) => (F0, F1) = (f0, f1); } public struct Hfa2 { public double F0, F1, F2, F3; public Hfa2(double f0, double f1, double f2, double f3) => (F0, F1, F2, F3) = (f0, f1, f2, f3); } + public struct I128_1 { public Int128 F0; public I128_1(Int128 f0) => F0 = f0; } + public struct I128_2 { public byte F0; public Int128 F1; public I128_2(byte f0, Int128 f1) => (F0, F1) = (f0, f1); } internal static class TypeExtensions { @@ -78,7 +80,8 @@ public static bool IsOurStructType(this Type t) t == typeof(S14U) || t == typeof(S15U) || t == typeof(S16U) || t == typeof(S17U) || t == typeof(S31U) || t == typeof(S32U) || - t == typeof(Hfa1) || t == typeof(Hfa2); + t == typeof(Hfa1) || t == typeof(Hfa2) || + t == typeof(I128_1) || t == typeof(I128_2); } } } From 22adce81387ccdcda0dd091433dc187c629e24bd Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 17 Aug 2022 18:47:12 -0700 Subject: [PATCH 05/11] Revert changes which attempted to enable by value passing for Int128 --- 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 ++---------- src/coreclr/vm/callingconvention.h | 20 ++--------- 6 files changed, 11 insertions(+), 93 deletions(-) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 271c4199d71c6c..4aa1b1e4d9865d 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -717,13 +717,6 @@ 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()) { @@ -1235,21 +1228,11 @@ 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), 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)); + // single stack slot). + assert(compMacOsArm64Abi() || ((varDscInfo->stackArgSize % argAlignment) == 0)); #endif varDscInfo->stackArgSize = roundUp(varDscInfo->stackArgSize, argAlignment); @@ -6334,19 +6317,11 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, } #endif // TARGET_ARM const bool isFloatHfa = (varDsc->lvIsHfa() && (varDsc->GetHfaType() == TYP_FLOAT)); - unsigned argAlignment = eeGetArgSizeAlignment(varDsc->lvType, isFloatHfa); - -#if TARGET_ARM64 - 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; - } - if (compMacOsArm64Abi() || (argAlignment == 16)) + const unsigned argAlignment = eeGetArgSizeAlignment(varDsc->lvType, isFloatHfa); + if (compMacOsArm64Abi()) { 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 1ab32540a8282b..fff00343e888ab 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2510,28 +2510,6 @@ 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 c8e414d8822a43..a90e61c3a59fde 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); } -#if defined(TARGET_ARM) || defined(TARGET_ARM64) +#ifdef TARGET_ARM unsigned InitVarDscInfo::alignReg(var_types type, unsigned requiredRegAlignment) { assert(requiredRegAlignment > 0); @@ -91,12 +91,10 @@ 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 @@ -104,7 +102,7 @@ unsigned InitVarDscInfo::alignReg(var_types type, unsigned requiredRegAlignment) return cAlignSkipped; } -#endif // TARGET_ARM || TARGET_ARM64 +#endif // TARGET_ARM 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 5537d71c847ede..0085e8ac80a2ea 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); -#if defined(TARGET_ARM) || defined(TARGET_ARM64) +#ifdef TARGET_ARM // 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 efd5b866290b57..73e0a1d4087d75 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,16 +1257,6 @@ 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) { @@ -1298,9 +1288,9 @@ public int GetNextOffset() } } - int alignment = 1; if (_transitionBlock.IsAppleArm64ABI) { + int alignment; if (!isValueType) { Debug.Assert((cbArg & (cbArg - 1)) == 0); @@ -1314,16 +1304,9 @@ 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/vm/callingconvention.h b/src/coreclr/vm/callingconvention.h index a4e525740a479a..a206eecbf04e49 100644 --- a/src/coreclr/vm/callingconvention.h +++ b/src/coreclr/vm/callingconvention.h @@ -1569,18 +1569,6 @@ 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() && ((m_idxGenReg & 1) == 1) && regSlots == 2 && thValueType.GetMethodTable()->GetFieldAlignmentRequirement() == 16) - { - m_idxGenReg++; - } -#endif - // Only x0-x7 are valid argument registers (x8 is always the return buffer) if (m_idxGenReg + regSlots <= 8) { @@ -1618,8 +1606,8 @@ int ArgIteratorTemplate::GetNextOffset() } } - int alignment = 1; #ifdef OSX_ARM64_ABI + int alignment; if (!isValueType) { _ASSERTE((cbArg & (cbArg - 1)) == 0); @@ -1633,12 +1621,8 @@ int ArgIteratorTemplate::GetNextOffset() { alignment = 8; } -#endif // OSX_ARM64_ABI - if (!this->IsVarArg() && cbArg == 16 && thValueType.GetMethodTable()->GetFieldAlignmentRequirement() == 16) - { - alignment = 16; - } m_ofsStack = (int)ALIGN_UP(m_ofsStack, alignment); +#endif // OSX_ARM64_ABI int argOfs = TransitionBlock::GetOffsetOfArgs() + m_ofsStack; m_ofsStack += cbArg; From e36292f22bc4e2a40d755acd91be0ad13342bac2 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 17 Aug 2022 18:50:54 -0700 Subject: [PATCH 06/11] Make Int128 have layout match the expected unmanaged field layout - On Unix platforms (64 bit) use 16 byte alignment - On Arm32 use 8 byte alignment matching the 128 byte vector type - On other Windows platforms the 128 bit integer type isn't defined by the C compiler, but match the behavior of other 128 bit types (16 byte alignment) Add tests to call down to native that should pass with these rules - Disable use of Int128 as p/invoke parameter passed by value --- src/coreclr/dlls/mscorrc/mscorrc.rc | 1 + src/coreclr/dlls/mscorrc/resource.h | 1 + .../Compiler/Int128FieldLayoutAlgorithm.cs | 3 +- .../TypeSystem/Interop/IL/MarshalHelpers.cs | 6 +++ .../Common/TypeSystem/Interop/InteropTypes.cs | 5 ++ .../ArchitectureSpecificFieldLayoutTests.cs | 8 +-- src/coreclr/vm/methodtablebuilder.cpp | 16 +++++- src/coreclr/vm/mlinfo.cpp | 15 ++++++ .../Interop/PInvoke/Int128/Int128Native.cpp | 24 ++++++++- .../Interop/PInvoke/Int128/Int128Test.cs | 18 ++++++- .../Interop/PInvoke/Int128/Int128Test.csproj | 4 +- .../Int128/Int128TestFieldLayout.csproj | 15 ++++++ .../PInvoke/Int128/ProgramFieldLayout.cs | 23 +++++++++ .../PInvoke/Int128/UInt128TestFieldLayout.cs | 51 +++++++++++++++++++ .../fieldlayout/fieldlayouttests.cs | 1 + 15 files changed, 179 insertions(+), 12 deletions(-) create mode 100644 src/tests/Interop/PInvoke/Int128/Int128TestFieldLayout.csproj create mode 100644 src/tests/Interop/PInvoke/Int128/ProgramFieldLayout.cs create mode 100644 src/tests/Interop/PInvoke/Int128/UInt128TestFieldLayout.cs diff --git a/src/coreclr/dlls/mscorrc/mscorrc.rc b/src/coreclr/dlls/mscorrc/mscorrc.rc index 892c1f1c27bfcb..e91bb09807bbdb 100644 --- a/src/coreclr/dlls/mscorrc/mscorrc.rc +++ b/src/coreclr/dlls/mscorrc/mscorrc.rc @@ -271,6 +271,7 @@ BEGIN IDS_EE_BADMARSHAL_ABSTRACTRETCRITICALHANDLE "Returned CriticalHandles cannot be abstract." IDS_EE_BADMARSHAL_CUSTOMMARSHALER "Custom marshalers are only allowed on classes, strings, arrays, and boxed value types." IDS_EE_BADMARSHAL_GENERICS_RESTRICTION "Non-blittable generic types cannot be marshaled." + IDS_EE_BADMARSHAL_INT128_RESTRICTION "System.Int128 and System.UInt128 cannot be passed by value to unmanaged." IDS_EE_BADMARSHAL_AUTOLAYOUT "Structures marked with [StructLayout(LayoutKind.Auto)] cannot be marshaled." IDS_EE_BADMARSHAL_STRING_OUT "Cannot marshal a string by-value with the [Out] attribute." IDS_EE_BADMARSHAL_MARSHAL_DISABLED "Cannot marshal managed types when the runtime marshalling system is disabled." diff --git a/src/coreclr/dlls/mscorrc/resource.h b/src/coreclr/dlls/mscorrc/resource.h index d2177bd56db2fd..3245329339c91f 100644 --- a/src/coreclr/dlls/mscorrc/resource.h +++ b/src/coreclr/dlls/mscorrc/resource.h @@ -283,6 +283,7 @@ #define IDS_EE_BADMARSHAL_ABSTRACTOUTCRITICALHANDLE 0x1a63 #define IDS_EE_BADMARSHAL_RETURNCHCOMTONATIVE 0x1a64 #define IDS_EE_BADMARSHAL_CRITICALHANDLE 0x1a65 +#define IDS_EE_BADMARSHAL_INT128_RESTRICTION 0x1a66 #define IDS_EE_BADMARSHAL_ABSTRACTRETCRITICALHANDLE 0x1a6a #define IDS_EE_CH_IN_VARIANT_NOT_SUPPORTED 0x1a6b diff --git a/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs index a78d540b7801cd..f32840367b2826 100644 --- a/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs @@ -28,7 +28,8 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp ComputedInstanceFieldLayout layoutFromMetadata = _fallbackAlgorithm.ComputeInstanceLayout(defType, layoutKind); - if (defType.Context.Target.Architecture != TargetArchitecture.ARM64 && defType.Context.Target.IsWindows || (defType.Context.Target.PointerSize == 4)) + // 32bit platforms use standard metadata layout engine + if (defType.Context.Target.Architecture == TargetArchitecture.ARM) { return layoutFromMetadata; } diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs index 658302d0faaf42..b3b8bad1faa7a7 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs @@ -434,6 +434,12 @@ internal static MarshallerKind GetMarshallerKind( return MarshallerKind.Invalid; } + if (!isField && InteropTypes.IsInt128Type(context, type)) + { + // Int128 types cannot be passed by value + return MarshallerKind.Invalid; + } + if (isBlittable) { if (nativeType != NativeTypeKind.Default && nativeType != NativeTypeKind.Struct) diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/InteropTypes.cs b/src/coreclr/tools/Common/TypeSystem/Interop/InteropTypes.cs index f69879a10ee15f..e6a684ba82e10f 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/InteropTypes.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/InteropTypes.cs @@ -137,6 +137,11 @@ public static bool IsSystemRuntimeIntrinsicsVector64T(TypeSystemContext context, return IsCoreNamedType(context, type, "System.Runtime.Intrinsics", "Vector64`1"); } + public static bool IsInt128Type(TypeSystemContext context, TypeDesc type) + { + return IsCoreNamedType(context, type, "System", "Int128") || IsCoreNamedType(context, type, "System", "UInt128"); + } + public static bool IsSystemRuntimeIntrinsicsVector128T(TypeSystemContext context, TypeDesc type) { return IsCoreNamedType(context, type, "System.Runtime.Intrinsics", "Vector128`1"); diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs index 7b3be7676c03b0..f9fe3b7687439d 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs @@ -504,14 +504,14 @@ public void TestAlignmentBehavior_AutoAlignmentRules(string wrapperType, int[] a [Theory] [InlineData("StructStructByte_Int128StructAuto", "ARM64", 16, 32)] [InlineData("StructStructByte_Int128StructAuto", "ARM", 8, 24)] - [InlineData("StructStructByte_Int128StructAuto", "X86", 8, 24)] + [InlineData("StructStructByte_Int128StructAuto", "X86", 16, 32)] [InlineData("StructStructByte_Int128StructAuto", "X64Linux", 16, 32)] - [InlineData("StructStructByte_Int128StructAuto", "X64Windows", 8, 24)] + [InlineData("StructStructByte_Int128StructAuto", "X64Windows", 16, 32)] [InlineData("StructStructByte_UInt128StructAuto", "ARM64", 16, 32)] [InlineData("StructStructByte_UInt128StructAuto", "ARM", 8, 24)] - [InlineData("StructStructByte_UInt128StructAuto", "X86", 8, 24)] + [InlineData("StructStructByte_UInt128StructAuto", "X86", 16, 32)] [InlineData("StructStructByte_UInt128StructAuto", "X64Linux", 16, 32)] - [InlineData("StructStructByte_UInt128StructAuto", "X64Windows", 8, 24)] + [InlineData("StructStructByte_UInt128StructAuto", "X64Windows", 16, 32)] // 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) { diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index d9fea5dddd1fce..e6a6c8ff993107 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -9990,17 +9990,29 @@ 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(); +#ifdef TARGET_ARM + // No such type exists for the Procedure Call Standard for ARM. We will default + // to the same alignment as __m128, which is supported by the ABI. + + pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = 8; +#elif defined(TARGET_64BIT) || defined(TARGET_X86) // These types correspond to fundamental data types in the underlying ABIs: // * Int128: __int128 // * UInt128: unsigned __int128 + // + // This behavior matches the ABI standard on various Unix platforms + // On Windows, no standard for Int128 has been established yet, + // although applying 16 byte alignment is consistent with treatment of 128 bit SSE types + // even on X86 pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = 16; // sizeof(__int128) +#else +#error Unknown architecture +#endif // TARGET_64BIT } -#endif // UNIX_AMD64_ABI || TARGET_ARM64 } else { diff --git a/src/coreclr/vm/mlinfo.cpp b/src/coreclr/vm/mlinfo.cpp index 2665a46d12f0ab..9af29919ca8c79 100644 --- a/src/coreclr/vm/mlinfo.cpp +++ b/src/coreclr/vm/mlinfo.cpp @@ -2283,6 +2283,21 @@ MarshalInfo::MarshalInfo(Module* pModule, IfFailGoto(E_FAIL, lFail); } + // * Int128: Represents the 128 bit integer ABI primitive type which requires currently unimplemented handling + // * UInt128: Represents the 128 bit integer ABI primitive type which requires currently unimplemented handling + // The field layout is correct, so field scenarios work, but these should not be passed by value as parameters + // + // TODO: Contact Jeremy to figure out how to prohibit these types from being used as fields of parameters passed by value + if (!IsFieldScenario()) + { + if (m_pMT == CoreLibBinder::GetClass(CLASS__INT128) + || m_pMT == CoreLibBinder::GetClass(CLASS__UINT128)) + { + m_resID = IDS_EE_BADMARSHAL_INT128_RESTRICTION; + IfFailGoto(E_FAIL, lFail); + } + } + if (!m_pMT->HasLayout()) { m_resID = IDS_EE_BADMARSHAL_AUTOLAYOUT; diff --git a/src/tests/Interop/PInvoke/Int128/Int128Native.cpp b/src/tests/Interop/PInvoke/Int128/Int128Native.cpp index cae08e6b244f5c..39cab1cb81e70f 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Native.cpp +++ b/src/tests/Interop/PInvoke/Int128/Int128Native.cpp @@ -12,7 +12,7 @@ typedef __int128 Int128; #else struct -#ifdef _M_ARM64 +#if defined(_M_ARM64) || defined(_M_AMD64) || defined(_M_IX86) alignas(16) #endif Int128 { @@ -25,7 +25,7 @@ static Int128 Int128Value = { }; struct StructWithInt128 { - int64_t messUpPadding; + int8_t messUpPadding; Int128 value; }; @@ -72,6 +72,26 @@ extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE AddInt128(Int128 lhs, Int128 rhs) return result; } +// Test that struct alignment behavior matches with the standard OS compiler +extern "C" DLL_EXPORT void STDMETHODCALLTYPE AddStructWithInt128_ByRef(StructWithInt128 *pLhs, StructWithInt128 *pRhs) +{ + StructWithInt128 result = {}; + StructWithInt128 lhs = *pLhs; + StructWithInt128 rhs = *pRhs; + + 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 + + *pLhs = result; +} + extern "C" DLL_EXPORT StructWithInt128 STDMETHODCALLTYPE AddStructWithInt128(StructWithInt128 lhs, StructWithInt128 rhs) { StructWithInt128 result = {}; diff --git a/src/tests/Interop/PInvoke/Int128/Int128Test.cs b/src/tests/Interop/PInvoke/Int128/Int128Test.cs index 95de72ba36d53f..e3ce25cdf68bd5 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Test.cs +++ b/src/tests/Interop/PInvoke/Int128/Int128Test.cs @@ -7,8 +7,8 @@ struct StructWithInt128 { - public StructWithInt128(Int128 val) { value = val; messUpPadding = 0x101010; } - public long messUpPadding; + public StructWithInt128(Int128 val) { value = val; messUpPadding = 0x10; } + public byte messUpPadding; public Int128 value; }; @@ -32,6 +32,10 @@ unsafe partial class Int128Native [DllImport(nameof(Int128Native))] public static extern Int128 AddInt128(Int128 lhs, Int128 rhs); + + [DllImport(nameof(Int128Native))] + public static extern void AddStructWithInt128_ByRef(ref StructWithInt128 lhs, ref StructWithInt128 rhs); + [DllImport(nameof(Int128Native))] public static extern StructWithInt128 AddStructWithInt128(StructWithInt128 lhs, StructWithInt128 rhs); @@ -81,6 +85,16 @@ unsafe partial class Int128Native unsafe partial class Int128Native { + public static void TestUInt128FieldLayout() + { + // This test checks that the alignment rules of Int128 structs match the native compiler + StructWithInt128 lhs = new StructWithInt128(new Int128(11, 12)); + StructWithInt128 rhs = new StructWithInt128(new Int128(13, 14)); + + Int128Native.AddInt128(ref lhs, ref rhs); + Assert.Equal(new StructWithInt128(new Int128(24, 26)), lhs); + } + private static void TestInt128() { Int128 value1 = Int128Native.GetInt128(1, 2); diff --git a/src/tests/Interop/PInvoke/Int128/Int128Test.csproj b/src/tests/Interop/PInvoke/Int128/Int128Test.csproj index 42fc09c10b7185..e13c49fe94c3f7 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Test.csproj +++ b/src/tests/Interop/PInvoke/Int128/Int128Test.csproj @@ -6,7 +6,9 @@ exe - + + + diff --git a/src/tests/Interop/PInvoke/Int128/Int128TestFieldLayout.csproj b/src/tests/Interop/PInvoke/Int128/Int128TestFieldLayout.csproj new file mode 100644 index 00000000000000..449dc00c211eff --- /dev/null +++ b/src/tests/Interop/PInvoke/Int128/Int128TestFieldLayout.csproj @@ -0,0 +1,15 @@ + + + + true + embedded + exe + + + + + + + + + diff --git a/src/tests/Interop/PInvoke/Int128/ProgramFieldLayout.cs b/src/tests/Interop/PInvoke/Int128/ProgramFieldLayout.cs new file mode 100644 index 00000000000000..5a31288c5d3266 --- /dev/null +++ b/src/tests/Interop/PInvoke/Int128/ProgramFieldLayout.cs @@ -0,0 +1,23 @@ +// 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; + +unsafe partial class Int128NativeFieldLayout +{ + public static int Main(string[] args) + { + try + { + Console.WriteLine("Testing Int128"); + Int128Native.TestInt128FieldLayout(); + } + catch (System.Exception ex) + { + Console.WriteLine(ex); + return 0; + } + return 100; + } +} diff --git a/src/tests/Interop/PInvoke/Int128/UInt128TestFieldLayout.cs b/src/tests/Interop/PInvoke/Int128/UInt128TestFieldLayout.cs new file mode 100644 index 00000000000000..567469b5e2e17b --- /dev/null +++ b/src/tests/Interop/PInvoke/Int128/UInt128TestFieldLayout.cs @@ -0,0 +1,51 @@ +// 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 Xunit; + +unsafe partial class Int128Native +{ + private static void TestUInt128() + { + UInt128 value1 = Int128Native.GetUInt128(1, 2); + Assert.Equal(new UInt128(1, 2), value1); + + UInt128 value2; + Int128Native.GetUInt128Out(3, 4, &value2); + Assert.Equal(new UInt128(3, 4), value2); + + Int128Native.GetUInt128Out(5, 6, out UInt128 value3); + Assert.Equal(new UInt128(5, 6), value3); + + UInt128* value4 = Int128Native.GetUInt128Ptr(7, 8); + Assert.Equal(new UInt128(7, 8), *value4); + + ref readonly UInt128 value5 = ref Int128Native.GetUInt128Ref(9, 10); + Assert.Equal(new UInt128(9, 10), value5); + + UInt128 value6 = Int128Native.AddUInt128(new UInt128(11, 12), new UInt128(13, 14)); + Assert.Equal(new UInt128(24, 26), value6); + + UInt128[] values = new UInt128[] { + new UInt128(15, 16), + new UInt128(17, 18), + new UInt128(19, 20), + new UInt128(21, 22), + new UInt128(23, 24), + }; + + fixed (UInt128* pValues = &values[0]) + { + UInt128 value7 = Int128Native.AddUInt128s(pValues, values.Length); + Assert.Equal(new UInt128(95, 100), value7); + } + + UInt128 value8 = Int128Native.AddUInt128s(values, values.Length); + Assert.Equal(new UInt128(95, 100), value8); + + UInt128 value9 = Int128Native.AddUInt128s(in values[0], values.Length); + Assert.Equal(new UInt128(95, 100), value9); + } +} diff --git a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs index be0b1fcc1baa7d..9b421859653648 100644 --- a/src/tests/readytorun/fieldlayout/fieldlayouttests.cs +++ b/src/tests/readytorun/fieldlayout/fieldlayouttests.cs @@ -195,6 +195,7 @@ class SequentialTest static Sequential.StructStructByte_Struct9BytesAuto _fld13; static Sequential.StructStructByte_Int128StructAuto _fld14; static Sequential.StructStructByte_UInt128StructAuto _fld15; + public static void Test() { _fld1.MyClass1SelfRef = _fld1; From 867ab2eaca64a4f1937e6099482d2e99558de433 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 17 Aug 2022 19:39:38 -0700 Subject: [PATCH 07/11] Mark Int128 types as not having a stable abi - This disables use of these types for parameter passing in R2R images --- .../tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs index f32840367b2826..058c7d5a7d42bd 100644 --- a/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs @@ -31,6 +31,7 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp // 32bit platforms use standard metadata layout engine if (defType.Context.Target.Architecture == TargetArchitecture.ARM) { + layoutFromMetadata.LayoutAbiStable = false; // Int128 parameter passing ABI is unstable at this time return layoutFromMetadata; } @@ -43,7 +44,7 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp FieldAlignment = new LayoutInt(16), FieldSize = layoutFromMetadata.FieldSize, Offsets = layoutFromMetadata.Offsets, - LayoutAbiStable = true + LayoutAbiStable = false // Int128 parameter passing ABI is unstable at this time }; } From 8585002f2221f9232f3d384ffddd6fb22e9ecdbc Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 18 Aug 2022 18:46:09 -0700 Subject: [PATCH 08/11] Address all known issues --- src/coreclr/inc/readytorun.h | 7 +-- .../Compiler/Int128FieldLayoutAlgorithm.cs | 4 +- .../Common/Internal/Runtime/ModuleHeaders.cs | 4 +- .../TypeSystem/Common/DefType.FieldLayout.cs | 23 +++++++++ .../TypeSystem/Common/FieldLayoutAlgorithm.cs | 1 + .../Common/MetadataFieldLayoutAlgorithm.cs | 31 ++++++++--- .../TypeSystem/Interop/IL/MarshalHelpers.cs | 2 +- src/coreclr/vm/class.h | 29 ++++++++++- src/coreclr/vm/classlayoutinfo.cpp | 27 +++++++++- src/coreclr/vm/dllimport.cpp | 6 +++ src/coreclr/vm/methodtable.h | 3 ++ src/coreclr/vm/methodtable.inl | 7 +++ src/coreclr/vm/methodtablebuilder.cpp | 1 + src/coreclr/vm/mlinfo.cpp | 12 +++-- .../DisabledRuntimeMarshallingNative.cs | 3 ++ .../PInvokes.cs | 7 +++ .../Interop/PInvoke/Int128/Int128Native.cpp | 23 +++++++++ .../Interop/PInvoke/Int128/Int128Test.cs | 49 ++++++++++++++---- .../PInvoke/Int128/UInt128TestFieldLayout.cs | 51 ------------------- src/tests/issues.targets | 6 +++ 20 files changed, 213 insertions(+), 83 deletions(-) delete mode 100644 src/tests/Interop/PInvoke/Int128/UInt128TestFieldLayout.cs diff --git a/src/coreclr/inc/readytorun.h b/src/coreclr/inc/readytorun.h index 0934f2ea627481..20a1462125534c 100644 --- a/src/coreclr/inc/readytorun.h +++ b/src/coreclr/inc/readytorun.h @@ -15,10 +15,10 @@ #define READYTORUN_SIGNATURE 0x00525452 // 'RTR' // Keep these in sync with src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs -#define READYTORUN_MAJOR_VERSION 0x0007 -#define READYTORUN_MINOR_VERSION 0x0001 +#define READYTORUN_MAJOR_VERSION 0x0008 +#define READYTORUN_MINOR_VERSION 0x0000 -#define MINIMUM_READYTORUN_MAJOR_VERSION 0x006 +#define MINIMUM_READYTORUN_MAJOR_VERSION 0x008 // R2R Version 2.1 adds the InliningInfo section // R2R Version 2.2 adds the ProfileDataInfo section @@ -26,6 +26,7 @@ // R2R 3.0 is not backward compatible with 2.x. // R2R Version 6.0 changes managed layout for sequential types with any unmanaged non-blittable fields. // R2R 6.0 is not backward compatible with 5.x or earlier. +// R2R Version 8.0 Changes the alignment of the Int128 type struct READYTORUN_CORE_HEADER { diff --git a/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs index 058c7d5a7d42bd..18f794a6f354e8 100644 --- a/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs @@ -32,6 +32,7 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp if (defType.Context.Target.Architecture == TargetArchitecture.ARM) { layoutFromMetadata.LayoutAbiStable = false; // Int128 parameter passing ABI is unstable at this time + layoutFromMetadata.IsInt128OrHasInt128Fields = true; return layoutFromMetadata; } @@ -44,7 +45,8 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp FieldAlignment = new LayoutInt(16), FieldSize = layoutFromMetadata.FieldSize, Offsets = layoutFromMetadata.Offsets, - LayoutAbiStable = false // Int128 parameter passing ABI is unstable at this time + LayoutAbiStable = false, // Int128 parameter passing ABI is unstable at this time + IsInt128OrHasInt128Fields = true }; } diff --git a/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs b/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs index c5fdfda3d0f58f..cc286f81b03619 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs @@ -14,8 +14,8 @@ internal struct ReadyToRunHeaderConstants { public const uint Signature = 0x00525452; // 'RTR' - public const ushort CurrentMajorVersion = 7; - public const ushort CurrentMinorVersion = 1; + public const ushort CurrentMajorVersion = 8; + public const ushort CurrentMinorVersion = 0; } #pragma warning disable 0169 diff --git a/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs b/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs index 2190f9ac1a2785..4c3555c198af66 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs @@ -68,6 +68,11 @@ private static class FieldLayoutFlags /// True if the type transitively has any types with LayoutKind.Auto in its layout. /// public const int IsAutoLayoutOrHasAutoLayoutFields = 0x400; + + /// + /// True if the type transitively has an Int128 in it or is an Int128 + /// + public const int IsInt128OrHasInt128Fields = 0x800; } private class StaticBlockInfo @@ -135,6 +140,20 @@ public virtual bool IsAutoLayoutOrHasAutoLayoutFields } } + /// + /// Is a type Int128 or transitively have any fields of a type Int128. + /// + public virtual bool IsInt128OrHasInt128Fields + { + get + { + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedInstanceTypeLayout)) + { + ComputeInstanceLayout(InstanceLayoutKind.TypeAndFields); + } + return _fieldLayoutFlags.HasFlags(FieldLayoutFlags.IsInt128OrHasInt128Fields); + } + } /// /// The number of bytes required to hold a field of this type @@ -430,6 +449,10 @@ public void ComputeInstanceLayout(InstanceLayoutKind layoutKind) { _fieldLayoutFlags.AddFlags(FieldLayoutFlags.IsAutoLayoutOrHasAutoLayoutFields); } + if (computedLayout.IsInt128OrHasInt128Fields) + { + _fieldLayoutFlags.AddFlags(FieldLayoutFlags.IsInt128OrHasInt128Fields); + } if (computedLayout.Offsets != null) { diff --git a/src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutAlgorithm.cs index a19ec4b3603bf4..53388c915b85d8 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutAlgorithm.cs @@ -83,6 +83,7 @@ public struct ComputedInstanceFieldLayout public LayoutInt ByteCountAlignment; public bool LayoutAbiStable; // Is the layout stable such that it can safely be used in function calling conventions public bool IsAutoLayoutOrHasAutoLayoutFields; + public bool IsInt128OrHasInt128Fields; /// /// If Offsets is non-null, then all field based layout is complete. diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 70b529a60601ce..9d291d8c372d69 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -110,6 +110,7 @@ out instanceByteSizeAndAlignment FieldSize = sizeAndAlignment.Size, LayoutAbiStable = true, IsAutoLayoutOrHasAutoLayoutFields = false, + IsInt128OrHasInt128Fields = false, }; if (numInstanceFields > 0) @@ -211,7 +212,7 @@ public override ComputedStaticFieldLayout ComputeStaticFieldLayout(DefType defTy } ref StaticsBlock block = ref GetStaticsBlockForField(ref result, field); - SizeAndAlignment sizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout: false, context.Target.DefaultPackingSize, out bool _, out bool _); + SizeAndAlignment sizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout: false, context.Target.DefaultPackingSize, out bool _, out bool _, out bool _); block.Size = LayoutInt.AlignUp(block.Size, sizeAndAlignment.Alignment, context.Target); result.Offsets[index] = new FieldAndOffset(field, block.Size); @@ -303,15 +304,18 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty int fieldOrdinal = 0; bool layoutAbiStable = true; bool hasAutoLayoutField = false; + bool hasInt128Field = type.BaseType == null ? false : type.BaseType.IsInt128OrHasInt128Fields; foreach (var fieldAndOffset in layoutMetadata.Offsets) { TypeDesc fieldType = fieldAndOffset.Field.FieldType; - var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType.UnderlyingType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable, out bool fieldHasAutoLayout); + var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType.UnderlyingType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable, out bool fieldHasAutoLayout, out bool fieldHasInt128Field); if (!fieldLayoutAbiStable) layoutAbiStable = false; if (fieldHasAutoLayout) hasAutoLayoutField = true; + if (fieldHasInt128Field) + hasInt128Field = true; largestAlignmentRequired = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequired); @@ -357,6 +361,7 @@ protected ComputedInstanceFieldLayout ComputeExplicitFieldLayout(MetadataType ty ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout { IsAutoLayoutOrHasAutoLayoutFields = hasAutoLayoutField, + IsInt128OrHasInt128Fields = hasInt128Field, }; computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; computedLayout.FieldSize = instanceSizeAndAlignment.Size; @@ -392,17 +397,20 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType int packingSize = ComputePackingSize(type, layoutMetadata); bool layoutAbiStable = true; bool hasAutoLayoutField = false; + bool hasInt128Field = type.BaseType == null ? false : type.BaseType.IsInt128OrHasInt128Fields; foreach (var field in type.GetFields()) { if (field.IsStatic) continue; - var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType.UnderlyingType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable, out bool fieldHasAutoLayout); + var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType.UnderlyingType, hasLayout: true, packingSize, out bool fieldLayoutAbiStable, out bool fieldHasAutoLayout, out bool fieldHasInt128Field); if (!fieldLayoutAbiStable) layoutAbiStable = false; if (fieldHasAutoLayout) hasAutoLayoutField = true; + if (fieldHasInt128Field) + hasInt128Field = true; largestAlignmentRequirement = LayoutInt.Max(fieldSizeAndAlignment.Alignment, largestAlignmentRequirement); @@ -424,6 +432,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout { IsAutoLayoutOrHasAutoLayoutFields = hasAutoLayoutField, + IsInt128OrHasInt128Fields = hasInt128Field, }; computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; computedLayout.FieldSize = instanceSizeAndAlignment.Size; @@ -459,6 +468,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, int instanceValueClassFieldCount = 0; int instanceGCPointerFieldsCount = 0; int[] instanceNonGCPointerFieldsCount = new int[maxLog2Size + 1]; + bool hasInt128Field = false; foreach (var field in type.GetFields()) { @@ -471,6 +481,8 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, { // Valuetypes which are not primitives or enums instanceValueClassFieldCount++; + if (((DefType)fieldType).IsInt128OrHasInt128Fields) + hasInt128Field = true; } else if (fieldType.IsGCPointer) { @@ -480,7 +492,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, { Debug.Assert(fieldType.IsPrimitive || fieldType.IsPointer || fieldType.IsFunctionPointer || fieldType.IsEnum || fieldType.IsByRef); - var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout, packingSize, out bool _, out bool _); + var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout, packingSize, out bool _, out bool _, out bool _); instanceNonGCPointerFieldsCount[CalculateLog2(fieldSizeAndAlignment.Size.AsInt)]++; } } @@ -517,7 +529,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, TypeDesc fieldType = field.FieldType; - var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout, packingSize, out bool fieldLayoutAbiStable, out bool _); + var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(fieldType, hasLayout, packingSize, out bool fieldLayoutAbiStable, out bool _, out bool _); if (!fieldLayoutAbiStable) layoutAbiStable = false; @@ -678,7 +690,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, for (int i = 0; i < instanceValueClassFieldsArr.Length; i++) { // Align the cumulative field offset to the indeterminate value - var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(instanceValueClassFieldsArr[i].FieldType, hasLayout, packingSize, out bool fieldLayoutAbiStable, out bool _); + var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(instanceValueClassFieldsArr[i].FieldType, hasLayout, packingSize, out bool fieldLayoutAbiStable, out bool _, out bool _); if (!fieldLayoutAbiStable) layoutAbiStable = false; @@ -729,6 +741,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, ComputedInstanceFieldLayout computedLayout = new ComputedInstanceFieldLayout { IsAutoLayoutOrHasAutoLayoutFields = true, + IsInt128OrHasInt128Fields = hasInt128Field, }; computedLayout.FieldAlignment = instanceSizeAndAlignment.Alignment; computedLayout.FieldSize = instanceSizeAndAlignment.Size; @@ -742,7 +755,7 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, private static void PlaceInstanceField(FieldDesc field, bool hasLayout, int packingSize, FieldAndOffset[] offsets, ref LayoutInt instanceFieldPos, ref int fieldOrdinal, LayoutInt offsetBias) { - var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, hasLayout, packingSize, out bool _, out bool _); + var fieldSizeAndAlignment = ComputeFieldSizeAndAlignment(field.FieldType, hasLayout, packingSize, out bool _, out bool _, out bool _); instanceFieldPos = AlignUpInstanceFieldOffset(field.OwningType, instanceFieldPos, fieldSizeAndAlignment.Alignment, field.Context.Target); offsets[fieldOrdinal] = new FieldAndOffset(field, instanceFieldPos + offsetBias); @@ -802,11 +815,12 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8 return cumulativeInstanceFieldPos; } - private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, bool hasLayout, int packingSize, out bool layoutAbiStable, out bool fieldTypeHasAutoLayout) + private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, bool hasLayout, int packingSize, out bool layoutAbiStable, out bool fieldTypeHasAutoLayout, out bool fieldTypeHasInt128Field) { SizeAndAlignment result; layoutAbiStable = true; fieldTypeHasAutoLayout = true; + fieldTypeHasInt128Field = false; if (fieldType.IsDefType) { @@ -817,6 +831,7 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType, result.Alignment = defType.InstanceFieldAlignment; layoutAbiStable = defType.LayoutAbiStable; fieldTypeHasAutoLayout = defType.IsAutoLayoutOrHasAutoLayoutFields; + fieldTypeHasInt128Field = defType.IsInt128OrHasInt128Fields; } else { diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs index b3b8bad1faa7a7..b70749272b0d38 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs @@ -893,7 +893,7 @@ internal static MarshallerKind GetDisabledMarshallerKind( else if (underlyingType.IsValueType) { var defType = (DefType)underlyingType; - if (!defType.ContainsGCPointers && !defType.IsAutoLayoutOrHasAutoLayoutFields) + if (!defType.ContainsGCPointers && !defType.IsAutoLayoutOrHasAutoLayoutFields && !defType.IsInt128OrHasInt128Fields) { return MarshallerKind.BlittableValue; } diff --git a/src/coreclr/vm/class.h b/src/coreclr/vm/class.h index 1fc909efe9c623..22ef2b9919cc19 100644 --- a/src/coreclr/vm/class.h +++ b/src/coreclr/vm/class.h @@ -376,7 +376,9 @@ class EEClassLayoutInfo // The size of the struct is explicitly specified in the meta-data. e_HAS_EXPLICIT_SIZE = 0x08, // The type recursively has a field that is LayoutKind.Auto and not an enum. - e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT = 0x10 + e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT = 0x10, + // Type type recursively has a field which is an Int128 + e_IS_OR_HAS_INT128_FIELD = 0x20, }; BYTE m_bFlags; @@ -426,6 +428,12 @@ class EEClassLayoutInfo return (m_bFlags & e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT) == e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT; } + BOOL IsInt128OrHasInt128Fields() const + { + LIMITED_METHOD_CONTRACT; + return (m_bFlags & e_IS_OR_HAS_INT128_FIELD) == e_IS_OR_HAS_INT128_FIELD; + } + BYTE GetPackingSize() const { LIMITED_METHOD_CONTRACT; @@ -467,6 +475,13 @@ class EEClassLayoutInfo m_bFlags = hasAutoLayoutField ? (m_bFlags | e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT) : (m_bFlags & ~e_HAS_AUTO_LAYOUT_FIELD_IN_LAYOUT); } + + void SetIsInt128OrHasInt128Fields(BOOL hasInt128Field) + { + LIMITED_METHOD_CONTRACT; + m_bFlags = hasInt128Field ? (m_bFlags | e_IS_OR_HAS_INT128_FIELD) + : (m_bFlags & ~e_IS_OR_HAS_INT128_FIELD); + } }; // @@ -1410,6 +1425,9 @@ class EEClass // DO NOT CREATE A NEW EEClass USING NEW! BOOL HasExplicitSize(); BOOL IsAutoLayoutOrHasAutoLayoutField(); + + // Only accurate on non-auto layout types + BOOL IsInt128OrHasInt128Fields(); static void GetBestFitMapping(MethodTable * pMT, BOOL *pfBestFitMapping, BOOL *pfThrowOnUnmappableChar); @@ -2105,6 +2123,15 @@ inline BOOL EEClass::IsAutoLayoutOrHasAutoLayoutField() return !HasLayout() || GetLayoutInfo()->HasAutoLayoutField(); } +inline BOOL EEClass::IsInt128OrHasInt128Fields() +{ + // The name of this type is a slight misnomer as it doesn't detect Int128 fields on + // auto layout types, but since we only need this for interop scenarios, it works out. + LIMITED_METHOD_CONTRACT; + // If this type is not auto + return HasLayout() && GetLayoutInfo()->IsInt128OrHasInt128Fields(); +} + //========================================================================== // These routines manage the prestub (a bootstrapping stub that all // FunctionDesc's are initialized with.) diff --git a/src/coreclr/vm/classlayoutinfo.cpp b/src/coreclr/vm/classlayoutinfo.cpp index 34b04dcd6f7ab3..9f1aedb19f4457 100644 --- a/src/coreclr/vm/classlayoutinfo.cpp +++ b/src/coreclr/vm/classlayoutinfo.cpp @@ -328,6 +328,20 @@ namespace return FALSE; } + BOOL TypeHasInt128Field(CorElementType corElemType, TypeHandle pNestedType) + { + if (CorTypeInfo::IsPrimitiveType(corElemType) || corElemType == ELEMENT_TYPE_PTR || corElemType == ELEMENT_TYPE_FNPTR) + { + return FALSE; + } + if (corElemType == ELEMENT_TYPE_VALUETYPE) + { + _ASSERTE(!pNestedType.IsNull()); + return pNestedType.GetMethodTable()->IsInt128OrHasInt128Fields(); + } + return FALSE; + } + #ifdef UNIX_AMD64_ABI void SystemVAmd64CheckForPassNativeStructInRegister(MethodTable* pMT, EEClassNativeLayoutInfo* pNativeLayoutInfo) { @@ -454,6 +468,7 @@ namespace const SigTypeContext* pTypeContext, BOOL* fDisqualifyFromManagedSequential, BOOL* fHasAutoLayoutField, + BOOL* fHasInt128Field, LayoutRawFieldInfo* pFieldInfoArrayOut, BOOL* pIsBlittableOut, ULONG* cInstanceFields @@ -532,6 +547,7 @@ namespace pFieldInfoArrayOut->m_placement = GetFieldPlacementInfo(corElemType, typeHandleMaybe); *fDisqualifyFromManagedSequential |= TypeHasGCPointers(corElemType, typeHandleMaybe); *fHasAutoLayoutField |= TypeHasAutoLayoutField(corElemType, typeHandleMaybe); + *fHasInt128Field |= TypeHasInt128Field(corElemType, typeHandleMaybe); if (!IsFieldBlittable(pModule, fd, fsig.GetArgProps(), pTypeContext, nativeTypeFlags)) *pIsBlittableOut = FALSE; @@ -625,6 +641,7 @@ VOID EEClassLayoutInfo::CollectLayoutFieldMetadataThrowing( // function exits. BOOL fDisqualifyFromManagedSequential; BOOL hasAutoLayoutField = FALSE; + BOOL hasInt128Field = FALSE; // Check if this type might be ManagedSequential. Only valuetypes marked Sequential can be // ManagedSequential. Other issues checked below might also disqualify the type. @@ -639,9 +656,12 @@ VOID EEClassLayoutInfo::CollectLayoutFieldMetadataThrowing( fDisqualifyFromManagedSequential = TRUE; } - if (pParentMT && !pParentMT->IsValueTypeClass() && pParentMT->IsAutoLayoutOrHasAutoLayoutField()) + if (pParentMT && !pParentMT->IsValueTypeClass()) { - hasAutoLayoutField = TRUE; + if (pParentMT->IsAutoLayoutOrHasAutoLayoutField()) + hasAutoLayoutField = TRUE; + if (pParentMT->IsInt128OrHasInt128Fields()) + hasInt128Field = TRUE; } @@ -692,6 +712,7 @@ VOID EEClassLayoutInfo::CollectLayoutFieldMetadataThrowing( pTypeContext, &fDisqualifyFromManagedSequential, &hasAutoLayoutField, + &hasInt128Field, pInfoArrayOut, &isBlittable, &cInstanceFields @@ -706,6 +727,8 @@ VOID EEClassLayoutInfo::CollectLayoutFieldMetadataThrowing( pEEClassLayoutInfoOut->SetHasAutoLayoutField(hasAutoLayoutField); + pEEClassLayoutInfoOut->SetIsInt128OrHasInt128Fields(hasInt128Field); + S_UINT32 cbSortArraySize = S_UINT32(cTotalFields) * S_UINT32(sizeof(LayoutRawFieldInfo*)); if (cbSortArraySize.IsOverflow()) { diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 239480035443d7..de49ed7b1781b4 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -3326,6 +3326,12 @@ BOOL NDirect::MarshalingRequired( { TypeHandle hndArgType = arg.GetTypeHandleThrowing(pModule, &emptyTypeContext); + if (hndArgType.GetMethodTable()->IsInt128OrHasInt128Fields()) + { + // Int128 cannot be marshalled by value at this time + return TRUE; + } + // When the runtime runtime marshalling system is disabled, we don't support // any types that contain gc pointers, but all "unmanaged" types are treated as blittable // as long as they aren't auto-layout and don't have any auto-layout fields. diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 515cc554b4f6d8..07792d5fbf36d9 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -1487,6 +1487,9 @@ class MethodTable inline BOOL IsAutoLayoutOrHasAutoLayoutField(); + // Only accurate on types which are not auto layout + inline BOOL IsInt128OrHasInt128Fields(); + UINT32 GetNativeSize(); DWORD GetBaseSize() diff --git a/src/coreclr/vm/methodtable.inl b/src/coreclr/vm/methodtable.inl index adcdbd33469858..8f8f8178e26023 100644 --- a/src/coreclr/vm/methodtable.inl +++ b/src/coreclr/vm/methodtable.inl @@ -948,6 +948,13 @@ inline BOOL MethodTable::IsAutoLayoutOrHasAutoLayoutField() return GetClass()->IsAutoLayoutOrHasAutoLayoutField(); } +//========================================================================================== +inline BOOL MethodTable::IsInt128OrHasInt128Fields() +{ + LIMITED_METHOD_CONTRACT; + return HasLayout() && GetClass()->IsInt128OrHasInt128Fields(); +} + //========================================================================================== inline DWORD MethodTable::GetPerInstInfoSize() { diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index e6a6c8ff993107..b0c0c1ce749d37 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -9993,6 +9993,7 @@ void MethodTableBuilder::CheckForSystemTypes() else if ((strcmp(name, g_Int128Name) == 0) || (strcmp(name, g_UInt128Name) == 0)) { EEClassLayoutInfo* pLayout = pClass->GetLayoutInfo(); + pLayout->SetIsInt128OrHasInt128Fields(TRUE); #ifdef TARGET_ARM // No such type exists for the Procedure Call Standard for ARM. We will default // to the same alignment as __m128, which is supported by the ABI. diff --git a/src/coreclr/vm/mlinfo.cpp b/src/coreclr/vm/mlinfo.cpp index 9af29919ca8c79..2ca3596ac9236f 100644 --- a/src/coreclr/vm/mlinfo.cpp +++ b/src/coreclr/vm/mlinfo.cpp @@ -1131,6 +1131,11 @@ namespace *errorResIDOut = IDS_EE_BADMARSHAL_AUTOLAYOUT; return MarshalInfo::MARSHAL_TYPE_UNKNOWN; } + if (pMT->IsInt128OrHasInt128Fields()) + { + *errorResIDOut = IDS_EE_BADMARSHAL_INT128_RESTRICTION; + return MarshalInfo::MARSHAL_TYPE_UNKNOWN; + } *pMTOut = pMT; return MarshalInfo::MARSHAL_TYPE_BLITTABLEVALUECLASS; } @@ -2286,12 +2291,9 @@ MarshalInfo::MarshalInfo(Module* pModule, // * Int128: Represents the 128 bit integer ABI primitive type which requires currently unimplemented handling // * UInt128: Represents the 128 bit integer ABI primitive type which requires currently unimplemented handling // The field layout is correct, so field scenarios work, but these should not be passed by value as parameters - // - // TODO: Contact Jeremy to figure out how to prohibit these types from being used as fields of parameters passed by value - if (!IsFieldScenario()) + if (!IsFieldScenario() && !m_byref) { - if (m_pMT == CoreLibBinder::GetClass(CLASS__INT128) - || m_pMT == CoreLibBinder::GetClass(CLASS__UINT128)) + if (m_pMT->IsInt128OrHasInt128Fields()) { m_resID = IDS_EE_BADMARSHAL_INT128_RESTRICTION; IfFailGoto(E_FAIL, lFail); diff --git a/src/tests/Interop/DisabledRuntimeMarshalling/Native_DisabledMarshalling/DisabledRuntimeMarshallingNative.cs b/src/tests/Interop/DisabledRuntimeMarshalling/Native_DisabledMarshalling/DisabledRuntimeMarshallingNative.cs index ed9b121bef6a23..8a0408768a346d 100644 --- a/src/tests/Interop/DisabledRuntimeMarshalling/Native_DisabledMarshalling/DisabledRuntimeMarshallingNative.cs +++ b/src/tests/Interop/DisabledRuntimeMarshalling/Native_DisabledMarshalling/DisabledRuntimeMarshallingNative.cs @@ -156,6 +156,9 @@ public enum ByteEnum : byte [DllImport(nameof(DisabledRuntimeMarshallingNative), EntryPoint = "Invalid")] public static extern void CallWithVarargs(__arglist); + [DllImport(nameof(DisabledRuntimeMarshallingNative), EntryPoint = "Invalid")] + public static extern void CallWithInt128(Int128 i); + [DllImport(nameof(DisabledRuntimeMarshallingNative))] public static extern delegate* unmanaged GetStructWithShortAndBoolCallback(); diff --git a/src/tests/Interop/DisabledRuntimeMarshalling/PInvokeAssemblyMarshallingDisabled/PInvokes.cs b/src/tests/Interop/DisabledRuntimeMarshalling/PInvokeAssemblyMarshallingDisabled/PInvokes.cs index 4fa7315d43063d..21b0716948d20b 100644 --- a/src/tests/Interop/DisabledRuntimeMarshalling/PInvokeAssemblyMarshallingDisabled/PInvokes.cs +++ b/src/tests/Interop/DisabledRuntimeMarshalling/PInvokeAssemblyMarshallingDisabled/PInvokes.cs @@ -148,4 +148,11 @@ public static void CanUseEnumsWithDisabledMarshalling() { Assert.Equal((byte)ByteEnum.Value, DisabledRuntimeMarshallingNative.GetEnumUnderlyingValue(ByteEnum.Value)); } + + [Fact] + [SkipOnMono("Blocking this on CoreCLR should be good enough.")] + public static void Int128_NotSupported() + { + Assert.Throws(() => DisabledRuntimeMarshallingNative.CallWithInt128(default(Int128))); + } } diff --git a/src/tests/Interop/PInvoke/Int128/Int128Native.cpp b/src/tests/Interop/PInvoke/Int128/Int128Native.cpp index 39cab1cb81e70f..9cd68311421632 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Native.cpp +++ b/src/tests/Interop/PInvoke/Int128/Int128Native.cpp @@ -29,6 +29,11 @@ struct StructWithInt128 Int128 value; }; +struct StructJustInt128 +{ + Int128 value; +}; + extern "C" DLL_EXPORT Int128 STDMETHODCALLTYPE GetInt128(uint64_t upper, uint64_t lower) { Int128 result; @@ -51,6 +56,24 @@ extern "C" DLL_EXPORT void STDMETHODCALLTYPE GetInt128Out(uint64_t upper, uint64 *pValue = value; } +extern "C" DLL_EXPORT uint64_t STDMETHODCALLTYPE GetInt128Lower(Int128 value) +{ +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + return (uint64_t)value; +#else + return value.lower; +#endif +} + +extern "C" DLL_EXPORT uint64_t STDMETHODCALLTYPE GetInt128Lower_S(StructJustInt128 value) +{ +#if (INT128_WIDTH == 128) || defined(__SIZEOF_INT128__) + return (uint64_t)value.value; +#else + return value.value.lower; +#endif +} + extern "C" DLL_EXPORT const Int128* STDMETHODCALLTYPE GetInt128Ptr(uint64_t upper, uint64_t lower) { GetInt128Out(upper, lower, &Int128Value); diff --git a/src/tests/Interop/PInvoke/Int128/Int128Test.cs b/src/tests/Interop/PInvoke/Int128/Int128Test.cs index e3ce25cdf68bd5..a29ff5bd53caf8 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Test.cs +++ b/src/tests/Interop/PInvoke/Int128/Int128Test.cs @@ -5,12 +5,18 @@ using System.Runtime.InteropServices; using Xunit; +struct StructJustInt128 +{ + public StructJustInt128(Int128 val) { value = val; } + public Int128 value; +} + struct StructWithInt128 { public StructWithInt128(Int128 val) { value = val; messUpPadding = 0x10; } public byte messUpPadding; public Int128 value; -}; +} unsafe partial class Int128Native { @@ -23,6 +29,15 @@ unsafe partial class Int128Native [DllImport(nameof(Int128Native))] public static extern void GetInt128Out(ulong upper, ulong lower, out Int128 value); + [DllImport(nameof(Int128Native))] + public static extern void GetInt128Out(ulong upper, ulong lower, out StructJustInt128 value); + + [DllImport(nameof(Int128Native))] + public static extern ulong GetInt128Lower_S(StructJustInt128 value); + + [DllImport(nameof(Int128Native))] + public static extern ulong GetInt128Lower(Int128 value); + [DllImport(nameof(Int128Native))] public static extern Int128* GetInt128Ptr(ulong upper, ulong lower); @@ -85,20 +100,14 @@ unsafe partial class Int128Native unsafe partial class Int128Native { - public static void TestUInt128FieldLayout() + public static void TestInt128FieldLayout() { // This test checks that the alignment rules of Int128 structs match the native compiler StructWithInt128 lhs = new StructWithInt128(new Int128(11, 12)); StructWithInt128 rhs = new StructWithInt128(new Int128(13, 14)); - Int128Native.AddInt128(ref lhs, ref rhs); + Int128Native.AddStructWithInt128_ByRef(ref lhs, ref rhs); Assert.Equal(new StructWithInt128(new Int128(24, 26)), lhs); - } - - private static void TestInt128() - { - Int128 value1 = Int128Native.GetInt128(1, 2); - Assert.Equal(new Int128(1, 2), value1); Int128 value2; Int128Native.GetInt128Out(3, 4, &value2); @@ -107,6 +116,28 @@ private static void TestInt128() Int128Native.GetInt128Out(5, 6, out Int128 value3); Assert.Equal(new Int128(5, 6), value3); + StructJustInt128 value4; + Int128Native.GetInt128Out(7, 8, out value4); + Assert.Equal(new StructJustInt128(new Int128(7, 8)), value4); + + // Until we implement the correct abi for Int128, validate that we don't marshal to native + + // Checking return value + Assert.Throws(() => GetInt128(0, 1)); + + // Checking input value as Int128 itself + Assert.Throws(() => GetInt128Lower(default(Int128))); + + // Checking input value as structure wrapping Int128 + Assert.Throws(() => GetInt128Lower_S(default(StructJustInt128))); + } + + private static void TestInt128() + { + Int128 value1 = Int128Native.GetInt128(1, 2); + Assert.Equal(new Int128(1, 2), value1); + + Int128* value4 = Int128Native.GetInt128Ptr(7, 8); Assert.Equal(new Int128(7, 8), *value4); diff --git a/src/tests/Interop/PInvoke/Int128/UInt128TestFieldLayout.cs b/src/tests/Interop/PInvoke/Int128/UInt128TestFieldLayout.cs deleted file mode 100644 index 567469b5e2e17b..00000000000000 --- a/src/tests/Interop/PInvoke/Int128/UInt128TestFieldLayout.cs +++ /dev/null @@ -1,51 +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 Xunit; - -unsafe partial class Int128Native -{ - private static void TestUInt128() - { - UInt128 value1 = Int128Native.GetUInt128(1, 2); - Assert.Equal(new UInt128(1, 2), value1); - - UInt128 value2; - Int128Native.GetUInt128Out(3, 4, &value2); - Assert.Equal(new UInt128(3, 4), value2); - - Int128Native.GetUInt128Out(5, 6, out UInt128 value3); - Assert.Equal(new UInt128(5, 6), value3); - - UInt128* value4 = Int128Native.GetUInt128Ptr(7, 8); - Assert.Equal(new UInt128(7, 8), *value4); - - ref readonly UInt128 value5 = ref Int128Native.GetUInt128Ref(9, 10); - Assert.Equal(new UInt128(9, 10), value5); - - UInt128 value6 = Int128Native.AddUInt128(new UInt128(11, 12), new UInt128(13, 14)); - Assert.Equal(new UInt128(24, 26), value6); - - UInt128[] values = new UInt128[] { - new UInt128(15, 16), - new UInt128(17, 18), - new UInt128(19, 20), - new UInt128(21, 22), - new UInt128(23, 24), - }; - - fixed (UInt128* pValues = &values[0]) - { - UInt128 value7 = Int128Native.AddUInt128s(pValues, values.Length); - Assert.Equal(new UInt128(95, 100), value7); - } - - UInt128 value8 = Int128Native.AddUInt128s(values, values.Length); - Assert.Equal(new UInt128(95, 100), value8); - - UInt128 value9 = Int128Native.AddUInt128s(in values[0], values.Length); - Assert.Equal(new UInt128(95, 100), value9); - } -} diff --git a/src/tests/issues.targets b/src/tests/issues.targets index f17f96f263b869..ff8fc01a8546c8 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -8,10 +8,16 @@ https://github.com/dotnet/runtime/issues/13703 + + https://github.com/dotnet/runtime/issues/74209 + + + https://github.com/dotnet/runtime/issues/11213 + https://github.com/dotnet/runtime/issues/11213 From 4fab4d37a17e808a866d08d61e85f88bbc0ad614 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 18 Aug 2022 22:47:17 -0700 Subject: [PATCH 09/11] Try to fix PR job --- src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h | 4 ++-- src/tests/Interop/PInvoke/Int128/Int128Test.cs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h b/src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h index 176aef2ad1033b..6f77813cd06143 100644 --- a/src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h +++ b/src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h @@ -10,8 +10,8 @@ struct ReadyToRunHeaderConstants { static const uint32_t Signature = 0x00525452; // 'RTR' - static const uint32_t CurrentMajorVersion = 7; - static const uint32_t CurrentMinorVersion = 1; + static const uint32_t CurrentMajorVersion = 8; + static const uint32_t CurrentMinorVersion = 0; }; struct ReadyToRunHeader diff --git a/src/tests/Interop/PInvoke/Int128/Int128Test.cs b/src/tests/Interop/PInvoke/Int128/Int128Test.cs index a29ff5bd53caf8..c6a8c378d6f25e 100644 --- a/src/tests/Interop/PInvoke/Int128/Int128Test.cs +++ b/src/tests/Interop/PInvoke/Int128/Int128Test.cs @@ -5,6 +5,7 @@ using System.Runtime.InteropServices; using Xunit; + struct StructJustInt128 { public StructJustInt128(Int128 val) { value = val; } From 1170afe4a396cfe72cc91850715ab652861e2d31 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 19 Aug 2022 00:48:37 -0700 Subject: [PATCH 10/11] Should fix the test issues --- src/tests/JIT/Stress/ABI/Gen.cs | 2 +- src/tests/issues.targets | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tests/JIT/Stress/ABI/Gen.cs b/src/tests/JIT/Stress/ABI/Gen.cs index c4cc8396994b68..ddb7c1215c87ef 100644 --- a/src/tests/JIT/Stress/ABI/Gen.cs +++ b/src/tests/JIT/Stress/ABI/Gen.cs @@ -49,7 +49,7 @@ internal static object GenConstant(Type type, FieldInfo[] fields, Random rand) return (double)rand.Next(); if (type == typeof(Int128)) - return new Int128((ulong)GenConstant(typeof(ulong), null, rand), (ulong)GenConstant(typeof(ulong), null, rand)); + return new Int128((ulong)(long)GenConstant(typeof(long), null, rand), (ulong)(long)GenConstant(typeof(long), null, rand)); if (type == typeof(Vector)) return GenConstantVector, int>(rand); diff --git a/src/tests/issues.targets b/src/tests/issues.targets index ff8fc01a8546c8..172e42e5431a23 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -15,9 +15,6 @@ - - https://github.com/dotnet/runtime/issues/11213 - https://github.com/dotnet/runtime/issues/11213 @@ -1424,6 +1421,9 @@ + + https://github.com/dotnet/runtime/issues/74223 + https://github.com/dotnet/runtime/issues/71656 From 81c4439e3a33dc32b5dbd4f81222cbb779fd1407 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 19 Aug 2022 11:54:36 -0700 Subject: [PATCH 11/11] Apply suggestions from code review Co-authored-by: Jeremy Koritzinsky --- src/coreclr/vm/classlayoutinfo.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coreclr/vm/classlayoutinfo.cpp b/src/coreclr/vm/classlayoutinfo.cpp index 9f1aedb19f4457..a37d7a06521216 100644 --- a/src/coreclr/vm/classlayoutinfo.cpp +++ b/src/coreclr/vm/classlayoutinfo.cpp @@ -330,10 +330,6 @@ namespace BOOL TypeHasInt128Field(CorElementType corElemType, TypeHandle pNestedType) { - if (CorTypeInfo::IsPrimitiveType(corElemType) || corElemType == ELEMENT_TYPE_PTR || corElemType == ELEMENT_TYPE_FNPTR) - { - return FALSE; - } if (corElemType == ELEMENT_TYPE_VALUETYPE) { _ASSERTE(!pNestedType.IsNull());