diff options
author | Eugene Rozenfeld <erozen@microsoft.com> | 2016-06-02 11:46:43 -0700 |
---|---|---|
committer | Eugene Rozenfeld <erozen@microsoft.com> | 2016-06-02 11:46:43 -0700 |
commit | f22b221718f3a243d4fc3fba4e037c2a9524718c (patch) | |
tree | bc157c7d42afa638618b8d66f93762dc260cd287 | |
parent | 3d53df24c366c8f0c44196039279d34f78f72b3e (diff) | |
parent | 3a97871a149dc116c80710ff51ecc85f62763909 (diff) | |
download | coreclr-f22b221718f3a243d4fc3fba4e037c2a9524718c.tar.gz coreclr-f22b221718f3a243d4fc3fba4e037c2a9524718c.tar.bz2 coreclr-f22b221718f3a243d4fc3fba4e037c2a9524718c.zip |
Merge pull request #5394 from erozenfeld/TailCallBug5164
Fix for methods that tail call via helpers
-rw-r--r-- | src/jit/morph.cpp | 11 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_5164/GitHub_5164.il | 150 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_5164/GitHub_5164.ilproj | 44 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_5164/app.config | 27 |
4 files changed, 226 insertions, 6 deletions
diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index e09e96baec..1cd9eaba5b 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -4875,7 +4875,11 @@ Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, if (lvaIsImplicitByRefLocal(varNum)) { LclVarDsc* varDsc = &lvaTable[varNum]; - if (varDsc->lvRefCnt == 1 && !fgMightHaveLoop()) + // JIT_TailCall helper has an implicit assumption that all tail call arguments live + // on the caller's frame. If an argument lives on the caller caller's frame, it may get + // overwritten if that frame is reused for the tail call. Therefore, we should always copy + // struct parameters if they are passed as arguments to a tail call. + if (!call->IsTailCallViaHelper() && (varDsc->lvRefCnt == 1) && !fgMightHaveLoop()) { varDsc->lvRefCnt = 0; args->gtOp.gtOp1 = lcl; @@ -4883,13 +4887,8 @@ Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, fp->node = lcl; JITDUMP("did not have to make outgoing copy for V%2d", varNum); - varDsc->lvRefCnt = 0; return; } - else - { - varDsc->lvRefCnt = 0; - } } } } diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_5164/GitHub_5164.il b/tests/src/JIT/Regression/JitBlue/GitHub_5164/GitHub_5164.il new file mode 100644 index 0000000000..d2cb5bd962 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_5164/GitHub_5164.il @@ -0,0 +1,150 @@ +// 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. + +// The tail call helper JIT_TailCall has an implicit assumption that all +// arguments of the tail call are allocated on the caller's frame. +// In this test A tail-calls B and B tail-calls A: Main --> A --> B --> A --> B --> A. +// In the last B --> A tail call the frame from the preceding A --> B tail call is resused. +// It's big enough because of the first B --> A tail call. +// The bug was that the code for B wasn't copying the incoming struct parameter to its frame. +// The struct lived on the caller's frame and when it was reused for the B --> A tail call, +// the struct was overwritten. + +.assembly extern mscorlib { } + +.assembly GitHub_5164 { } + +.class public sequential ansi sealed beforefieldinit LargeStruct + extends [mscorlib]System.ValueType +{ + .field public int64 l1 + .field public int64 l2 + .method public hidebysig specialname rtspecialname + instance void .ctor(int64 l1, + int64 l2) cil managed + { + // Code size 15 (0xf) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld int64 LargeStruct::l1 + IL_0007: ldarg.0 + IL_0008: ldarg.2 + IL_0009: stfld int64 LargeStruct::l2 + IL_000e: ret + } // end of method LargeStruct::.ctor + +} // end of class LargeStruct + +.class private auto ansi beforefieldinit Test + extends [mscorlib]System.Object +{ + .method public hidebysig static int32 Main() cil managed + { + .entrypoint + // Code size 48 (0x30) + .maxstack 3 + .locals init (valuetype LargeStruct V_0, + valuetype LargeStruct V_1, + valuetype LargeStruct V_2) + IL_0000: ldloca.s V_0 + IL_0002: ldc.i4.1 + IL_0003: conv.i8 + IL_0004: ldc.i4.2 + IL_0005: conv.i8 + IL_0006: call instance void LargeStruct::.ctor(int64, + int64) + IL_000b: ldloca.s V_1 + IL_000d: ldc.i4.3 + IL_000e: conv.i8 + IL_000f: ldc.i4.4 + IL_0010: conv.i8 + IL_0011: call instance void LargeStruct::.ctor(int64, + int64) + IL_0016: ldc.i4.0 + IL_0017: ldloc.0 + IL_0018: ldloc.1 + IL_0019: call valuetype LargeStruct Test::A(int32, + valuetype LargeStruct, + valuetype LargeStruct) + IL_001e: stloc.2 + IL_001f: ldloca.s V_2 + IL_0021: ldfld int64 LargeStruct::l1 + IL_0026: ldloca.s V_2 + IL_0028: ldfld int64 LargeStruct::l2 + IL_002d: add + IL_002e: conv.i4 + IL_002f: ret + } // end of method Test::Main + + .method public hidebysig static valuetype LargeStruct + A(int32 count, + valuetype LargeStruct s1, + valuetype LargeStruct s2) cil managed noinlining + { + // Code size 58 (0x3a) + .maxstack 4 + .locals init (valuetype LargeStruct V_0) + IL_0000: ldloca.s V_0 + IL_0002: ldarga.s s1 + IL_0004: ldfld int64 LargeStruct::l1 + IL_0009: ldarga.s s2 + IL_000b: ldfld int64 LargeStruct::l1 + IL_0010: add + IL_0011: ldarga.s s1 + IL_0013: ldfld int64 LargeStruct::l2 + IL_0018: ldarga.s s2 + IL_001a: ldfld int64 LargeStruct::l2 + IL_001f: add + IL_0020: call instance void LargeStruct::.ctor(int64, + int64) + IL_0025: ldarg.0 + IL_0026: ldc.i4.3 + IL_0027: bge.s IL_0038 + + IL_0029: ldarg.0 + IL_002a: ldc.i4.1 + IL_002b: add + IL_002c: dup + IL_002d: starg.s count + IL_002f: ldloc.0 + IL_0030: tail. + IL_0032: call valuetype LargeStruct Test::B(int32, + valuetype LargeStruct) + IL_0037: ret + + IL_0038: ldloc.0 + IL_0039: ret + } // end of method Test::A + + .method public hidebysig static valuetype LargeStruct + B(int32 count, + valuetype LargeStruct s) cil managed noinlining + { + // Code size 28 (0x1c) + .maxstack 3 + .locals init (valuetype LargeStruct V_0) + IL_0000: ldloca.s V_0 + IL_0002: ldc.i4.1 + IL_0003: conv.i8 + IL_0004: ldc.i4.s 44 + IL_0006: conv.i8 + IL_0007: call instance void LargeStruct::.ctor(int64, + int64) + IL_000c: ldarg.0 + IL_000d: ldc.i4.1 + IL_000e: add + IL_000f: dup + IL_0010: starg.s count + IL_0012: ldloc.0 + IL_0013: ldarg.1 + IL_0014: tail. + IL_0016: call valuetype LargeStruct Test::A(int32, + valuetype LargeStruct, + valuetype LargeStruct) + IL_001b: ret + } // end of method Test::B + +} // end of class Test + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_5164/GitHub_5164.ilproj b/tests/src/JIT/Regression/JitBlue/GitHub_5164/GitHub_5164.ilproj new file mode 100644 index 0000000000..b5d5ee9077 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_5164/GitHub_5164.ilproj @@ -0,0 +1,44 @@ +<?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 .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="GitHub_5164.il" /> + </ItemGroup> + <ItemGroup> + <None Include="app.config" /> + </ItemGroup> + <ItemGroup> + <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" /> + </ItemGroup> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> + <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "> + </PropertyGroup> +</Project> diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_5164/app.config b/tests/src/JIT/Regression/JitBlue/GitHub_5164/app.config new file mode 100644 index 0000000000..8077c95440 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_5164/app.config @@ -0,0 +1,27 @@ +<?xml version="1.0" encoding="utf-8"?> +<configuration> + <runtime> + <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1"> + <dependentAssembly> + <assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.20.0" newVersion="4.0.20.0" /> + </dependentAssembly> + <dependentAssembly> + <assemblyIdentity name="System.Text.Encoding" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" /> + </dependentAssembly> + <dependentAssembly> + <assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" /> + </dependentAssembly> + <dependentAssembly> + <assemblyIdentity name="System.IO" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" /> + </dependentAssembly> + <dependentAssembly> + <assemblyIdentity name="System.Reflection" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" /> + </dependentAssembly> + </assemblyBinding> + </runtime> +</configuration>
\ No newline at end of file |