Skip to content

Commit

Permalink
[RISC-V][LoongArch64] Pass structs containing empty struct arrays acc…
Browse files Browse the repository at this point in the history 
…ording to integer calling convention (#106266)

* Don't pass structs containing empty struct arrays according to hardware floating-point calling convention

* Fix adding registers after the passed struct in native version of Echo_DoubleFloatNestedEmpty_InIntegerRegs_RiscV

* Make sure xunit uses the Equals implementation provided by overriding Equals from object

* Add comment to clarify why EmptyStructs test uses C++ instead of C
  • Loading branch information
@tomeksowi
tomeksowi committed Aug 19, 2024
1 parent e96eaee commit 71373ae
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 30 deletions.
11 changes: 9 additions & 2 deletions src/coreclr/tools/Common/Internal/Runtime/RiscVLoongArch64FpStruct.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ public struct FpStructInRegistersInfo
public uint offset2nd;

public uint SizeShift1st() { return (uint)((int)flags >> (int)FpStruct.PosSizeShift1st) & 0b11; }

public uint SizeShift2nd() { return (uint)((int)flags >> (int)FpStruct.PosSizeShift2nd) & 0b11; }

public uint Size1st() { return 1u << (int)SizeShift1st(); }
Expand Down Expand Up @@ -84,7 +83,7 @@ private static bool HandleInlineArray(int elementTypeIndex, int nElements, ref F
int nFlattenedFieldsPerElement = typeIndex - elementTypeIndex;
if (nFlattenedFieldsPerElement == 0)
{
Debug.Assert(nElements == 1, "HasImpliedRepeatedFields must have returned a false positive");
Debug.Assert(nElements == 1, "HasImpliedRepeatedFields must have returned a false, it can't be an array");
return true; // ignoring empty struct
}

Expand Down Expand Up @@ -158,6 +157,14 @@ private static bool FlattenFields(TypeDesc td, uint offset, ref FpStructInRegist
{
Debug.Assert(nFields == 1);
int nElements = td.GetElementSize().AsInt / prevField.FieldType.GetElementSize().AsInt;

// Only InlineArrays can have element type of empty struct, fixed-size buffers take only primitives
if ((typeIndex - elementTypeIndex) == 0 && (td as MetadataType).IsInlineArray)
{
Debug.Assert(nElements > 0, "InlineArray length must be > 0");
return false; // struct containing an array of empty structs is passed by integer calling convention
}

if (!HandleInlineArray(elementTypeIndex, nElements, ref info, ref typeIndex))
return false;
}
Expand Down
13 changes: 12 additions & 1 deletion src/coreclr/vm/methodtable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2710,7 +2710,7 @@ static bool HandleInlineArray(int elementTypeIndex, int nElements, FpStructInReg
int nFlattenedFieldsPerElement = typeIndex - elementTypeIndex;
if (nFlattenedFieldsPerElement == 0)
{
assert(nElements == 1); // HasImpliedRepeatedFields must have returned a false positive
assert(nElements == 1); // HasImpliedRepeatedFields must have returned a false positive, it can't be an array
LOG((LF_JIT, LL_EVERYTHING, "FpStructInRegistersInfo:%*s * ignoring empty struct\n",
nestingLevel * 4, ""));
return true;
Expand Down Expand Up @@ -2819,6 +2819,17 @@ static bool FlattenFields(TypeHandle th, uint32_t offset, FpStructInRegistersInf
{
assert(nFields == 1);
int nElements = pMT->GetNumInstanceFieldBytes() / fields[0].GetSize();

// Only InlineArrays can have element type of empty struct, fixed-size buffers take only primitives
if ((typeIndex - elementTypeIndex) == 0 && pMT->GetClass()->IsInlineArray())
{
assert(nElements > 0); // InlineArray length must be > 0
LOG((LF_JIT, LL_EVERYTHING, "FpStructInRegistersInfo:%*s "
" * struct %s containing a %i-element array of empty structs %s is passed by integer calling convention\n",
nestingLevel * 4, "", pMT->GetDebugClassName(), nElements, fields[0].GetDebugName()));
return false;
}

if (!HandleInlineArray(elementTypeIndex, nElements, info, typeIndex DEBUG_ARG(nestingLevel + 1)))
return false;
}
Expand Down
22 changes: 21 additions & 1 deletion src/tests/JIT/Directed/StructABI/EmptyStructs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,28 @@ extern "C" DLLEXPORT DoubleFloatNestedEmpty Echo_DoubleFloatNestedEmpty_InIntege
float fa0, float fa1, float fa2, float fa3, float fa4, float fa5, float fa6,
DoubleFloatNestedEmpty a1_a2, int a3, float fa7)
{
return a1_a2;
a1_a2.Float0 += (float)a3 + fa7;
return a1_a2;
}


struct ArrayOfEmpties
{
Empty e[1];
};

struct ArrayOfEmptiesFloatDouble
{
ArrayOfEmpties ArrayOfEmpties0;
float Float0;
double Double0;
};

extern "C" DLLEXPORT ArrayOfEmptiesFloatDouble Echo_ArrayOfEmptiesFloatDouble_RiscV(int a0, float fa0,
ArrayOfEmptiesFloatDouble a1_a2, int a3, float fa1)
{
a1_a2.Double0 += (double)a3 + fa1;
return a1_a2;
}


Expand Down
118 changes: 92 additions & 26 deletions src/tests/JIT/Directed/StructABI/EmptyStructs.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

// Note: this test checks passing empty struct fields in .NET; confronting it against C++ on native compilers is just
// a means to assert compliance to the platform calling convention. The native part is using C++ because it defines
// empty structs as 1 byte like in .NET. Empty structs in C are undefined (it's a GCC extension to define them as 0
// bytes) and .NET managed/unmanaged interop follows the C ABI, not C++, so signatures with empty struct fields should
// not be used in any real-world interop calls.

using System;
using System.Runtime.InteropServices;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -77,8 +83,8 @@ public struct IntEmpty
public static IntEmpty Get()
=> new IntEmpty { Int0 = 0xBabc1a };

public bool Equals(IntEmpty other)
=> Int0 == other.Int0;
public override bool Equals(object other)
=> other is IntEmpty o && Int0 == o.Int0;

public override string ToString()
=> $"{{Int0:{Int0:x}}}";
Expand Down Expand Up @@ -128,8 +134,8 @@ public struct IntEmptyPair
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 bool Equals(object other)
=> other is IntEmptyPair o && IntEmpty0.Equals(o.IntEmpty0) && IntEmpty1.Equals(o.IntEmpty1);

public override string ToString()
=> $"{{IntEmpty0:{IntEmpty0}, IntEmpty1:{IntEmpty1}}}";
Expand Down Expand Up @@ -181,8 +187,8 @@ public struct EmptyFloatIntInt
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 bool Equals(object other)
=> other is EmptyFloatIntInt o && Float0 == o.Float0 && Int0 == o.Int0 && Int1 == o.Int1;

public override string ToString()
=> $"{{Float0:{Float0}, Int0:{Int0:x}, Int1:{Int1:x}}}";
Expand Down Expand Up @@ -236,8 +242,8 @@ public struct FloatFloatEmptyFloat
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 bool Equals(object other)
=> other is FloatFloatEmptyFloat o && Float0 == o.Float0 && Float1 == o.Float1 && Float2 == o.Float2;

public override string ToString()
=> $"{{Float0:{Float0}, Float1:{Float1}, Float2:{Float2}}}";
Expand Down Expand Up @@ -294,8 +300,8 @@ public struct Empty8Float
public static Empty8Float Get()
=> new Empty8Float { Float0 = 2.71828f };

public bool Equals(Empty8Float other)
=> Float0 == other.Float0;
public override bool Equals(object other)
=> other is Empty8Float o && Float0 == o.Float0;

public override string ToString()
=> $"{{Float0:{Float0}}}";
Expand Down Expand Up @@ -474,8 +480,8 @@ public struct FloatEmpty8Float
public static FloatEmpty8Float Get()
=> new FloatEmpty8Float { Float0 = 2.71828f, Float1 = 3.14159f };

public bool Equals(FloatEmpty8Float other)
=> Float0 == other.Float0 && Float1 == other.Float1;
public override bool Equals(object other)
=> other is FloatEmpty8Float o && Float0 == o.Float0 && Float1 == o.Float1;

public override string ToString()
=> $"{{Float0:{Float0}, Float1:{Float1}}}";
Expand Down Expand Up @@ -654,8 +660,8 @@ public struct FloatEmptyShort
public static FloatEmptyShort Get()
=> new FloatEmptyShort { Float0 = 2.71828f, Short0 = 0x1dea };

public bool Equals(FloatEmptyShort other)
=> Float0 == other.Float0 && Short0 == other.Short0;
public override bool Equals(object other)
=> other is FloatEmptyShort o && Float0 == o.Float0 && Short0 == o.Short0;

public override string ToString()
=> $"{{Float0:{Float0}, Short0:{Short0}}}";
Expand Down Expand Up @@ -793,8 +799,8 @@ public struct EmptyFloatEmpty5Sbyte
public static EmptyFloatEmpty5Sbyte Get()
=> new EmptyFloatEmpty5Sbyte { Float0 = 2.71828f, Sbyte0 = -123 };

public bool Equals(EmptyFloatEmpty5Sbyte other)
=> Float0 == other.Float0 && Sbyte0 == other.Sbyte0;
public override bool Equals(object other)
=> other is EmptyFloatEmpty5Sbyte o && Float0 == o.Float0 && Sbyte0 == o.Sbyte0;

public override string ToString()
=> $"{{Float0:{Float0}, Sbyte0:{Sbyte0}}}";
Expand Down Expand Up @@ -848,8 +854,8 @@ public struct EmptyFloatEmpty5Byte
public static EmptyFloatEmpty5Byte Get()
=> new EmptyFloatEmpty5Byte { Float0 = 2.71828f, Byte0 = 123 };

public bool Equals(EmptyFloatEmpty5Byte other)
=> Float0 == other.Float0 && Byte0 == other.Byte0;
public override bool Equals(object other)
=> other is EmptyFloatEmpty5Byte o && Float0 == o.Float0 && Byte0 == o.Byte0;

public override string ToString()
=> $"{{Float0:{Float0}, Byte0:{Byte0}}}";
Expand Down Expand Up @@ -1037,8 +1043,8 @@ public struct DoubleFloatNestedEmpty
public static DoubleFloatNestedEmpty Get()
=> new DoubleFloatNestedEmpty { Double0 = 2.71828, Float0 = 3.14159f };

public bool Equals(DoubleFloatNestedEmpty other)
=> Double0 == other.Double0 && Float0 == other.Float0;
public override bool Equals(object other)
=> other is DoubleFloatNestedEmpty o && Double0 == o.Double0 && Float0 == o.Float0;

public override string ToString()
=> $"{{Double0:{Double0}, Float0:{Float0}}}";
Expand Down Expand Up @@ -1123,6 +1129,66 @@ public static void Test_DoubleFloatNestedEmpty_InIntegerRegs_ByReflection_RiscV(
}
#endregion

#region ArrayOfEmptiesFloatDouble_RiscVTests
[InlineArray(1)]
public struct ArrayOfEmpties
{
public Empty e;
}

public struct ArrayOfEmptiesFloatDouble
{
public ArrayOfEmpties ArrayOfEmpties0;
public float Float0;
public double Double0;

public static ArrayOfEmptiesFloatDouble Get()
=> new ArrayOfEmptiesFloatDouble { Float0 = 3.14159f, Double0 = 2.71828 };

public override bool Equals(object other)
=> other is ArrayOfEmptiesFloatDouble o && Float0 == o.Float0 && Double0 == o.Double0;

public override string ToString()
=> $"{{Float0:{Float0}, Double0:{Double0}}}";
}

[DllImport("EmptyStructsLib")]
public static extern ArrayOfEmptiesFloatDouble Echo_ArrayOfEmptiesFloatDouble_RiscV(int a0, float fa0,
ArrayOfEmptiesFloatDouble a1_a2, int a3, float fa1);

[MethodImpl(MethodImplOptions.NoInlining)]
public static ArrayOfEmptiesFloatDouble Echo_ArrayOfEmptiesFloatDouble_RiscV_Managed(int a0, float fa0,
ArrayOfEmptiesFloatDouble a1_a2, int a3, float fa1)
{
a1_a2.Double0 += (double)a3 + fa1;
return a1_a2;
}

[Fact]
public static void Test_ArrayOfEmptiesFloatDouble_RiscV()
{
ArrayOfEmptiesFloatDouble expected = ArrayOfEmptiesFloatDouble.Get();
ArrayOfEmptiesFloatDouble native = Echo_ArrayOfEmptiesFloatDouble_RiscV(0, 0f, expected, 1, -1f);
ArrayOfEmptiesFloatDouble managed = Echo_ArrayOfEmptiesFloatDouble_RiscV_Managed(0, 0f, expected, 1, -1f);

Assert.Equal(expected, native);
Assert.Equal(expected, managed);
}

[Fact]
public static void Test_ArrayOfEmptiesFloatDouble_ByReflection_RiscV()
{
var expected = ArrayOfEmptiesFloatDouble.Get();
var native = (ArrayOfEmptiesFloatDouble)typeof(Program).GetMethod("Echo_ArrayOfEmptiesFloatDouble_RiscV").Invoke(
null, new object[] {0, 0f, expected, 1, -1f});
var managed = (ArrayOfEmptiesFloatDouble)typeof(Program).GetMethod("Echo_ArrayOfEmptiesFloatDouble_RiscV_Managed").Invoke(
null, new object[] {0, 0f, expected, 1, -1f});

Assert.Equal(expected, native);
Assert.Equal(expected, managed);
}
#endregion

#region EmptyUshortAndDouble_RiscVTests
public struct EmptyUshortAndDouble
{
Expand All @@ -1138,8 +1204,8 @@ public struct EmptyUshort
EmptyUshort0 = new EmptyUshort { Ushort0 = 0xBaca }, Double0 = 2.71828
};

public bool Equals(EmptyUshortAndDouble other)
=> EmptyUshort0.Ushort0 == other.EmptyUshort0.Ushort0 && Double0 == other.Double0;
public override bool Equals(object other)
=> other is EmptyUshortAndDouble o && EmptyUshort0.Ushort0 == o.EmptyUshort0.Ushort0 && Double0 == o.Double0;

public override string ToString()
=> $"{{EmptyUshort0.Ushort0:{EmptyUshort0.Ushort0}, Double0:{Double0}}}";
Expand Down Expand Up @@ -1193,8 +1259,8 @@ public struct PackedEmptyFloatLong
public static PackedEmptyFloatLong Get()
=> new PackedEmptyFloatLong { Float0 = 2.71828f, Long0 = 0xDadAddedC0ffee };

public bool Equals(PackedEmptyFloatLong other)
=> Float0 == other.Float0 && Long0 == other.Long0;
public override bool Equals(object other)
=> other is PackedEmptyFloatLong o && Float0 == o.Float0 && Long0 == o.Long0;

public override string ToString()
=> $"{{Float0:{Float0}, Long0:{Long0}}}";
Expand Down Expand Up @@ -1383,8 +1449,8 @@ public struct PackedFloatEmptyByte
public static PackedFloatEmptyByte Get()
=> new PackedFloatEmptyByte { Float0 = 2.71828f, Byte0 = 0xba };

public bool Equals(PackedFloatEmptyByte other)
=> Float0 == other.Float0 && Byte0 == other.Byte0;
public override bool Equals(object other)
=> other is PackedFloatEmptyByte o && Float0 == o.Float0 && Byte0 == o.Byte0;

public override string ToString()
=> $"{{Float0:{Float0}, Byte0:{Byte0}}}";
Expand Down

0 comments on commit 71373ae

Please sign in to comment.