Fix Issue 6585: Add offset to struct field arg (dotnet/coreclr#6616)

When the child of a GT_OBJ is a lclFldAddr, genConsumePutStructArgStk()
was not adding the offset of the struct field to the base address of
the local.

Commit migrated from https://github.com/dotnet/coreclr/commit/1608c2c68af836736cbeff81cdd10212666a17cd

diff --git a/src/coreclr/src/jit/codegenxarch.cpp b/src/coreclr/src/jit/codegenxarch.cpp
index 78a8db7..4706017 100755 (executable)
--- a/src/coreclr/src/jit/codegenxarch.cpp
+++ b/src/coreclr/src/jit/codegenxarch.cpp
@@ -5264,13 +5264,16 @@ void CodeGen::genConsumeOperands(GenTreeOp* tree)
 //    dstReg     - the dstReg for the rep move operation.
 //    srcReg     - the srcReg for the rep move operation.
 //    sizeReg    - the sizeReg for the rep move operation.
-//    baseVarNum - the base for var numfor placing the "by-value" args on the stack.
+//    baseVarNum - the varnum for the local used for placing the "by-value" args on the stack.
 //
 // Return Value:
 //    None.
 //
 // Note: sizeReg can be REG_NA when this function is used to consume the dstReg and srcReg
 //           for copying on the stack a struct with references.
+//       The source address/offset is determined from the address on the GT_OBJ node, while
+//       the destination address is the address contained in 'baseVarNum' plus the offset
+//       provided in the 'putArgNode'. 
 
 void CodeGen::genConsumePutStructArgStk(GenTreePutArgStk* putArgNode, regNumber dstReg, regNumber srcReg, regNumber sizeReg, unsigned baseVarNum)
 {
@@ -5280,32 +5283,27 @@ void CodeGen::genConsumePutStructArgStk(GenTreePutArgStk* putArgNode, regNumber
     // The putArgNode children are always contained. We should not consume any registers.
     assert(putArgNode->gtGetOp1()->isContained());
 
-    GenTree* dst = putArgNode;
+    GenTree* dstAddr = putArgNode;
 
-    // Get the GT_ADDR node, which is GT_LCL_VAR_ADDR (asserted below.)
+    // Get the source address.
     GenTree* src = putArgNode->gtGetOp1();
     assert((src->gtOper == GT_OBJ) || ((src->gtOper == GT_IND && varTypeIsSIMD(src))));
-    src = src->gtGetOp1();
+    GenTree* srcAddr = src->gtGetOp1();
     
     size_t size = putArgNode->getArgSize();
-    GenTree* op1;
-    GenTree* op2;
-
-    op1 = dst;
-    op2 = src;
 
     assert(dstReg != REG_NA);
     assert(srcReg != REG_NA);
 
     // Consume the registers only if they are not contained or set to REG_NA.
-    if (op2->gtRegNum != REG_NA)
+    if (srcAddr->gtRegNum != REG_NA)
     {
-        genConsumeReg(op2);
+        genConsumeReg(srcAddr);
     }
 
     // If the op1 is already in the dstReg - nothing to do.
     // Otherwise load the op1 (GT_ADDR) into the dstReg to copy the struct on the stack by value.
-    if (op1->gtRegNum != dstReg)
+    if (dstAddr->gtRegNum != dstReg)
     {
         // Generate LEA instruction to load the stack of the outgoing var + SlotNum offset (or the incoming arg area
         // for tail calls) in RDI.
@@ -5313,23 +5311,28 @@ void CodeGen::genConsumePutStructArgStk(GenTreePutArgStk* putArgNode, regNumber
         getEmitter()->emitIns_R_S(INS_lea, EA_PTRSIZE, dstReg, baseVarNum, putArgNode->getArgOffset());
     }
     
-    if (op2->gtRegNum != srcReg)
+    if (srcAddr->gtRegNum != srcReg)
     {
-        if (src->OperIsLocalAddr())
+        if (srcAddr->OperIsLocalAddr())
         {
             // The OperLocalAddr is always contained.
-            assert(src->isContained());
-            GenTreeLclVarCommon* lclNode = src->AsLclVarCommon();
+            assert(srcAddr->isContained());
+            GenTreeLclVarCommon* lclNode = srcAddr->AsLclVarCommon();
 
             // Generate LEA instruction to load the LclVar address in RSI.
             // Source is known to be on the stack. Use EA_PTRSIZE.
-            getEmitter()->emitIns_R_S(INS_lea, EA_PTRSIZE, srcReg, lclNode->gtLclNum, 0);
+            unsigned int offset = 0;
+            if (srcAddr->OperGet() == GT_LCL_FLD_ADDR)
+            {
+                offset = srcAddr->AsLclFld()->gtLclOffs;
+            }
+            getEmitter()->emitIns_R_S(INS_lea, EA_PTRSIZE, srcReg, lclNode->gtLclNum, offset);
         }
         else
         {
-            assert(src->gtRegNum != REG_NA);
+            assert(srcAddr->gtRegNum != REG_NA);
             // Source is not known to be on the stack. Use EA_BYREF.
-            getEmitter()->emitIns_R_R(INS_mov, EA_BYREF, srcReg, src->gtRegNum);
+            getEmitter()->emitIns_R_R(INS_mov, EA_BYREF, srcReg, srcAddr->gtRegNum);
         }
     }
 

diff --git a/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam.cs b/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam.cs
new file mode 100755 (executable)
index 0000000..e2b6515
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam.cs
@@ -0,0 +1,141 @@
+// 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 is a repro for Issue6585. The problem was that the source of
+// a GT_OBJ (struct argument) node was a lclFldAddr, and codegen was
+// treating it as a lclVarAddr, i.e. not adding in the offset.
+// The inner struct must either be larger than CPBLK_UNROLL_LIMIT,
+// which is currently 64 bytes, or must contain GC refs.
+
+using System;
+using System.Runtime.CompilerServices;
+namespace structfieldparam
+{
+
+    struct Inner1
+    {
+        public long l1;
+        public long l2;
+        public long l3;
+        public long l4;
+        public long l5;
+        public long l6;
+        public long l7;
+        public long l8;
+        public long[] arr;
+
+        public Inner1(int seed)
+        {
+            l1 = seed;
+            l2 = seed + 1;
+            l3 = seed + 2;
+            l4 = seed + 3;
+            l5 = seed + 4;
+            l6 = seed + 5;
+            l7 = seed + 6;
+            l8 = seed + 7;
+            arr = new long[4];
+        }
+
+        [MethodImplAttribute(MethodImplOptions.NoInlining)]
+        public long sum()
+        {
+            return l1 + l2 + l3 + l4 + l5 + l6 + l7 + l8;
+        }
+    }
+
+    struct Outer1
+    {
+        public int i1;
+        public long l1;
+        public Inner1 inner;
+    }
+
+    struct Inner2
+    {
+        public long l1;
+        public long l2;
+        public long l3;
+        public long l4;
+        public long l5;
+        public long l6;
+        public long l7;
+        public long l8;
+        public long l9;
+
+        public Inner2(int seed)
+        {
+            l1 = seed;
+            l2 = seed + 1;
+            l3 = seed + 2;
+            l4 = seed + 3;
+            l5 = seed + 4;
+            l6 = seed + 5;
+            l7 = seed + 6;
+            l8 = seed + 7;
+            l9 = seed + 8;
+        }
+
+        [MethodImplAttribute(MethodImplOptions.NoInlining)]
+        public long sum()
+        {
+            return l1 + l2 + l3 + l4 + l5 + l6 + l7 + l8 + l9;
+        }
+    }
+
+    struct Outer2
+    {
+        public int i1;
+        public long l1;
+        public Inner2 inner;
+    }
+
+    public class Program
+    {
+        [MethodImplAttribute(MethodImplOptions.NoInlining)]
+        static long test1(Inner1 s)
+        {
+            return s.sum();
+        }
+
+        [MethodImplAttribute(MethodImplOptions.NoInlining)]
+        static long test2(Inner2 s)
+        {
+            return s.sum();
+        }
+
+        public static int Main()
+        {
+            int result = 100;
+
+            Inner1 t1 = new Inner1(10);
+            Outer1 o1;
+            o1.i1 = 1;
+            o1.l1 = 2;
+            o1.inner = t1;
+            long direct = t1.sum();
+            long indirect = test1(o1.inner);
+            if (direct != indirect)
+            {
+                Console.WriteLine("t1.sum() returns " + direct + ", but test(o1.inner) returns " + indirect);
+                result = -1;
+            }
+
+            Inner2 t2 = new Inner2(10);
+            Outer2 o2;
+            o2.i1 = 1;
+            o2.l1 = 2;
+            o2.inner = t2;
+            direct = t2.sum();
+            indirect = test2(o2.inner);
+            if (direct != indirect)
+            {
+                Console.WriteLine("t2.sum() returns " + direct + ", but test(o2.inner) returns " + indirect);
+                result = -1;
+            }
+
+            return result;
+        }
+    }
+}

diff --git a/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam_r.csproj b/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam_r.csproj
new file mode 100644 (file)
index 0000000..35be9e7
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam_r.csproj
@@ -0,0 +1,49 @@
+<?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>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+  </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>None</DebugType>
+    <Optimize>False</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="structfieldparam.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <None Include="$(JitPackagesConfigFileDirectory)minimal\project.json" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <PropertyGroup>
+    <ProjectJson>$(JitPackagesConfigFileDirectory)minimal\project.json</ProjectJson>
+    <ProjectLockJson>$(JitPackagesConfigFileDirectory)minimal\project.lock.json</ProjectLockJson>
+  </PropertyGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup> 
+</Project>

diff --git a/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam_ro.csproj b/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam_ro.csproj
new file mode 100644 (file)
index 0000000..d3f8f04
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Directed/StructABI/structfieldparam_ro.csproj
@@ -0,0 +1,49 @@
+<?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>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+  </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>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="structfieldparam.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <None Include="$(JitPackagesConfigFileDirectory)minimal\project.json" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <PropertyGroup>
+    <ProjectJson>$(JitPackagesConfigFileDirectory)minimal\project.json</ProjectJson>
+    <ProjectLockJson>$(JitPackagesConfigFileDirectory)minimal\project.lock.json</ProjectLockJson>
+  </PropertyGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup> 
+</Project>