Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit a764455

Browse files
AndyAyersMSAndyAyersMS
authored
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.
1 parent d36b8e1 commit a764455

File tree

6 files changed

+201
-9
lines changed

6 files changed

+201
-9
lines changed

‎src/jit/compiler.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4573,6 +4573,17 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
45734573
/* Filter out unimported BBs */
45744574

45754575
fgRemoveEmptyBlocks();
4576+
4577+
// Update type of return spill temp if we have gathered better info
4578+
// when importing the inlinee.
4579+
if (fgNeedReturnSpillTemp())
4580+
{
4581+
CORINFO_CLASS_HANDLE retExprClassHnd = impInlineInfo->retExprClassHnd;
4582+
if (retExprClassHnd != nullptr)
4583+
{
4584+
lvaUpdateClass(lvaInlineeReturnSpillTemp, retExprClassHnd, impInlineInfo->retExprClassHndIsExact);
4585+
}
4586+
}
45764587
}
45774588

45784589
EndPhase(PHASE_POST_IMPORT);

‎src/jit/flowgraph.cpp‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5929,6 +5929,18 @@ void Compiler::fgFindBasicBlocks()
59295929
// The lifetime of this var might expand multiple BBs. So it is a long lifetime compiler temp.
59305930
lvaInlineeReturnSpillTemp = lvaGrabTemp(false DEBUGARG("Inline return value spill temp"));
59315931
lvaTable[lvaInlineeReturnSpillTemp].lvType = info.compRetNativeType;
5932+
5933+
// If the method returns a ref class, set the class of the spill temp
5934+
// to the method's return value. We may update this later if it turns
5935+
// out we can prove the method returns a more specific type.
5936+
if (info.compRetType == TYP_REF)
5937+
{
5938+
CORINFO_CLASS_HANDLE retClassHnd = impInlineInfo->inlineCandidateInfo->methInfo.args.retTypeClass;
5939+
if (retClassHnd != nullptr)
5940+
{
5941+
lvaSetClass(lvaInlineeReturnSpillTemp, retClassHnd);
5942+
}
5943+
}
59325944
}
59335945

59345946
return;
@@ -22441,13 +22453,15 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe
2244122453
memset(&inlineInfo, 0, sizeof(inlineInfo));
2244222454
CORINFO_METHOD_HANDLE fncHandle = call->gtCallMethHnd;
2244322455

22444-
inlineInfo.fncHandle = fncHandle;
22445-
inlineInfo.iciCall = call;
22446-
inlineInfo.iciStmt = fgMorphStmt;
22447-
inlineInfo.iciBlock = compCurBB;
22448-
inlineInfo.thisDereferencedFirst = false;
22449-
inlineInfo.retExpr = nullptr;
22450-
inlineInfo.inlineResult = inlineResult;
22456+
inlineInfo.fncHandle = fncHandle;
22457+
inlineInfo.iciCall = call;
22458+
inlineInfo.iciStmt = fgMorphStmt;
22459+
inlineInfo.iciBlock = compCurBB;
22460+
inlineInfo.thisDereferencedFirst = false;
22461+
inlineInfo.retExpr = nullptr;
22462+
inlineInfo.retExprClassHnd = nullptr;
22463+
inlineInfo.retExprClassHndIsExact = false;
22464+
inlineInfo.inlineResult = inlineResult;
2245122465
#ifdef FEATURE_SIMD
2245222466
inlineInfo.hasSIMDTypeArgLocalOrReturn = false;
2245322467
#endif // FEATURE_SIMD

‎src/jit/importer.cpp‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
12
// Licensed to the .NET Foundation under one or more agreements.
23
// The .NET Foundation licenses this file to you under the MIT license.
34
// See the LICENSE file in the project root for more information.
@@ -15838,6 +15839,31 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
1583815839
assert(info.compRetNativeType != TYP_VOID &&
1583915840
(fgMoreThanOneReturnBlock() || impInlineInfo->HasGcRefLocals()));
1584015841

15842+
// If this method returns a ref type, track the actual types seen
15843+
// in the returns.
15844+
if (info.compRetType == TYP_REF)
15845+
{
15846+
bool isExact = false;
15847+
bool isNonNull = false;
15848+
CORINFO_CLASS_HANDLE returnClsHnd = gtGetClassHandle(op2, &isExact, &isNonNull);
15849+
15850+
if (impInlineInfo->retExpr == nullptr)
15851+
{
15852+
// This is the first return, so best known type is the type
15853+
// of this return value.
15854+
impInlineInfo->retExprClassHnd = returnClsHnd;
15855+
impInlineInfo->retExprClassHndIsExact = isExact;
15856+
}
15857+
else if (impInlineInfo->retExprClassHnd != returnClsHnd)
15858+
{
15859+
// This return site type differs from earlier seen sites,
15860+
// so reset the info and we'll fall back to using the method's
15861+
// declared return type for the return spill temp.
15862+
impInlineInfo->retExprClassHnd = nullptr;
15863+
impInlineInfo->retExprClassHndIsExact = false;
15864+
}
15865+
}
15866+
1584115867
// This is a bit of a workaround...
1584215868
// If we are inlining a call that returns a struct, where the actual "native" return type is
1584315869
// 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&
1588115907
impAssignTempGen(lvaInlineeReturnSpillTemp, op2, se.seTypeInfo.GetClassHandle(),
1588215908
(unsigned)CHECK_SPILL_ALL);
1588315909

15884-
GenTreePtr tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, op2->TypeGet());
15910+
GenTree* tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, op2->TypeGet());
1588515911

1588615912
if (restoreType)
1588715913
{

‎src/jit/inline.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,9 @@ struct InlineInfo
562562

563563
InlineResult* inlineResult;
564564

565-
GenTreePtr retExpr; // The return expression of the inlined candidate.
565+
GenTreePtr retExpr; // The return expression of the inlined candidate.
566+
CORINFO_CLASS_HANDLE retExprClassHnd;
567+
bool retExprClassHndIsExact;
566568

567569
CORINFO_CONTEXT_HANDLE tokenLookupContextHandle; // The context handle that will be passed to
568570
// impTokenLookupContextHandle in Inlinee's Compiler.

‎tests/src/JIT/opt/Devirtualization/spilledreturn.cs‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Runtime.CompilerServices;
7+
8+
// Examples where methods potentially return multiple types
9+
// but the jit can prune the set down to one type during
10+
// importation, which then triggers late devirtualization.
11+
12+
public class Base
13+
{
14+
public virtual void Foo() { Console.WriteLine("Base:Foo"); }
15+
public virtual void Bar() { Console.WriteLine("Base:Bar"); }
16+
}
17+
18+
public class Derived : Base
19+
{
20+
public override sealed void Foo() { Console.WriteLine("Derived:Foo"); }
21+
public override void Bar() { Console.WriteLine("Derived:Bar"); }
22+
}
23+
24+
public class Derived2 : Base
25+
{
26+
public override sealed void Foo() { Console.WriteLine("Derived2:Foo"); }
27+
public override void Bar() { Console.WriteLine("Derived2:Bar"); }
28+
}
29+
30+
public class Test
31+
{
32+
static bool vague;
33+
34+
// Constant prop
35+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
36+
public static Base M(int x)
37+
{
38+
if (x > 0)
39+
{
40+
return new Derived();
41+
}
42+
else
43+
{
44+
return new Derived2();
45+
}
46+
}
47+
48+
// All returns agree on type
49+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
50+
public static Base N(bool b)
51+
{
52+
if (b)
53+
{
54+
Console.WriteLine("b true");
55+
return new Derived();
56+
}
57+
else
58+
{
59+
Console.WriteLine("b false");
60+
return new Derived();
61+
}
62+
}
63+
64+
// Type specialization
65+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
66+
public static Base G<T>()
67+
{
68+
if (typeof(T) == typeof(int))
69+
{
70+
return new Derived();
71+
}
72+
else
73+
{
74+
return new Derived2();
75+
}
76+
}
77+
78+
public static int Main(string[] args)
79+
{
80+
vague = args.Length > 0;
81+
82+
M(0).Foo();
83+
M(0).Bar();
84+
M(1).Foo();
85+
M(1).Bar();
86+
87+
N(vague).Foo();
88+
N(!vague).Bar();
89+
90+
G<int>().Foo();
91+
G<byte>().Foo();
92+
G<string>().Foo();
93+
94+
return 100;
95+
}
96+
}
97+
98+
99+
100+

‎tests/src/JIT/opt/Devirtualization/spilledreturn.csproj‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
4+
<PropertyGroup>
5+
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
6+
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
7+
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
8+
<SchemaVersion>2.0</SchemaVersion>
9+
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
10+
<OutputType>Exe</OutputType>
11+
<AppDesignerFolder>Properties</AppDesignerFolder>
12+
<FileAlignment>512</FileAlignment>
13+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
14+
<ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT .0\UITestExtensionPackages</ReferencePath>
15+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
16+
<NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
17+
<CLRTestPriority>1</CLRTestPriority>
18+
</PropertyGroup>
19+
<!-- Default configurations to help VS understand the configurations -->
20+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
21+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
22+
<ItemGroup>
23+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
24+
<Visible>False</Visible>
25+
</CodeAnalysisDependentAssemblyPaths>
26+
</ItemGroup>
27+
<PropertyGroup>
28+
<DebugType>PdbOnly</DebugType>
29+
<Optimize>True</Optimize>
30+
</PropertyGroup>
31+
<ItemGroup>
32+
<Compile Include="spilledreturn.cs" />
33+
</ItemGroup>
34+
<ItemGroup>
35+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
36+
</ItemGroup>
37+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
38+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
39+
</Project>

0 commit comments

Comments
 (0)