diff options
| -rw-r--r-- | src/jit/morph.cpp | 55 | ||||
| -rw-r--r-- | tests/arm/Tests.lst | 7 | ||||
| -rw-r--r-- | tests/arm64/Tests.lst | 8 | ||||
| -rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_19288/GitHub_19288.cs | 60 | ||||
| -rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_19288/GitHub_19288.csproj | 35 |
5 files changed, 141 insertions, 24 deletions
diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 990c0a8aa4..7c093c4b46 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -3637,36 +3637,43 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) structBaseType = getArgTypeForStruct(objClass, &howToPassStruct, callIsVararg, originalSize); -#if defined(_TARGET_ARM64_) || defined(UNIX_AMD64_ABI) - // For ARM64 or AMD64/UX we can pass non-power-of-2 structs in a register. - if ((howToPassStruct == SPK_PrimitiveType) && // Passed in a single register - !isPow2(originalSize)) // size is 3,5,6 or 7 bytes - { - originalSize = genTypeSize(structBaseType); - } -#endif // _TARGET_ARM64_ || UNIX_AMD64_ABI + bool passStructByRef = false; + unsigned passingSize = originalSize; - bool passStructByRef = false; +#ifndef _TARGET_X86_ // Check to see if we can transform this struct load (GT_OBJ) into a GT_IND of the appropriate size. // That is the else clause of the if statement below. // When it can do this is platform-dependent: - // - In general, it can be done for power of 2 structs that fit in a single register - // (or, for ARM64 and AMD64/UX, lclVars that are less than pointer size, see above). + // - In general, it can be done for power of 2 structs that fit in a single register. // - For ARM and ARM64 it must also be a non-HFA struct, or have a single field. // - This is irrelevant for X86, since structs are always passed by value on the stack. // Note that 'howToPassStruct' captures all but the power-of-2 requirement. - CLANG_FORMAT_COMMENT_ANCHOR; -#ifndef _TARGET_X86_ - // Check for struct argument with size 1, 2, 4 or 8 bytes - // As we can optimize these by turning them into a GT_IND of the correct type - // - // Check for cases that we cannot optimize: - bool canTransformToInd = (howToPassStruct == SPK_PrimitiveType) && isPow2(originalSize); + GenTree* lclVar = fgIsIndirOfAddrOfLocal(argObj); + bool canTransformToInd = false; + if (howToPassStruct == SPK_PrimitiveType) + { + if (isPow2(passingSize)) + { + canTransformToInd = true; + } + +#if defined(_TARGET_ARM64_) || defined(UNIX_AMD64_ABI) + // For ARM64 or AMD64/UX we can pass non-power-of-2 structs in a register, but we can + // only transform to an indirection in that case if we are loading from a local. + // TODO-CQ: This transformation should be applicable in general, not just for the ARM64 + // or UNIX_AMD64_ABI cases where they will be passed in registers. + else + { + canTransformToInd = (lclVar != nullptr); + passingSize = genTypeSize(structBaseType); + } +#endif // _TARGET_ARM64_ || UNIX_AMD64_ABI + } + if (!canTransformToInd) { - GenTree* lclVar = fgIsIndirOfAddrOfLocal(argObj); // Normalize 'size' to the number of pointer sized items // 'size' is the number of register slots that we will use to pass the argument size = roundupSize / TARGET_POINTER_SIZE; @@ -3693,7 +3700,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) { copyBlkClass = objClass; } - else if (originalSize != structSize) + else if (passingSize != structSize) { copyBlkClass = objClass; } @@ -3714,7 +3721,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) passStructByRef = true; copyBlkClass = objClass; } - else if ((originalSize != structSize) && (lclVar == nullptr)) + else if ((passingSize != structSize) && (lclVar == nullptr)) { copyBlkClass = objClass; } @@ -3751,7 +3758,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) assert(howToPassStruct == SPK_PrimitiveType); noway_assert(structBaseType != TYP_UNKNOWN); - assert(originalSize == genTypeSize(structBaseType)); + assert(passingSize == genTypeSize(structBaseType)); argObj->ChangeOper(GT_IND); @@ -3787,13 +3794,13 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) { // get the first and only promoted field LclVarDsc* fieldVarDsc = &lvaTable[varDsc->lvFieldLclStart]; - if (genTypeSize(fieldVarDsc->TypeGet()) >= originalSize) + if (genTypeSize(fieldVarDsc->TypeGet()) >= passingSize) { // we will use the first and only promoted field argObj->gtLclVarCommon.SetLclNum(varDsc->lvFieldLclStart); if (varTypeCanReg(fieldVarDsc->TypeGet()) && - (genTypeSize(fieldVarDsc->TypeGet()) == originalSize)) + (genTypeSize(fieldVarDsc->TypeGet()) == passingSize)) { // Just use the existing field's type argObj->gtType = fieldVarDsc->TypeGet(); diff --git a/tests/arm/Tests.lst b/tests/arm/Tests.lst index 49a092c005..9b5576ebf5 100644 --- a/tests/arm/Tests.lst +++ b/tests/arm/Tests.lst @@ -94828,3 +94828,10 @@ MaxAllowedDurationSeconds=600 Categories=EXPECTED_PASS HostStyle=0 +[GitHub_19288.cmd_11915] +RelativePath=JIT\Regression\JitBlue\GitHub_19288\GitHub_19288\GitHub_19288.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_19288\GitHub_19288 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 diff --git a/tests/arm64/Tests.lst b/tests/arm64/Tests.lst index c89410a8da..20df01b753 100644 --- a/tests/arm64/Tests.lst +++ b/tests/arm64/Tests.lst @@ -94843,3 +94843,11 @@ Expected=0 MaxAllowedDurationSeconds=600 Categories=EXPECTED_PASS HostStyle=0 + +[GitHub_19288.cmd_12234] +RelativePath=JIT\Regression\JitBlue\GitHub_19288\GitHub_19288\GitHub_19288.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_19288\GitHub_19288 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_19288/GitHub_19288.cs b/tests/src/JIT/Regression/JitBlue/GitHub_19288/GitHub_19288.cs new file mode 100644 index 0000000000..8aac6a3081 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_19288/GitHub_19288.cs @@ -0,0 +1,60 @@ +// 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 test exhibits a case where the JIT was passing a 3-byte struct +// by loading 4-bytes, which is not always safe. + +using System; +using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; + +public class GitHub_19288 +{ + static int returnVal = 100; + + [StructLayout(LayoutKind.Sequential)] + public struct PixelData + { + public byte blue; + public byte green; + public byte red; + } + + public unsafe class MyClass + { + [MethodImpl(MethodImplOptions.NoInlining)] + unsafe void CheckPointer(PixelData p) + { + if (&p == null) + { + Console.WriteLine("FAIL"); + returnVal = -1; + } + } + + unsafe int DoStuff() + { + IntPtr pBase = Marshal.AllocCoTaskMem(0x40000 * 3); + PixelData* foo = (PixelData*)(pBase + 511 * (512 * sizeof(PixelData)) + 511 * sizeof(PixelData)); + + CheckPointer(*foo); + + return 0; + } + + static int Main() + { + try + { + new MyClass().DoStuff(); + } + catch (Exception e) + { + Console.WriteLine("Failed with exception: " + e.Message); + returnVal = -1; + } + return returnVal; + } + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_19288/GitHub_19288.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_19288/GitHub_19288.csproj new file mode 100644 index 0000000000..7e73aa0a21 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_19288/GitHub_19288.csproj @@ -0,0 +1,35 @@ +<?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>{ADEEA3D1-B67B-456E-8F2B-6DCCACC2D34C}</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>Full</DebugType> + <Optimize>True</Optimize> + <AllowUnsafeBlocks>True</AllowUnsafeBlocks> + </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> |