JIT: improve return types in cases with spill temps (#15766)

If the jit sees that an inlinee has multiple return sites or has gc ref locals
it will choose to return the inline result via a temp. The jit was not assigning
a type to that temp and so losing track of some type information.

So, for inlinees returning ref types, initially type the return spill temp with
the declared return type of the method.

When importing we may discover that particular return sites will return more
specific types. If all discovered return sites agree, we can update the return
type for the spill temp to match the consensus improved type.

This can lead to removal of some type checks and also to devirtualization.

Addresses issues discussed in #9908 and #15743.

diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp
index b6a2771..ce88954 100644 (file)
--- a/src/jit/compiler.cpp
+++ b/src/jit/compiler.cpp
@@ -4573,6 +4573,17 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
             /* Filter out unimported BBs */
 
             fgRemoveEmptyBlocks();
+
+            // Update type of return spill temp if we have gathered better info
+            // when importing the inlinee.
+            if (fgNeedReturnSpillTemp())
+            {
+                CORINFO_CLASS_HANDLE retExprClassHnd = impInlineInfo->retExprClassHnd;
+                if (retExprClassHnd != nullptr)
+                {
+                    lvaUpdateClass(lvaInlineeReturnSpillTemp, retExprClassHnd, impInlineInfo->retExprClassHndIsExact);
+                }
+            }
         }
 
         EndPhase(PHASE_POST_IMPORT);

diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index 22fd0d3..d3e6c8c 100644 (file)
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -5929,6 +5929,18 @@ void Compiler::fgFindBasicBlocks()
             // The lifetime of this var might expand multiple BBs. So it is a long lifetime compiler temp.
             lvaInlineeReturnSpillTemp                  = lvaGrabTemp(false DEBUGARG("Inline return value spill temp"));
             lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetNativeType;
+
+            // If the method returns a ref class, set the class of the spill temp
+            // to the method's return value. We may update this later if it turns
+            // out we can prove the method returns a more specific type.
+            if (info.compRetType == TYP_REF)
+            {
+                CORINFO_CLASS_HANDLE retClassHnd = impInlineInfo->inlineCandidateInfo->methInfo.args.retTypeClass;
+                if (retClassHnd != nullptr)
+                {
+                    lvaSetClass(lvaInlineeReturnSpillTemp, retClassHnd);
+                }
+            }
         }
 
         return;
@@ -22441,13 +22453,15 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe
     memset(&inlineInfo, 0, sizeof(inlineInfo));
     CORINFO_METHOD_HANDLE fncHandle = call->gtCallMethHnd;
 
-    inlineInfo.fncHandle             = fncHandle;
-    inlineInfo.iciCall               = call;
-    inlineInfo.iciStmt               = fgMorphStmt;
-    inlineInfo.iciBlock              = compCurBB;
-    inlineInfo.thisDereferencedFirst = false;
-    inlineInfo.retExpr               = nullptr;
-    inlineInfo.inlineResult          = inlineResult;
+    inlineInfo.fncHandle              = fncHandle;
+    inlineInfo.iciCall                = call;
+    inlineInfo.iciStmt                = fgMorphStmt;
+    inlineInfo.iciBlock               = compCurBB;
+    inlineInfo.thisDereferencedFirst  = false;
+    inlineInfo.retExpr                = nullptr;
+    inlineInfo.retExprClassHnd        = nullptr;
+    inlineInfo.retExprClassHndIsExact = false;
+    inlineInfo.inlineResult           = inlineResult;
 #ifdef FEATURE_SIMD
     inlineInfo.hasSIMDTypeArgLocalOrReturn = false;
 #endif // FEATURE_SIMD

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 84c7f61..6fd836f 100644 (file)
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -1,3 +1,4 @@
+
 // 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.
@@ -15838,6 +15839,31 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
                     assert(info.compRetNativeType != TYP_VOID &&
                            (fgMoreThanOneReturnBlock() || impInlineInfo->HasGcRefLocals()));
 
+                    // If this method returns a ref type, track the actual types seen
+                    // in the returns.
+                    if (info.compRetType == TYP_REF)
+                    {
+                        bool                 isExact      = false;
+                        bool                 isNonNull    = false;
+                        CORINFO_CLASS_HANDLE returnClsHnd = gtGetClassHandle(op2, &isExact, &isNonNull);
+
+                        if (impInlineInfo->retExpr == nullptr)
+                        {
+                            // This is the first return, so best known type is the type
+                            // of this return value.
+                            impInlineInfo->retExprClassHnd        = returnClsHnd;
+                            impInlineInfo->retExprClassHndIsExact = isExact;
+                        }
+                        else if (impInlineInfo->retExprClassHnd != returnClsHnd)
+                        {
+                            // This return site type differs from earlier seen sites,
+                            // so reset the info and we'll fall back to using the method's
+                            // declared return type for the return spill temp.
+                            impInlineInfo->retExprClassHnd        = nullptr;
+                            impInlineInfo->retExprClassHndIsExact = false;
+                        }
+                    }
+
                     // This is a bit of a workaround...
                     // If we are inlining a call that returns a struct, where the actual "native" return type is
                     // not a struct (for example, the struct is composed of exactly one int, and the native
@@ -15881,7 +15907,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
                     impAssignTempGen(lvaInlineeReturnSpillTemp, op2, se.seTypeInfo.GetClassHandle(),
                                      (unsigned)CHECK_SPILL_ALL);
 
-                    GenTreePtr tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, op2->TypeGet());
+                    GenTree* tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, op2->TypeGet());
 
                     if (restoreType)
                     {

diff --git a/src/jit/inline.h b/src/jit/inline.h
index 3402c52..b5a2613 100644 (file)
--- a/src/jit/inline.h
+++ b/src/jit/inline.h
@@ -562,7 +562,9 @@ struct InlineInfo
 
     InlineResult* inlineResult;
 
-    GenTreePtr retExpr; // The return expression of the inlined candidate.
+    GenTreePtr           retExpr; // The return expression of the inlined candidate.
+    CORINFO_CLASS_HANDLE retExprClassHnd;
+    bool                 retExprClassHndIsExact;
 
     CORINFO_CONTEXT_HANDLE tokenLookupContextHandle; // The context handle that will be passed to
                                                      // impTokenLookupContextHandle in Inlinee's Compiler.

diff --git a/tests/src/JIT/opt/Devirtualization/spilledreturn.cs b/tests/src/JIT/opt/Devirtualization/spilledreturn.cs
new file mode 100644 (file)
index 0000000..ac63806
--- /dev/null
+++ b/tests/src/JIT/opt/Devirtualization/spilledreturn.cs
@@ -0,0 +1,100 @@
+// 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;
+
+// Examples where methods potentially return multiple types
+// but the jit can prune the set down to one type during
+// importation, which then triggers late devirtualization.
+
+public class Base 
+{
+    public virtual void Foo() { Console.WriteLine("Base:Foo"); }
+    public virtual void Bar() { Console.WriteLine("Base:Bar"); }
+}
+
+public class Derived : Base
+{
+    public override sealed void Foo() { Console.WriteLine("Derived:Foo"); }
+    public override void Bar() { Console.WriteLine("Derived:Bar"); }
+}
+
+public class Derived2 : Base
+{
+    public override sealed void Foo() { Console.WriteLine("Derived2:Foo"); }
+    public override void Bar() { Console.WriteLine("Derived2:Bar"); }
+}
+
+public class Test
+{
+    static bool vague;
+
+    // Constant prop
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static Base M(int x)
+    {
+        if (x > 0)
+        {
+            return new Derived();
+        }
+        else 
+        {
+            return new Derived2();
+        }
+    }
+
+    // All returns agree on type
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static Base N(bool b)
+    {
+        if (b)
+        {
+            Console.WriteLine("b true");
+            return new Derived();
+        }
+        else 
+        {
+            Console.WriteLine("b false");
+            return new Derived();
+        }
+    }
+
+    // Type specialization
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static Base G<T>()
+    {
+        if (typeof(T) == typeof(int))
+        {
+            return new Derived();
+        }
+        else
+        {
+            return new Derived2();
+        }
+    }
+
+    public static int Main(string[] args)
+    {
+        vague = args.Length > 0;
+
+        M(0).Foo();
+        M(0).Bar();
+        M(1).Foo();
+        M(1).Bar();
+
+        N(vague).Foo();
+        N(!vague).Bar();
+
+        G<int>().Foo();
+        G<byte>().Foo();
+        G<string>().Foo();
+
+        return 100;
+    }
+}
+
+
+        
+    

diff --git a/tests/src/JIT/opt/Devirtualization/spilledreturn.csproj b/tests/src/JIT/opt/Devirtualization/spilledreturn.csproj
new file mode 100644 (file)
index 0000000..b3ee960
--- /dev/null
+++ b/tests/src/JIT/opt/Devirtualization/spilledreturn.csproj
@@ -0,0 +1,39 @@
+<?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>
+    <CLRTestPriority>1</CLRTestPriority>
+  </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>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="spilledreturn.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>
\ No newline at end of file