Fix incoming arg struct splitting (#71701)
authorBruce Forstall <brucefo@microsoft.com>
Thu, 7 Jul 2022 16:14:07 +0000 (09:14 -0700)
committerGitHub <noreply@github.com>
Thu, 7 Jul 2022 16:14:07 +0000 (09:14 -0700)
This change only affects Windows/arm64 with varargs.

For incoming structs split between register and stack, handle the splitting
correctly.

Fixes #57606

src/coreclr/jit/codegencommon.cpp
src/tests/JIT/Regression/JitBlue/Runtime_57606/Runtime_57606.cs [new file with mode: 0644]
src/tests/JIT/Regression/JitBlue/Runtime_57606/Runtime_57606.csproj [new file with mode: 0644]
src/tests/issues.targets

index 9a49677..72f2dd5 100644 (file)
@@ -3155,24 +3155,18 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
         {
             // Bingo - add it to our table
             regArgNum = genMapRegNumToRegArgNum(varDsc->GetArgReg(), regType);
+            slots     = 1;
 
-            noway_assert(regArgNum < argMax);
-            // We better not have added it already (there better not be multiple vars representing this argument
-            // register)
-            noway_assert(regArgTab[regArgNum].slot == 0);
-
-#if defined(UNIX_AMD64_ABI)
-            // Set the register type.
-            regArgTab[regArgNum].type = regType;
-#endif // defined(UNIX_AMD64_ABI)
-
-            regArgTab[regArgNum].varNum = varNum;
-            regArgTab[regArgNum].slot   = 1;
-
-            slots = 1;
-
+            if (TargetArchitecture::IsArm32)
+            {
+                int lclSize = compiler->lvaLclSize(varNum);
+                if (lclSize > REGSIZE_BYTES)
+                {
+                    slots = lclSize / REGSIZE_BYTES;
+                }
+            }
 #if FEATURE_MULTIREG_ARGS
-            if (compiler->lvaIsMultiregStruct(varDsc, compiler->info.compIsVarArgs))
+            else if (compiler->lvaIsMultiregStruct(varDsc, compiler->info.compIsVarArgs))
             {
                 if (varDsc->lvIsHfaRegArg())
                 {
@@ -3186,47 +3180,40 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
                     // We have a non-HFA multireg argument, set slots to two
                     slots = 2;
                 }
+            }
+#endif // FEATURE_MULTIREG_ARGS
 
-                // Note that regArgNum+1 represents an argument index not an actual argument register.
-                // see genMapRegArgNumToRegNum(unsigned argNum, var_types type)
-
-                // This is the setup for the rest of a multireg struct arg
-
-                for (int i = 1; i < slots; i++)
+            // Handle args split between registers and stack. The arm64 fixed ret buf arg is never split.
+            if (compFeatureArgSplit() && (fixedRetBufIndex != regArgNum))
+            {
+                unsigned maxRegArgNum = doingFloat ? MAX_FLOAT_REG_ARG : MAX_REG_ARG;
+                if (regArgNum + slots > maxRegArgNum)
                 {
-                    noway_assert((regArgNum + i) < argMax);
-
-                    // We better not have added it already (there better not be multiple vars representing this argument
-                    // register)
-                    noway_assert(regArgTab[regArgNum + i].slot == 0);
-
-                    regArgTab[regArgNum + i].varNum = varNum;
-                    regArgTab[regArgNum + i].slot   = (char)(i + 1);
+                    JITDUMP("Splitting V%02u: %u registers, %u stack slots\n", varNum, maxRegArgNum - regArgNum,
+                            regArgNum + slots - maxRegArgNum);
+                    slots = maxRegArgNum - regArgNum;
                 }
             }
-#endif // FEATURE_MULTIREG_ARGS
-        }
 
-#ifdef TARGET_ARM
-        int lclSize = compiler->lvaLclSize(varNum);
+            // Note that regArgNum + 1 represents an argument index not an actual argument register;
+            // see genMapRegArgNumToRegNum().
 
-        if (lclSize > REGSIZE_BYTES)
-        {
-            unsigned maxRegArgNum = doingFloat ? MAX_FLOAT_REG_ARG : MAX_REG_ARG;
-            slots                 = lclSize / REGSIZE_BYTES;
-            if (regArgNum + slots > maxRegArgNum)
+            for (int i = 0; i < slots; i++)
             {
-                slots = maxRegArgNum - regArgNum;
+                noway_assert((regArgNum + i) < argMax);
+
+                // We better not have added it already (there better not be multiple vars representing this argument
+                // register)
+                noway_assert(regArgTab[regArgNum + i].slot == 0);
+
+                regArgTab[regArgNum + i].varNum = varNum;
+                regArgTab[regArgNum + i].slot   = static_cast<char>(i + 1);
+
+#if defined(UNIX_AMD64_ABI)
+                regArgTab[regArgNum + i].type = regType; // Set the register type.
+#endif                                                   // defined(UNIX_AMD64_ABI)
             }
         }
-        C_ASSERT((char)MAX_REG_ARG == MAX_REG_ARG);
-        assert(slots < INT8_MAX);
-        for (int i = 1; i < slots; i++)
-        {
-            regArgTab[regArgNum + i].varNum = varNum;
-            regArgTab[regArgNum + i].slot   = static_cast<char>(i + 1);
-        }
-#endif // TARGET_ARM
 
         for (int i = 0; i < slots; i++)
         {
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57606/Runtime_57606.cs b/src/tests/JIT/Regression/JitBlue/Runtime_57606/Runtime_57606.cs
new file mode 100644 (file)
index 0000000..a65fe93
--- /dev/null
@@ -0,0 +1,40 @@
+// 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.CompilerServices;
+
+class Runtime_57606
+{
+    public struct CompositeType16Bytes
+    {
+        public ulong _0;
+        public ulong _1;
+    }
+
+    public struct CompositeTypeMoreThan16Bytes
+    {
+        public ulong _0;
+        public ulong _1;
+        public byte _2;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static CompositeTypeMoreThan16Bytes ReturnsViaBuffer(int x1, int x2, int x3, int x4, int x5, int x6, CompositeType16Bytes x7Stack, __arglist)
+    {
+        // Note that VarArgHnd is passed in register x0
+        // When allocating a parameter of CompositeType16Bytes to registers and stack
+        // NGRN = 7 and since the value can not be allocated to a single GP register
+        // the JIT splits the value between x7 and stack.
+
+        CompositeTypeMoreThan16Bytes r = default;
+        r._2 = (byte)(x1 + x2 + x3 + x4 + x5 + x6 + (int)x7Stack._0 + (int)x7Stack._1 + 79);
+        return r;
+    }
+
+    public static int Main()
+    {
+        CompositeTypeMoreThan16Bytes r = ReturnsViaBuffer(1, 2, 3, 4, 5, 6, default(CompositeType16Bytes), __arglist());
+        return r._2;
+    }
+}
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_57606/Runtime_57606.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_57606/Runtime_57606.csproj
new file mode 100644 (file)
index 0000000..6946bed
--- /dev/null
@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
index 992e462..8db949c 100644 (file)
@@ -67,6 +67,9 @@
          <ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/arglist/vararg_TargetUnix/*">
             <Issue>https://github.com/dotnet/runtime/issues/10478 </Issue>
         </ExcludeList>
+        <ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_57606/Runtime_57606/*">
+            <Issue>https://github.com/dotnet/runtime/issues/10478</Issue>
+        </ExcludeList>
     </ItemGroup>
 
     <!-- All Unix targets  on CoreCLR Runtime -->