diff --git a/src/coreclr/tools/Common/JitInterface/SystemVStructClassificator.cs b/src/coreclr/tools/Common/JitInterface/SystemVStructClassificator.cs index bfb88cf877537e..5aa1265a56bdcb 100644 --- a/src/coreclr/tools/Common/JitInterface/SystemVStructClassificator.cs +++ b/src/coreclr/tools/Common/JitInterface/SystemVStructClassificator.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using ILCompiler; using Internal.TypeSystem; +using System.Runtime.CompilerServices; using static Internal.JitInterface.SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR; using static Internal.JitInterface.SystemVClassificationType; @@ -207,7 +208,10 @@ private static bool ClassifyEightBytes(TypeDesc typeDesc, if (numIntroducedFields == 0) { - return false; + // Classify empty struct like padding + helper.LargestFieldOffset = startOffsetOfStruct; + AssignClassifiedEightByteTypes(ref helper); + return true; } // The SIMD and Int128 Intrinsic types are meant to be handled specially and should not be passed as struct registers @@ -375,8 +379,12 @@ private static void AssignClassifiedEightByteTypes(ref SystemVStructRegisterPass // Calculate the eightbytes and their types. int lastFieldOrdinal = sortedFieldOrder[largestFieldOffset]; - int offsetAfterLastFieldByte = largestFieldOffset + helper.FieldSizes[lastFieldOrdinal]; - SystemVClassificationType lastFieldClassification = helper.FieldClassifications[lastFieldOrdinal]; + int lastFieldSize = (lastFieldOrdinal >= 0) ? helper.FieldSizes[lastFieldOrdinal] : 0; + int offsetAfterLastFieldByte = largestFieldOffset + lastFieldSize; + Debug.Assert(offsetAfterLastFieldByte <= helper.StructSize); + SystemVClassificationType lastFieldClassification = (lastFieldOrdinal >= 0) + ? helper.FieldClassifications[lastFieldOrdinal] + : SystemVClassificationTypeNoClass; int usedEightBytes = 0; int accumulatedSizeForEightBytes = 0; @@ -403,6 +411,8 @@ private static void AssignClassifiedEightByteTypes(ref SystemVStructRegisterPass // the SysV ABI spec. fieldSize = 1; fieldClassificationType = offset < offsetAfterLastFieldByte ? SystemVClassificationTypeNoClass : lastFieldClassification; + if (offset % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES == 0) // new eightbyte + foundFieldInEightByte = false; } else { @@ -455,13 +465,16 @@ private static void AssignClassifiedEightByteTypes(ref SystemVStructRegisterPass } } - if ((offset + 1) % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES == 0) // If we just finished checking the last byte of an eightbyte + // If we just finished checking the last byte of an eightbyte or the entire struct + if ((offset + 1) % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES == 0 || (offset + 1) == helper.StructSize) { if (!foundFieldInEightByte) { - // If we didn't find a field in an eight-byte (i.e. there are no explicit offsets that start a field in this eightbyte) + // If we didn't find a field in an eightbyte (i.e. there are no explicit offsets that start a field in this eightbyte) // then the classification of this eightbyte might be NoClass. We can't hand a classification of NoClass to the JIT // so set the class to Integer (as though the struct has a char[8] padding) if the class is NoClass. + // + // TODO: Fix JIT, NoClass eightbytes are valid and passing them is broken because of this. if (helper.EightByteClassifications[offset / SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES] == SystemVClassificationTypeNoClass) { helper.EightByteClassifications[offset / SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES] = SystemVClassificationTypeInteger; diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index a3e9641bc73e1a..e16db1435c1e72 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -2114,11 +2114,14 @@ bool MethodTable::ClassifyEightBytesWithManagedLayout(SystemVStructRegisterPassi DWORD numIntroducedFields = GetNumIntroducedInstanceFields(); - // It appears the VM gives a struct with no fields of size 1. - // Don't pass in register such structure. if (numIntroducedFields == 0) { - return false; + helperPtr->largestFieldOffset = startOffsetOfStruct; + LOG((LF_JIT, LL_EVERYTHING, "%*s**** Classify empty struct %s (%p) like padding, startOffset %d, total struct size %d\n", + nestingLevel * 5, "", this->GetDebugClassName(), this, startOffsetOfStruct, helperPtr->structSize)); + + AssignClassifiedEightByteTypes(helperPtr, nestingLevel); + return true; } // The SIMD Intrinsic types are meant to be handled specially and should not be passed as struct registers @@ -2334,7 +2337,12 @@ bool MethodTable::ClassifyEightBytesWithNativeLayout(SystemVStructRegisterPassin // No fields. if (numIntroducedFields == 0) { - return false; + helperPtr->largestFieldOffset = startOffsetOfStruct; + LOG((LF_JIT, LL_EVERYTHING, "%*s**** Classify empty struct %s (%p) like padding, startOffset %d, total struct size %d\n", + nestingLevel * 5, "", this->GetDebugClassName(), this, startOffsetOfStruct, helperPtr->structSize)); + + AssignClassifiedEightByteTypes(helperPtr, nestingLevel); + return true; } bool hasImpliedRepeatedFields = HasImpliedRepeatedFields(this); @@ -2586,8 +2594,12 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe // Calculate the eightbytes and their types. int lastFieldOrdinal = sortedFieldOrder[largestFieldOffset]; - unsigned int offsetAfterLastFieldByte = largestFieldOffset + helperPtr->fieldSizes[lastFieldOrdinal]; - SystemVClassificationType lastFieldClassification = helperPtr->fieldClassifications[lastFieldOrdinal]; + unsigned int lastFieldSize = (lastFieldOrdinal >= 0) ? helperPtr->fieldSizes[lastFieldOrdinal] : 0; + unsigned int offsetAfterLastFieldByte = largestFieldOffset + lastFieldSize; + _ASSERTE(offsetAfterLastFieldByte <= helperPtr->structSize); + SystemVClassificationType lastFieldClassification = (lastFieldOrdinal >= 0) + ? helperPtr->fieldClassifications[lastFieldOrdinal] + : SystemVClassificationTypeNoClass; unsigned int usedEightBytes = 0; unsigned int accumulatedSizeForEightBytes = 0; @@ -2614,6 +2626,8 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe // the SysV ABI spec. fieldSize = 1; fieldClassificationType = offset < offsetAfterLastFieldByte ? SystemVClassificationTypeNoClass : lastFieldClassification; + if (offset % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES == 0) // new eightbyte + foundFieldInEightByte = false; } else { @@ -2666,13 +2680,16 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe } } - if ((offset + 1) % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES == 0) // If we just finished checking the last byte of an eightbyte + // If we just finished checking the last byte of an eightbyte or the entire struct + if ((offset + 1) % SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES == 0 || (offset + 1) == helperPtr->structSize) { if (!foundFieldInEightByte) { - // If we didn't find a field in an eight-byte (i.e. there are no explicit offsets that start a field in this eightbyte) + // If we didn't find a field in an eightbyte (i.e. there are no explicit offsets that start a field in this eightbyte) // then the classification of this eightbyte might be NoClass. We can't hand a classification of NoClass to the JIT // so set the class to Integer (as though the struct has a char[8] padding) if the class is NoClass. + // + // TODO: Fix JIT, NoClass eightbytes are valid and passing them is broken because of this. if (helperPtr->eightByteClassifications[offset / SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES] == SystemVClassificationTypeNoClass) { helperPtr->eightByteClassifications[offset / SYSTEMV_EIGHT_BYTE_SIZE_IN_BYTES] = SystemVClassificationTypeInteger; @@ -2705,9 +2722,9 @@ void MethodTable::AssignClassifiedEightByteTypes(SystemVStructRegisterPassingHe LOG((LF_JIT, LL_EVERYTHING, " **** Number EightBytes: %d\n", helperPtr->eightByteCount)); for (unsigned i = 0; i < helperPtr->eightByteCount; i++) { - _ASSERTE(helperPtr->eightByteClassifications[i] != SystemVClassificationTypeNoClass); LOG((LF_JIT, LL_EVERYTHING, " **** eightByte %d -- classType: %s, eightByteOffset: %d, eightByteSize: %d\n", i, GetSystemVClassificationTypeName(helperPtr->eightByteClassifications[i]), helperPtr->eightByteOffsets[i], helperPtr->eightByteSizes[i])); + _ASSERTE(helperPtr->eightByteClassifications[i] != SystemVClassificationTypeNoClass); } #endif // _DEBUG } diff --git a/src/tests/JIT/Directed/StructABI/CMakeLists.txt b/src/tests/JIT/Directed/StructABI/CMakeLists.txt index 1c7c8684995a59..cf028a3c8670e8 100644 --- a/src/tests/JIT/Directed/StructABI/CMakeLists.txt +++ b/src/tests/JIT/Directed/StructABI/CMakeLists.txt @@ -2,13 +2,15 @@ project (StructABILib) include_directories(${INC_PLATFORM_DIR}) if(CLR_CMAKE_HOST_WIN32) - add_compile_options(/TC) # compile all files as C + set_source_files_properties(StructABI.c PROPERTIES COMPILE_OPTIONS /TC) # compile as C else() set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fvisibility=hidden") + set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -fvisibility=hidden -Wno-return-type-c-linkage") endif() # add the executable add_library (StructABILib SHARED StructABI.c) +add_library (EmptyStructsLib SHARED EmptyStructs.cpp) # add the install targets -install (TARGETS StructABILib DESTINATION bin) +install (TARGETS StructABILib EmptyStructsLib DESTINATION bin) diff --git a/src/tests/JIT/Directed/StructABI/EmptyStructs.cpp b/src/tests/JIT/Directed/StructABI/EmptyStructs.cpp new file mode 100644 index 00000000000000..8e21d2642bf49e --- /dev/null +++ b/src/tests/JIT/Directed/StructABI/EmptyStructs.cpp @@ -0,0 +1,68 @@ +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#include +#include + +#ifdef _MSC_VER +#define DLLEXPORT __declspec(dllexport) +#else +#define DLLEXPORT __attribute__((visibility("default"))) +#endif // _MSC_VER + +struct Empty +{ +}; +static_assert(sizeof(Empty) == 1, "Empty struct must be sized like in .NET"); + + +struct IntEmpty +{ + int32_t Int0; + Empty Empty0; +}; + +extern "C" DLLEXPORT IntEmpty EchoIntEmptySysV(int i0, IntEmpty val) +{ + return val; +} + + +struct IntEmptyPair +{ + IntEmpty IntEmpty0; + IntEmpty IntEmpty1; +}; + +extern "C" DLLEXPORT IntEmptyPair EchoIntEmptyPairSysV(int i0, IntEmptyPair val) +{ + return val; +} + + +struct EmptyFloatIntInt +{ + Empty Empty0; + float Float0; + int32_t Int0; + int32_t Int1; +}; + +extern "C" DLLEXPORT EmptyFloatIntInt EchoEmptyFloatIntIntSysV(int i0, float f0, EmptyFloatIntInt val) +{ + return val; +} + + +struct FloatFloatEmptyFloat +{ + float Float0; + float Float1; + Empty Empty0; + float Float2; +}; + +extern "C" DLLEXPORT FloatFloatEmptyFloat EchoFloatFloatEmptyFloatSysV(float f0, FloatFloatEmptyFloat val) +{ + return val; +} + diff --git a/src/tests/JIT/Directed/StructABI/EmptyStructs.cs b/src/tests/JIT/Directed/StructABI/EmptyStructs.cs new file mode 100644 index 00000000000000..d6b3334f80ba96 --- /dev/null +++ b/src/tests/JIT/Directed/StructABI/EmptyStructs.cs @@ -0,0 +1,150 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; +using Xunit; + +public static class Program +{ + public struct Empty + { + } + + + public struct IntEmpty + { + public int Int0; + public Empty Empty0; + + public static IntEmpty Get() + => new IntEmpty { Int0 = 0xBabc1a }; + + public bool Equals(IntEmpty other) + => Int0 == other.Int0; + + public override string ToString() + => $"{{Int0:{Int0:x}}}"; + } + + [DllImport("EmptyStructsLib")] + public static extern IntEmpty EchoIntEmptySysV(int i0, IntEmpty val); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static IntEmpty EchoIntEmptySysVManaged(int i0, IntEmpty val) => val; + + [Fact] + public static void TestIntEmptySysV() + { + IntEmpty expected = IntEmpty.Get(); + IntEmpty native = EchoIntEmptySysV(0, expected); + IntEmpty managed = EchoIntEmptySysVManaged(0, expected); + + Assert.Equal(expected, native); + Assert.Equal(expected, managed); + } + + + public struct IntEmptyPair + { + public IntEmpty IntEmpty0; + public IntEmpty IntEmpty1; + + public static IntEmptyPair Get() + => new IntEmptyPair { IntEmpty0 = IntEmpty.Get(), IntEmpty1 = IntEmpty.Get() }; + + public bool Equals(IntEmptyPair other) + => IntEmpty0.Equals(other.IntEmpty0) && IntEmpty1.Equals(other.IntEmpty1); + + public override string ToString() + => $"{{IntEmpty0:{IntEmpty0}, IntEmpty1:{IntEmpty1}}}"; + } + + [DllImport("EmptyStructsLib")] + public static extern IntEmptyPair EchoIntEmptyPairSysV(int i0, IntEmptyPair val); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static IntEmptyPair EchoIntEmptyPairSysVManaged(int i0, IntEmptyPair val) => val; + + [Fact] + public static void TestIntEmptyPairSysV() + { + IntEmptyPair expected = IntEmptyPair.Get(); + IntEmptyPair native = EchoIntEmptyPairSysV(0, expected); + IntEmptyPair managed = EchoIntEmptyPairSysVManaged(0, expected); + + Assert.Equal(expected, native); + Assert.Equal(expected, managed); + } + + + public struct EmptyFloatIntInt + { + public Empty Empty0; + public float Float0; + public int Int0; + public int Int1; + + public static EmptyFloatIntInt Get() + => new EmptyFloatIntInt { Float0 = 2.71828f, Int0 = 0xBabc1a, Int1 = 0xC10c1a }; + + public bool Equals(EmptyFloatIntInt other) + => Float0 == other.Float0 && Int0 == other.Int0 && Int1 == other.Int1; + + public override string ToString() + => $"{{Float0:{Float0}, Int0:{Int0:x}, Int1:{Int1:x}}}"; + } + + [DllImport("EmptyStructsLib")] + public static extern EmptyFloatIntInt EchoEmptyFloatIntIntSysV(int i0, float f0, EmptyFloatIntInt val); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static EmptyFloatIntInt EchoEmptyFloatIntIntSysVManaged(int i0, float f0, EmptyFloatIntInt val) => val; + + [Fact] + public static void TestEmptyFloatIntIntSysV() + { + EmptyFloatIntInt expected = EmptyFloatIntInt.Get(); + EmptyFloatIntInt native = EchoEmptyFloatIntIntSysV(0, 0f, expected); + EmptyFloatIntInt managed = EchoEmptyFloatIntIntSysVManaged(0, 0f, expected); + + Assert.Equal(expected, native); + Assert.Equal(expected, managed); + } + + + public struct FloatFloatEmptyFloat + { + public float Float0; + public float Float1; + public Empty Empty0; + public float Float2; + + public static FloatFloatEmptyFloat Get() + => new FloatFloatEmptyFloat { Float0 = 2.71828f, Float1 = 3.14159f, Float2 = 1.61803f }; + + public bool Equals(FloatFloatEmptyFloat other) + => Float0 == other.Float0 && Float1 == other.Float1 && Float2 == other.Float2; + + public override string ToString() + => $"{{Float0:{Float0}, Float1:{Float1}, Float2:{Float2}}}"; + } + + [DllImport("EmptyStructsLib")] + public static extern FloatFloatEmptyFloat EchoFloatFloatEmptyFloatSysV(float f0, FloatFloatEmptyFloat val); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static FloatFloatEmptyFloat EchoFloatFloatEmptyFloatSysVManaged(float f0, FloatFloatEmptyFloat val) => val; + + [Fact] + public static void TestFloatFloatEmptyFloatSysV() + { + FloatFloatEmptyFloat expected = FloatFloatEmptyFloat.Get(); + FloatFloatEmptyFloat native = EchoFloatFloatEmptyFloatSysV(0f, expected); + FloatFloatEmptyFloat managed = EchoFloatFloatEmptyFloatSysVManaged(0f, expected); + + Assert.Equal(expected, native); + Assert.Equal(expected, managed); + } +} \ No newline at end of file diff --git a/src/tests/JIT/Directed/StructABI/EmptyStructs.csproj b/src/tests/JIT/Directed/StructABI/EmptyStructs.csproj new file mode 100644 index 00000000000000..372d01156084c0 --- /dev/null +++ b/src/tests/JIT/Directed/StructABI/EmptyStructs.csproj @@ -0,0 +1,17 @@ + + + + true + true + + + PdbOnly + True + + + + + + + +