Skip to content

Commit

Permalink
JIT: improve return types in cases with spill temps
Browse files Browse the repository at this point in the history
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 dotnet#15743.
  • Loading branch information
AndyAyersMS committed Jan 6, 2018
1 parent 0bb37fd commit 4a1d407
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 9 deletions.
11 changes: 11 additions & 0 deletions src/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4571,6 +4571,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);
Expand Down
28 changes: 21 additions & 7 deletions src/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -22455,13 +22467,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
Expand Down
28 changes: 27 additions & 1 deletion src/jit/importer.cpp
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -15829,6 +15830,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
Expand Down Expand Up @@ -15872,7 +15898,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)
{
Expand Down
4 changes: 3 additions & 1 deletion src/jit/inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,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.
Expand Down
100 changes: 100 additions & 0 deletions tests/src/JIT/opt/Devirtualization/spilledreturn.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}




39 changes: 39 additions & 0 deletions tests/src/JIT/opt/Devirtualization/spilledreturn.csproj
Original file line number Diff line number Diff line change
@@ -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>

0 comments on commit 4a1d407

Please sign in to comment.