Skip to content

Commit 4a1d407

Browse files
AndyAyersMSAndyAyersMS
committed
JIT: improve return types in cases with spill temps
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.
1 parent 0bb37fd commit 4a1d407

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
@@ -4571,6 +4571,17 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
45714571
/* Filter out unimported BBs */
45724572

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

45764587
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;
@@ -22455,13 +22467,15 @@ void Compiler::fgInvokeInlineeCompiler(GenTreeCall* call, InlineResult* inlineRe
2245522467
memset(&inlineInfo, 0, sizeof(inlineInfo));
2245622468
CORINFO_METHOD_HANDLE fncHandle = call->gtCallMethHnd;
2245722469

22458-
inlineInfo.fncHandle = fncHandle;
22459-
inlineInfo.iciCall = call;
22460-
inlineInfo.iciStmt = fgMorphStmt;
22461-
inlineInfo.iciBlock = compCurBB;
22462-
inlineInfo.thisDereferencedFirst = false;
22463-
inlineInfo.retExpr = nullptr;
22464-
inlineInfo.inlineResult = inlineResult;
22470+
inlineInfo.fncHandle = fncHandle;
22471+
inlineInfo.iciCall = call;
22472+
inlineInfo.iciStmt = fgMorphStmt;
22473+
inlineInfo.iciBlock = compCurBB;
22474+
inlineInfo.thisDereferencedFirst = false;
22475+
inlineInfo.retExpr = nullptr;
22476+
inlineInfo.retExprClassHnd = nullptr;
22477+
inlineInfo.retExprClassHndIsExact = false;
22478+
inlineInfo.inlineResult = inlineResult;
2246522479
#ifdef FEATURE_SIMD
2246622480
inlineInfo.hasSIMDTypeArgLocalOrReturn = false;
2246722481
#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.
@@ -15829,6 +15830,31 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
1582915830
assert(info.compRetNativeType != TYP_VOID &&
1583015831
(fgMoreThanOneReturnBlock() || impInlineInfo->HasGcRefLocals()));
1583115832

15833+
// If this method returns a ref type, track the actual types seen
15834+
// in the returns.
15835+
if (info.compRetType == TYP_REF)
15836+
{
15837+
bool isExact = false;
15838+
bool isNonNull = false;
15839+
CORINFO_CLASS_HANDLE returnClsHnd = gtGetClassHandle(op2, &isExact, &isNonNull);
15840+
15841+
if (impInlineInfo->retExpr == nullptr)
15842+
{
15843+
// This is the first return, so best known type is the type
15844+
// of this return value.
15845+
impInlineInfo->retExprClassHnd = returnClsHnd;
15846+
impInlineInfo->retExprClassHndIsExact = isExact;
15847+
}
15848+
else if (impInlineInfo->retExprClassHnd != returnClsHnd)
15849+
{
15850+
// This return site type differs from earlier seen sites,
15851+
// so reset the info and we'll fall back to using the method's
15852+
// declared return type for the return spill temp.
15853+
impInlineInfo->retExprClassHnd = nullptr;
15854+
impInlineInfo->retExprClassHndIsExact = false;
15855+
}
15856+
}
15857+
1583215858
// This is a bit of a workaround...
1583315859
// If we are inlining a call that returns a struct, where the actual "native" return type is
1583415860
// not a struct (for example, the struct is composed of exactly one int, and the native
@@ -15872,7 +15898,7 @@ bool Compiler::impReturnInstruction(BasicBlock* block, int prefixFlags, OPCODE&
1587215898
impAssignTempGen(lvaInlineeReturnSpillTemp, op2, se.seTypeInfo.GetClassHandle(),
1587315899
(unsigned)CHECK_SPILL_ALL);
1587415900

15875-
GenTreePtr tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, op2->TypeGet());
15901+
GenTree* tmpOp2 = gtNewLclvNode(lvaInlineeReturnSpillTemp, op2->TypeGet());
1587615902

1587715903
if (restoreType)
1587815904
{

‎src/jit/inline.h

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

549549
InlineResult* inlineResult;
550550

551-
GenTreePtr retExpr; // The return expression of the inlined candidate.
551+
GenTreePtr retExpr; // The return expression of the inlined candidate.
552+
CORINFO_CLASS_HANDLE retExprClassHnd;
553+
bool retExprClassHndIsExact;
552554

553555
CORINFO_CONTEXT_HANDLE tokenLookupContextHandle; // The context handle that will be passed to
554556
// 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)