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

Commit c2ba64d

Browse files
AndyAyersMSAndyAyersMS
authored
Merge pull request #10371 from AndyAyersMS/FixDevirt
Fix several devirtualization issues
2 parents 6638432 + d02fbd2 commit c2ba64d

File tree

11 files changed

+560
-10
lines changed

11 files changed

+560
-10
lines changed

‎src/jit/importer.cpp‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18521,7 +18521,14 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,
1852118521
// the time of jitting, objClass has no subclasses that
1852218522
// override this method), then perhaps we'd be willing to
1852318523
// make a bet...?
18524-
JITDUMP(" Class NOT final or exact, no devirtualization\n");
18524+
JITDUMP(" Class not final or exact, method not final, no devirtualization\n");
18525+
return;
18526+
}
18527+
18528+
// For interface calls we must have an exact type or final class.
18529+
if (isInterface && !isExact && !objClassIsFinal)
18530+
{
18531+
JITDUMP(" Class not final or exact for interface, no devirtualization\n");
1852518532
return;
1852618533
}
1852718534

@@ -18604,4 +18611,4 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,
1860418611
baseMethodName, derivedClassName, derivedMethodName, note);
1860518612
}
1860618613
#endif // defined(DEBUG)
18607-
}
18614+
}

‎src/vm/jitinterface.cpp‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8714,12 +8714,16 @@ void CEEInfo::getMethodVTableOffset (CORINFO_METHOD_HANDLE methodHnd,
87148714
}
87158715

87168716
/*********************************************************************/
8717-
static CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(MethodDesc* callerMethod,
8718-
CORINFO_METHOD_HANDLE baseMethod,
8719-
CORINFO_CLASS_HANDLE derivedClass,
8720-
CORINFO_CONTEXT_HANDLE ownerType)
8717+
CORINFO_METHOD_HANDLE CEEInfo::resolveVirtualMethodHelper(CORINFO_METHOD_HANDLE baseMethod,
8718+
CORINFO_CLASS_HANDLE derivedClass,
8719+
CORINFO_CONTEXT_HANDLE ownerType)
87218720
{
8722-
STANDARD_VM_CONTRACT;
8721+
CONTRACTL {
8722+
SO_TOLERANT;
8723+
THROWS;
8724+
GC_TRIGGERS;
8725+
MODE_PREEMPTIVE;
8726+
} CONTRACTL_END;
87238727

87248728
MethodDesc* pBaseMD = GetMethod(baseMethod);
87258729
MethodTable* pBaseMT = pBaseMD->GetMethodTable();
@@ -8747,6 +8751,15 @@ static CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(MethodDesc* callerMethod
87478751

87488752
if (pBaseMT->IsInterface())
87498753
{
8754+
8755+
#ifdef FEATURE_COMINTEROP
8756+
// Don't try and devirtualize com interface calls.
8757+
if (pDerivedMT->IsComObjectType())
8758+
{
8759+
return nullptr;
8760+
}
8761+
#endif // FEATURE_COMINTEROP
8762+
87508763
// Interface call devirtualization.
87518764
//
87528765
// We must ensure that pDerivedMT actually implements the
@@ -8760,7 +8773,17 @@ static CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(MethodDesc* callerMethod
87608773
// safely devirtualize.
87618774
if (ownerType != nullptr)
87628775
{
8763-
pDevirtMD = pDerivedMT->GetMethodDescForInterfaceMethod(GetTypeFromContext(ownerType), pBaseMD);
8776+
TypeHandle OwnerClsHnd = GetTypeFromContext(ownerType);
8777+
MethodTable* pOwnerMT = OwnerClsHnd.GetMethodTable();
8778+
8779+
// If the derived class is a shared class, make sure the
8780+
// owner class is too.
8781+
if (pDerivedMT->IsSharedByGenericInstantiations())
8782+
{
8783+
pOwnerMT = pOwnerMT->GetCanonicalMethodTable();
8784+
}
8785+
8786+
pDevirtMD = pDerivedMT->GetMethodDescForInterfaceMethod(TypeHandle(pOwnerMT), pBaseMD);
87648787
}
87658788
else if (!pBaseMD->HasClassOrMethodInstantiation())
87668789
{
@@ -8810,6 +8833,7 @@ static CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(MethodDesc* callerMethod
88108833
// bubble information and if so, disallow it.
88118834
if (IsReadyToRunCompilation())
88128835
{
8836+
MethodDesc* callerMethod = m_pMethodBeingCompiled;
88138837
Assembly* pCallerAssembly = callerMethod->GetModule()->GetAssembly();
88148838
bool allowDevirt =
88158839
IsInSameVersionBubble(pCallerAssembly , pDevirtMD->GetModule()->GetAssembly())
@@ -8829,13 +8853,18 @@ CORINFO_METHOD_HANDLE CEEInfo::resolveVirtualMethod(CORINFO_METHOD_HANDLE method
88298853
CORINFO_CLASS_HANDLE derivedClass,
88308854
CORINFO_CONTEXT_HANDLE ownerType)
88318855
{
8832-
STANDARD_VM_CONTRACT;
8856+
CONTRACTL {
8857+
SO_TOLERANT;
8858+
THROWS;
8859+
GC_TRIGGERS;
8860+
MODE_PREEMPTIVE;
8861+
} CONTRACTL_END;
88338862

88348863
CORINFO_METHOD_HANDLE result = nullptr;
88358864

88368865
JIT_TO_EE_TRANSITION();
88378866

8838-
result = resolveVirtualMethodHelper(m_pMethodBeingCompiled, methodHnd, derivedClass, ownerType);
8867+
result = resolveVirtualMethodHelper(methodHnd, derivedClass, ownerType);
88398868

88408869
EE_TO_JIT_TRANSITION();
88418870

‎src/vm/jitinterface.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,12 @@ class CEEInfo : public ICorJitInfo
735735
CORINFO_CONTEXT_HANDLE ownerType
736736
);
737737

738+
CORINFO_METHOD_HANDLE resolveVirtualMethodHelper(
739+
CORINFO_METHOD_HANDLE virtualMethod,
740+
CORINFO_CLASS_HANDLE implementingClass,
741+
CORINFO_CONTEXT_HANDLE ownerType
742+
);
743+
738744
CorInfoIntrinsics getIntrinsicID(CORINFO_METHOD_HANDLE method,
739745
bool * pMustExpand = NULL);
740746

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
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.Collections;
7+
using System.Collections.Generic;
8+
9+
public class MyCollection<T> : ICollection<T>
10+
{
11+
private List<T> _items = new List<T>();
12+
13+
public MyCollection()
14+
{
15+
}
16+
17+
public MyCollection(params T[] values)
18+
{
19+
_items.AddRange(values);
20+
}
21+
22+
public void Add(T item)
23+
{
24+
_items.Add(item);
25+
}
26+
27+
public void Clear()
28+
{
29+
_items.Clear();
30+
}
31+
32+
public bool Contains(T item)
33+
{
34+
return _items.Contains(item);
35+
}
36+
37+
public void CopyTo(T[] array, int arrayIndex)
38+
{
39+
_items.CopyTo(array, arrayIndex);
40+
}
41+
42+
public int Count
43+
{
44+
get { return _items.Count; }
45+
}
46+
47+
public bool IsReadOnly
48+
{
49+
get { return ((ICollection<T>)_items).IsReadOnly; }
50+
}
51+
52+
public bool Remove(T item)
53+
{
54+
return _items.Remove(item);
55+
}
56+
57+
public IEnumerator<T> GetEnumerator()
58+
{
59+
return ((ICollection<T>)_items).GetEnumerator();
60+
}
61+
62+
IEnumerator IEnumerable.GetEnumerator()
63+
{
64+
return ((IEnumerable)_items).GetEnumerator();
65+
}
66+
}
67+
68+
class Bug
69+
{
70+
public static int Main()
71+
{
72+
int v = 0;
73+
MyCollection<string> x = new MyCollection<string>("a1", "a2");
74+
foreach (string item in x)
75+
{
76+
v += item[0];
77+
}
78+
return v - 94;
79+
}
80+
}

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
12+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
13+
</PropertyGroup>
14+
<!-- Default configurations to help VS understand the configurations -->
15+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
16+
</PropertyGroup>
17+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
18+
</PropertyGroup>
19+
<ItemGroup>
20+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
21+
<Visible>False</Visible>
22+
</CodeAnalysisDependentAssemblyPaths>
23+
</ItemGroup>
24+
<PropertyGroup>
25+
<DebugType>None</DebugType>
26+
<Optimize>True</Optimize>
27+
</PropertyGroup>
28+
<ItemGroup>
29+
<Compile Include="GitHub_10311.cs" />
30+
</ItemGroup>
31+
<ItemGroup>
32+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
33+
</ItemGroup>
34+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
35+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
36+
</PropertyGroup>
37+
</Project>

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
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+
7+
interface Ix
8+
{
9+
int F();
10+
int G();
11+
}
12+
13+
public class B<T> : Ix
14+
{
15+
int Ix.F()
16+
{
17+
if (typeof(T) == typeof(string))
18+
{
19+
return 3;
20+
}
21+
else
22+
{
23+
return 5;
24+
}
25+
}
26+
27+
public virtual int G()
28+
{
29+
if (typeof(T) == typeof(object))
30+
{
31+
return 7;
32+
}
33+
else
34+
{
35+
return 11;
36+
}
37+
38+
}
39+
}
40+
41+
public class D : B<string>, Ix
42+
{
43+
int Ix.F() { return 13; }
44+
}
45+
46+
class E : D
47+
{
48+
public sealed override int G() { return 17; }
49+
}
50+
51+
// K overrides E.G for interface purposes, even though it is sealed
52+
class K : E, Ix
53+
{
54+
int Ix.G() { return 19; }
55+
}
56+
57+
sealed class J : E, Ix
58+
{
59+
int Ix.F() { return 21; }
60+
}
61+
62+
public class Z
63+
{
64+
static int IxF(Ix x) { return x.F(); }
65+
static int IxG(Ix x) { return x.G(); }
66+
67+
public static int Main(string[] args)
68+
{
69+
E e = new E();
70+
K k = new K();
71+
J j = new J();
72+
E q = k;
73+
74+
int callsBFs = IxF(new B<string>());
75+
int callsBFo = IxF(new B<object>());
76+
int callsBGo = IxG(new B<object>());
77+
int callsBGs = IxG(new B<string>()) + IxG(new D());
78+
int callsDF = IxF(new D()) + IxF(e) + IxF(k) + IxF(q);
79+
int callsEG = IxG(e) + IxG(j);
80+
int callsKG = IxG(k) + IxG(q);
81+
int callsJF = IxF(j);
82+
83+
int expected = 3 + 5 + 7 + 2 * 11 + 4 * 13 + 2 * 17 + 2 * 19 + 21;
84+
int val = callsBFs + callsBFo + callsDF + callsBGs + callsBGo + callsEG + callsKG + callsJF;
85+
86+
return val - expected + 100;
87+
}
88+
}
89+
90+

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
12+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
13+
</PropertyGroup>
14+
<!-- Default configurations to help VS understand the configurations -->
15+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
16+
</PropertyGroup>
17+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
18+
</PropertyGroup>
19+
<ItemGroup>
20+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
21+
<Visible>False</Visible>
22+
</CodeAnalysisDependentAssemblyPaths>
23+
</ItemGroup>
24+
<PropertyGroup>
25+
<DebugType>PdbOnly</DebugType>
26+
<Optimize>True</Optimize>
27+
</PropertyGroup>
28+
<ItemGroup>
29+
<Compile Include="generic.cs" />
30+
</ItemGroup>
31+
<ItemGroup>
32+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
33+
</ItemGroup>
34+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
35+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
36+
</PropertyGroup>
37+
</Project>

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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+
7+
interface Io<T,U> where T:class where U:class
8+
{
9+
T FromU(U u);
10+
T FromS(string s);
11+
}
12+
13+
public class Z : Io<string, string>
14+
{
15+
string Io<string, string>.FromU(string s) { return "U"; }
16+
string Io<string, string>.FromS(string s) { return "S"; }
17+
18+
public static int Main(string[] args)
19+
{
20+
string fromU = ((Io<string, string>) new Z()).FromU("u");
21+
string fromS = ((Io<string, string>) new Z()).FromS("s");
22+
23+
return fromU[0] + fromS[0] - 68;
24+
}
25+
}
26+
27+

0 commit comments

Comments
 (0)