summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormikedn <onemihaid@hotmail.com>2019-04-13 20:39:20 +0300
committerBruce Forstall <brucefo@microsoft.com>2019-04-13 10:39:20 -0700
commit8ce1e716841e4a9d6c54be45eb75e740872a4b3e (patch)
treef504a4e57155260099fcb439eeb5202b0025e703
parent3bb584305549f865af443472100641de7b5d848e (diff)
downloadcoreclr-8ce1e716841e4a9d6c54be45eb75e740872a4b3e.tar.gz
coreclr-8ce1e716841e4a9d6c54be45eb75e740872a4b3e.tar.bz2
coreclr-8ce1e716841e4a9d6c54be45eb75e740872a4b3e.zip
Fix ARM's genPutArgStk codegen (#23867)
* Fix ARM's genPutArgStk codegen When the OBJ node wraps a LCL_VAR node the code uses the type information (struct size, GC layout) from LclVarDsc. This is not always correct because the OBJ may actually have a different struct type due to type reinterpretation (e.g. Unsafe.As<X, Y>). * Fix genPutArgStk comment
-rw-r--r--src/jit/codegenarmarch.cpp32
-rw-r--r--tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.cs58
-rw-r--r--tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.csproj17
3 files changed, 105 insertions, 2 deletions
diff --git a/src/jit/codegenarmarch.cpp b/src/jit/codegenarmarch.cpp
index add7b9867d..66d5b22031 100644
--- a/src/jit/codegenarmarch.cpp
+++ b/src/jit/codegenarmarch.cpp
@@ -714,7 +714,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
BYTE* gcPtrs = gcPtrArray;
unsigned gcPtrCount; // The count of GC pointers in the struct
- int structSize;
+ unsigned structSize;
bool isHfa;
// This is the varNum for our load operations,
@@ -764,12 +764,40 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
hiReg = addrReg;
}
#endif // _TARGET_ARM64_
+ }
+ if (source->OperIs(GT_OBJ))
+ {
+ // If the source is an OBJ node then we need to use the type information
+ // it provides (size and GC layout) even if the node wraps a lclvar. Due
+ // to struct reinterpretation (e.g. Unsafe.As<X, Y>) it is possible that
+ // the OBJ node has a different type than the lclvar.
CORINFO_CLASS_HANDLE objClass = source->gtObj.gtClass;
structSize = compiler->info.compCompHnd->getClassSize(objClass);
- isHfa = compiler->IsHfa(objClass);
+
+ // The codegen code below doesn't have proper support for struct sizes
+ // that are not multiple of the slot size. Call arg morphing handles this
+ // case by copying non-local values to temporary local variables.
+ // More generally, we can always round up the struct size when the OBJ node
+ // wraps a local variable because the local variable stack allocation size
+ // is also rounded up to be a multiple of the slot size.
+ if (varNode != nullptr)
+ {
+ structSize = roundUp(structSize, TARGET_POINTER_SIZE);
+ }
+ else
+ {
+ assert((structSize % TARGET_POINTER_SIZE) == 0);
+ }
+
+ isHfa = compiler->IsHfa(objClass);
+
#ifdef _TARGET_ARM64_
+ // On ARM32, Lowering places the correct GC layout information in the
+ // GenTreePutArgStk node and the code above already use that. On ARM64,
+ // this information is not available (in order to keep GenTreePutArgStk
+ // nodes small) and we need to retrieve it from the VM here.
gcPtrCount = compiler->info.compCompHnd->getClassGClayout(objClass, &gcPtrs[0]);
#endif
}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.cs b/tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.cs
new file mode 100644
index 0000000000..bfa6fcc492
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.cs
@@ -0,0 +1,58 @@
+// 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.
+
+using System;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+class Program
+{
+ [StructLayout(LayoutKind.Sequential)]
+ struct S
+ {
+ public uint i0;
+ public uint i1;
+ public uint i2;
+ public uint i3;
+
+ public int i4;
+ public int i5;
+ }
+
+ [StructLayout(LayoutKind.Sequential)]
+ struct S16
+ {
+ public uint i0;
+ public uint i1;
+ public uint i2;
+ public uint i3;
+ }
+
+ static int Main()
+ {
+ S s = new S();
+ s.i0 = 0x12345678;
+ s.i1 = 0x87654321;
+ return Test(s);
+ }
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static int Call(int r0, int r1, int r2, int r3, int r4, int r5, int r6, S16 s)
+ {
+ return (s.i0 == 0x12345678 && s.i1 == 0x87654321) ? 100 : 1;
+ }
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static void Escape<T>(ref T t)
+ {
+ }
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static int Test(S p)
+ {
+ S s = p;
+ Escape(ref s);
+ return Call(0, 1, 2, 3, 4, 5, 6, Unsafe.As<S, S16>(ref s));
+ }
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.csproj
new file mode 100644
index 0000000000..83594da8fa
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.csproj
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+ <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+ <PropertyGroup>
+ <Configuration Condition=" '$(Configuration)' == '' ">Release</Configuration>
+ <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+ <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+ <OutputType>Exe</OutputType>
+ <DebugType></DebugType>
+ <Optimize>True</Optimize>
+ </PropertyGroup>
+ <ItemGroup>
+ <Compile Include="$(MSBuildProjectName).cs" />
+ </ItemGroup>
+ <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+ <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>