Fix fast tail call lowering bug.

Lowering::LowerFastTailCall has code that handles the case when the caller and the callee both have parameters
passed on the stack and the callee has register arguments that are computed from the caller's stack-passed arguments.
In that case the stack arguments of the callee are set up in the caller's incoming argument area so they override
the caller's arguments. Lowering::LowerFastTailCall creates temps for any caller's arguments passed on the stack that are
used to compute callee's register arguments. That code had a bug where it was converting GT_LCL_FLD nodes into
GT_LCL_VAR nodes. That's problematic both for the case of a single-field struct with a double field and for multi-field structs.
In the former case we were getting LSRA asserts in checked builds and SBCG in release builds. In the latter case we were getting SBCG
both in checked and release builds.

The fix is not to convert GT_LCL_FLD nodes into GT_LCL_VAR nodes and mark the new temps as not-enregisterable if the
local is not-enregisterable.

An alternative fix would be to create multiple register temps for each GT_LCL_FLD or GT_LCL_VAR targeting the local but that
would complicate the code and I decided to go with a simlper solution for this uncommon case.

diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp
index c303b17..87c96d5 100644 (file)
--- a/src/jit/lower.cpp
+++ b/src/jit/lower.cpp
@@ -1738,14 +1738,14 @@ void Lowering::LowerFastTailCall(GenTreeCall* call)
                     {
                         tmpLclNum = comp->lvaGrabTemp(
                             true DEBUGARG("Fast tail call lowering is creating a new local variable"));
-                        comp->lvaSortAgain                 = true;
-                        tmpType                            = genActualType(callerArgDsc->lvaArgType());
-                        comp->lvaTable[tmpLclNum].lvType   = tmpType;
-                        comp->lvaTable[tmpLclNum].lvRefCnt = 1;
+                        comp->lvaSortAgain                          = true;
+                        tmpType                                     = genActualType(callerArgDsc->lvaArgType());
+                        comp->lvaTable[tmpLclNum].lvType            = tmpType;
+                        comp->lvaTable[tmpLclNum].lvRefCnt          = 1;
+                        comp->lvaTable[tmpLclNum].lvDoNotEnregister = comp->lvaTable[lcl->gtLclNum].lvDoNotEnregister;
                     }
 
                     lcl->SetLclNum(tmpLclNum);
-                    lcl->SetOper(GT_LCL_VAR);
                 }
             }
         }

diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_12037/GitHub_12037.cs b/tests/src/JIT/Regression/JitBlue/GitHub_12037/GitHub_12037.cs
new file mode 100644 (file)
index 0000000..527a8a1
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_12037/GitHub_12037.cs
@@ -0,0 +1,109 @@
+// 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;
+
+public class TailCallOptTest
+{
+    public static int Main()
+    {
+        bool res1 = Caller1(new object(), 0L, 0xBEEF, new TypedDouble(1.0), new TypedDouble(2.0), new TypedDouble(3.0));
+        bool res2 = Caller2(new object(), 0L, 0xBEEF, new TypedDouble(1.0), new TwoInts(3, 5), new TypedDouble(3.0));
+        return (res1 && res2) ? 100 : 0;
+    }
+
+    // In this test typedDouble2 is passed to Caller1 on the stack. Then typedDouble2.Value is passed to Callee1 in a register.
+    // Since Calee1 also has a stack argument (typedDouble3.Value) and the call is dispatched as a fast tail call,
+    // it's set up in the incoming argument area of Caller1. JIT lowering code needs to ensure that typedDouble2
+    // in that area is not overwritten by typedDouble3.Value before typedDouble2.Value is computed. The JIT had a bug in that code
+    // because typedDouble2.Value was represented as GT_LCL_FLD and it was incorrectly converted into GT_LCL_VAR resulting in type
+    // mismatches (long vs. double since the struct is passed as a long but its only field is double).
+    public static bool Caller1(object parameters, long l,
+        double doubleArg, TypedDouble typedDouble1, TypedDouble typedDouble2, TypedDouble typedDouble3)
+    {
+        double param = 19.0;
+
+        Console.Write("Let's ");
+        Console.Write("Discourage ");
+        Console.Write("Inlining ");
+        Console.Write("Of ");
+        Console.Write("Caller ");
+        Console.Write("Into ");
+        Console.WriteLine("Main.");
+
+        return Callee1(doubleArg, param, typedDouble1.Value, typedDouble2.Value, typedDouble3.Value);
+    }
+
+    public static bool Callee1(double doubleArg, double param, double typedDoubleArg1, double typedDoubleArg2, double typedDoubleArg3)
+    {
+        Console.WriteLine("{0} {1} {2} {3} {4}", doubleArg, param, typedDoubleArg1, typedDoubleArg2, typedDoubleArg3);
+        if ((doubleArg == 0xBEEF) && (param == 19.0) && (typedDoubleArg1 == 1.0) && (typedDoubleArg2 == 2.0) && (typedDoubleArg3 == 3.0))
+        {
+            Console.WriteLine("PASSED");
+            return true;
+        }
+        else
+        {
+            Console.WriteLine("FAILED");
+            return false;
+        }
+    }
+
+    // In this test twoInts is passed to Caller2 on the stack. Then twoInts.Value1 and twoInts.Value2 were passed to Callee2 in registers.
+    // Since Calee2 also has a stack argument (i3) and the call is dispatched as a fast tail call,
+    // it's set up in the incoming argument area of Caller2. JIT lowering code needs to ensure that twoInts
+    // in that area is not overwritten by i3 before twoInts.Value1 and twoInts.Value2 are computed. The JIT had a bug in that code
+    // because twoInts.Value1 and twoInts.Value2 were represented as GT_LCL_FLD and they were incorrectly converted into GT_LCL_VAR
+    // resulting in an identical value passed for both fields.
+    public static bool Caller2(object parameters, long l,
+        double doubleArg, TypedDouble typedDouble1, TwoInts twoInts, TypedDouble typedDouble3)
+    {
+        double param = 19.0;
+
+        Console.Write("Let's ");
+        Console.Write("Discourage ");
+        Console.Write("Inlining ");
+        Console.Write("Of ");
+        Console.Write("Caller ");
+        Console.Write("Into ");
+        Console.WriteLine("Main.");
+
+        return Callee2(twoInts.Value1, twoInts.Value2, typedDouble1.Value, param, 11);
+    }
+
+    public static bool Callee2(int i1, int i2, double typedDoubleArg1, double typedDoubleArg2, int i3)
+    {
+        Console.WriteLine("{0} {1} {2} {3} {4}", i1, i2, typedDoubleArg1, typedDoubleArg2, i3);
+        if ((i1 == 3) && (i2 == 5) && (typedDoubleArg1 == 1.0) && (typedDoubleArg2 == 19) && (i3 == 11))
+        {
+            Console.WriteLine("PASSED");
+            return true;
+        }
+        else
+        {
+            Console.WriteLine("FAILED");
+            return false;
+        }
+    }
+
+    public struct TypedDouble
+    {
+        public TypedDouble(double value)
+        {
+            Value = value;
+        }
+        public readonly double Value;
+    }
+
+    public struct TwoInts
+    {
+        public TwoInts(int value1, int value2)
+        {
+            Value1 = value1;
+            Value2 = value2;
+        }
+        public readonly int Value1;
+        public readonly int Value2;
+    }
+}

diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_12037/GitHub_12037.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_12037/GitHub_12037.csproj
new file mode 100644 (file)
index 0000000..2cafa4c
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_12037/GitHub_12037.csproj
@@ -0,0 +1,38 @@
+<?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>
+    <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></DebugType>
+    <Optimize>True</Optimize>
+  </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>