If a struct is passed on the stack, it must live on the stack, unless/until we support `GT_FIELD_LIST` for these args. This is unlikely to represent a significant code quality issue, since ARM supports many register args, and this has gone undetected thus far.
This was exposed by tailcall stress on desktop.
I've added a test that exposes the issue without tailcall stress (though it gets a different assert than the desktop failure).
It seemed that `fgMorphMultiregStructArg()` was the best place to fix this - and I noted that this is called for any struct that is larger than a single register. So I updated the comments to reflect that.
I thought about putting the test in the JIT\Regressions test directory, but I consider that it is addressing basic missing test coverage, so I added it to JIT\Methodical\structs.
SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;
#endif // FEATURE_UNIX_AMD64_STRUCT_PASSING
- bool hasStructArgument = false; // @TODO-ARM64-UNIX: Remove this bool during a future refactoring
+ bool hasStructArgument = false; // @TODO-ARM64-UNIX: Remove this bool during a future refactoring
+ // hasMultiregStructArgs is true if there are any structs that are eligible for passing
+ // in registers; this is true even if it is not actually passed in registers (i.e. because
+ // previous arguments have used up available argument registers).
bool hasMultiregStructArgs = false;
for (args = call->gtCallArgs; args; args = args->gtOp.gtOp2, argIndex++)
{
if (size == 2)
{
- // Structs that are the size of 2 pointers are passed by value in multiple registers
+ // Structs that are the size of 2 pointers are passed by value in multiple registers,
+ // if sufficient registers are available.
hasMultiregStructArgs = true;
}
else if (size > 2)
// In the future we can migrate UNIX_AMD64 to use this
// method instead of fgMorphSystemVStructArgs
- // We only build GT_FIELD_LISTs for MultiReg structs for the RyuJIT backend
+ // We only require morphing of structs that may be passed in multiple registers
+ // for the RyuJIT backend.
if (hasMultiregStructArgs)
{
fgMorphMultiregStructArgs(call);
// call: a GenTreeCall node that has one or more TYP_STRUCT arguments
//
// Notes:
-// We only call fgMorphMultiregStructArg for the register passed TYP_STRUCT arguments.
-// The call to fgMorphMultiregStructArg will mutate the argument into the GT_FIELD_LIST form
-// which is only used for struct arguments.
+// We only call fgMorphMultiregStructArg for struct arguments that are not passed as simple types.
+// It will ensure that the struct arguments are in the correct form.
// If this method fails to find any TYP_STRUCT arguments it will assert.
//
void Compiler::fgMorphMultiregStructArgs(GenTreeCall* call)
}
//-----------------------------------------------------------------------------
-// fgMorphMultiregStructArg: Given a multireg TYP_STRUCT arg from a call argument list
-// Morph the argument into a set of GT_FIELD_LIST nodes.
+// fgMorphMultiregStructArg: Given a TYP_STRUCT arg from a call argument list,
+// morph the argument as needed to be passed correctly.
//
// Arguments:
-// arg - A GenTree node containing a TYP_STRUCT arg that
-// is to be passed in multiple registers
+// arg - A GenTree node containing a TYP_STRUCT arg
// fgEntryPtr - the fgArgTabEntry information for the current 'arg'
//
// Notes:
-// arg must be a GT_OBJ or GT_LCL_VAR or GT_LCL_FLD of TYP_STRUCT that is suitable
-// for passing in multiple registers.
+// The arg must be a GT_OBJ or GT_LCL_VAR or GT_LCL_FLD of TYP_STRUCT.
+// If 'arg' is a lclVar passed on the stack, we will ensure that any lclVars that must be on the
+// stack are marked as doNotEnregister, and then we return.
+//
+// If it is passed by register, we mutate the argument into the GT_FIELD_LIST form
+// which is only used for struct arguments.
+//
// If arg is a LclVar we check if it is struct promoted and has the right number of fields
// and if they are at the appropriate offsets we will use the struct promted fields
// in the GT_FIELD_LIST nodes that we create.
if ((fgEntryPtr->isSplit && fgEntryPtr->numSlots + fgEntryPtr->numRegs > 4) ||
(!fgEntryPtr->isSplit && fgEntryPtr->regNum == REG_STK))
{
+ GenTreeLclVarCommon* lcl = nullptr;
+
// If already OBJ it is set properly already.
if (arg->OperGet() == GT_OBJ)
{
- return arg;
+ if (arg->gtGetOp1()->OperIs(GT_ADDR) && arg->gtGetOp1()->gtGetOp1()->OperIs(GT_LCL_VAR))
+ {
+ lcl = arg->gtGetOp1()->gtGetOp1()->AsLclVarCommon();
+ }
}
+ else
+ {
+ assert(arg->OperGet() == GT_LCL_VAR);
- assert(arg->OperGet() == GT_LCL_VAR);
-
- // We need to construct a `GT_OBJ` node for the argmuent,
- // so we need to get the address of the lclVar.
- GenTreeLclVarCommon* lclCommon = arg->AsLclVarCommon();
+ // We need to construct a `GT_OBJ` node for the argmuent,
+ // so we need to get the address of the lclVar.
+ lcl = arg->AsLclVarCommon();
- arg = gtNewOperNode(GT_ADDR, TYP_I_IMPL, arg);
+ arg = gtNewOperNode(GT_ADDR, TYP_I_IMPL, arg);
- // Create an Obj of the temp to use it as a call argument.
- arg = gtNewObjNode(lvaGetStruct(lclCommon->gtLclNum), arg);
+ // Create an Obj of the temp to use it as a call argument.
+ arg = gtNewObjNode(lvaGetStruct(lcl->gtLclNum), arg);
+ }
+ if (lcl != nullptr)
+ {
+ // Its fields will need to accessed by address.
+ lvaSetVarDoNotEnregister(lcl->gtLclNum, DNER_IsStructArg);
+ }
return arg;
}
--- /dev/null
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+//
+
+// This tests passing structs that are less than 64-bits in size, but that
+// don't match the size of a primitive type, and passes them as the 6th
+// parameter so that they are likely to wind up on the stack for ABIs that
+// pass structs by value.
+
+using System;
+using System.Runtime.CompilerServices;
+
+// Struct that's greater than 32-bits, but not a multiple of 32-bits.
+public struct MyStruct1
+{
+ public byte f1;
+ public byte f2;
+ public short f3;
+ public short f4;
+}
+
+// Struct that's less than 32-bits, but not the same size as any primitive type.
+public struct MyStruct2
+{
+ public byte f1;
+ public byte f2;
+ public byte f3;
+}
+
+// Struct that's less than 64-bits, but not the same size as any primitive type.
+public struct MyStruct3
+{
+ public short f1;
+ public short f2;
+ public short f3;
+}
+
+// Struct that's greater than 64-bits, but not a multiple of 64-bits.
+public struct MyStruct4
+{
+ public int f1;
+ public int f2;
+ public short f3;
+}
+
+public class MyProgram
+{
+ const int Pass = 100;
+ const int Fail = -1;
+
+ [MethodImplAttribute(MethodImplOptions.NoInlining)]
+ public static byte GetByte(byte i)
+ {
+ return i;
+ }
+
+ [MethodImplAttribute(MethodImplOptions.NoInlining)]
+ public static short GetShort(short i)
+ {
+ return i;
+ }
+
+ [MethodImplAttribute(MethodImplOptions.NoInlining)]
+ public static int GetInt(int i)
+ {
+ return i;
+ }
+
+ [MethodImplAttribute(MethodImplOptions.NoInlining)]
+ public static int Check1(int w, int i1, int i2, int i3, int i4, int i5, int i6, int i7, MyStruct1 s1)
+ {
+ if ((w != 1) || (s1.f1 != i1) || (s1.f2 != i2) || (s1.f3 != i3) || (s1.f4 != i4))
+ {
+ Console.WriteLine("FAIL");
+ return Fail;
+ }
+ Console.WriteLine("PASS");
+ return Pass;
+ }
+
+ public static int TestStruct1()
+ {
+ MyStruct1 s1;
+ s1.f1 = GetByte(1); s1.f2 = GetByte(2); s1.f3 = GetShort(3); s1.f4 = GetShort(4);
+ int x = (s1.f1 * s1.f2 * s1.f3 * s1.f4);
+ int y = (s1.f1 - s1.f2) * (s1.f3 - s1.f4);
+ int z = (s1.f1 + s1.f2) * (s1.f3 + s1.f4);
+ int w = (x + y) / z;
+
+ return Check1(w, 1, 2, 3, 4, 5, 6, 7, s1);
+ }
+
+ [MethodImplAttribute(MethodImplOptions.NoInlining)]
+ public static int Check2(int w, int i1, int i2, int i3, int i4, int i5, int i6, int i7, MyStruct2 s2)
+ {
+ if ((w != 2) || (s2.f1 != i1) || (s2.f2 != i2) || (s2.f3 != i3) || (i4 != 4))
+ {
+ Console.WriteLine("FAIL");
+ return Fail;
+ }
+ Console.WriteLine("PASS");
+ return Pass;
+ }
+
+ public static int TestStruct2()
+ {
+ MyStruct2 s2;
+ s2.f1 = GetByte(1); s2.f2 = GetByte(2); s2.f3 = GetByte(3);
+ int x = s2.f1 * s2.f2 * s2.f3;
+ int y = (s2.f1 + s2.f2) * s2.f3;
+ int z = s2.f1 + s2.f2 + s2.f3;
+ int w = (x + y) / z;
+
+ return Check2(w, 1, 2, 3, 4, 5, 6, 7, s2);
+ }
+
+ [MethodImplAttribute(MethodImplOptions.NoInlining)]
+ public static int Check3(int w, int i1, int i2, int i3, int i4, int i5, int i6, int i7, MyStruct3 s3)
+ {
+ if ((w != 2) || (s3.f1 != i1) || (s3.f2 != i2) || (s3.f3 != i3) || (i4 != 4))
+ {
+ Console.WriteLine("FAIL");
+ return Fail;
+ }
+ Console.WriteLine("PASS");
+ return Pass;
+ }
+
+ public static int TestStruct3()
+ {
+ MyStruct3 s3;
+ s3.f1 = GetByte(1); s3.f2 = GetByte(2); s3.f3 = GetByte(3);
+ int x = s3.f1 * s3.f2 * s3.f3;
+ int y = (s3.f1 + s3.f2) * s3.f3;
+ int z = s3.f1 + s3.f2 + s3.f3;
+ int w = (x + y) / z;
+
+ return Check3(w, 1, 2, 3, 4, 5, 6, 7, s3);
+ }
+
+ [MethodImplAttribute(MethodImplOptions.NoInlining)]
+ public static int Check4(int w, int i1, int i2, int i3, int i4, int i5, int i6, int i7, MyStruct4 s4)
+ {
+ if ((w != 2) || (s4.f1 != i1) || (s4.f2 != i2) || (s4.f3 != i3) || (i4 != 4))
+ {
+ Console.WriteLine("FAIL");
+ return Fail;
+ }
+ Console.WriteLine("PASS");
+ return Pass;
+ }
+
+ public static int TestStruct4()
+ {
+ MyStruct4 s4;
+ s4.f1 = GetInt(1); s4.f2 = GetInt(2); s4.f3 = GetShort(3);
+ int x = s4.f1 * s4.f2 * s4.f3;
+ int y = (s4.f1 + s4.f2) * s4.f3;
+ int z = s4.f1 + s4.f2 + s4.f3;
+ int w = (x + y) / z;
+
+ return Check4(w, 1, 2, 3, 4, 5, 6, 7, s4);
+ }
+
+ public static int Main()
+ {
+ int retVal = Pass;
+ if (TestStruct1() != Pass)
+ {
+ retVal = Fail;
+ }
+ if (TestStruct2() != Pass)
+ {
+ retVal = Fail;
+ }
+ if (TestStruct3() != Pass)
+ {
+ retVal = Fail;
+ }
+ if (TestStruct4() != Pass)
+ {
+ retVal = Fail;
+ }
+ return retVal;
+ }
+}
--- /dev/null
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+ <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+ <PropertyGroup>
+ <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+ <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+ <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+ <SchemaVersion>2.0</SchemaVersion>
+ <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+ <OutputType>Exe</OutputType>
+ <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+ <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+ </PropertyGroup>
+ <!-- Default configurations to help VS understand the configurations -->
+ <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+ </PropertyGroup>
+ <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+ </PropertyGroup>
+ <ItemGroup>
+ <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+ <Visible>False</Visible>
+ </CodeAnalysisDependentAssemblyPaths>
+ </ItemGroup>
+ <PropertyGroup>
+ <DebugType></DebugType>
+ <Optimize>True</Optimize>
+ </PropertyGroup>
+ <ItemGroup>
+ <Compile Include="$(MSBuildProjectName).cs" />
+ </ItemGroup>
+ <ItemGroup>
+ <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+ </ItemGroup>
+ <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+ <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+ </PropertyGroup>
+</Project>
projects / platform / upstream / coreclr.git / commitdiff