diff options
author | mikedn <onemihaid@hotmail.com> | 2019-04-13 20:39:20 +0300 |
---|---|---|
committer | Bruce Forstall <brucefo@microsoft.com> | 2019-04-13 10:39:20 -0700 |
commit | 8ce1e716841e4a9d6c54be45eb75e740872a4b3e (patch) | |
tree | f504a4e57155260099fcb439eeb5202b0025e703 | |
parent | 3bb584305549f865af443472100641de7b5d848e (diff) | |
download | coreclr-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.cpp | 32 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.cs | 58 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_23794/GitHub_23794.csproj | 17 |
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> |