Skip to content

Commit 704d4d9

Browse files
MichalStrehovskylambdageek
authored andcommitted
Fix devirtualization across genericness in the hierarchy (dotnet#108442)
This code was trying to answer question: "Was this method overriden by something else in a more derived class"? It was walking the base hierarchy in canonical form, but that was leading to methods not resolving at all. The fix is to walk the non-canonical hierarchy and canonicalize after we resolved everything. I ran into this in dotnet#108379 that unlocked more whole program devirtualization and `StringSearchValuesBase` is in this shape.
1 parent 9f2c6b5 commit 704d4d9

File tree

2 files changed

+37
-6
lines changed

2 files changed

+37
-6
lines changed

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ static List<MethodDesc> BuildVTable(NodeFactory factory, TypeDesc currentType, T
559559
if (currentType == null)
560560
return vtable;
561561

562-
BuildVTable(factory, currentType.BaseType?.ConvertToCanonForm(CanonicalFormKind.Specific), implType, vtable);
562+
BuildVTable(factory, currentType.BaseType, implType, vtable);
563563

564564
IReadOnlyList<MethodDesc> slice = factory.VTable(currentType).Slots;
565565
foreach (MethodDesc decl in slice)
@@ -571,19 +571,19 @@ static List<MethodDesc> BuildVTable(NodeFactory factory, TypeDesc currentType, T
571571
return vtable;
572572
}
573573

574-
baseType = canonType.BaseType?.ConvertToCanonForm(CanonicalFormKind.Specific);
575-
if (!canonType.IsArray && baseType != null)
574+
baseType = type.BaseType;
575+
if (!type.IsArray && baseType != null)
576576
{
577577
if (!vtables.TryGetValue(baseType, out List<MethodDesc> baseVtable))
578578
vtables.Add(baseType, baseVtable = BuildVTable(factory, baseType, baseType, new List<MethodDesc>()));
579579

580-
if (!vtables.TryGetValue(canonType, out List<MethodDesc> vtable))
581-
vtables.Add(canonType, vtable = BuildVTable(factory, canonType, canonType, new List<MethodDesc>()));
580+
if (!vtables.TryGetValue(type, out List<MethodDesc> vtable))
581+
vtables.Add(type, vtable = BuildVTable(factory, type, type, new List<MethodDesc>()));
582582

583583
for (int i = 0; i < baseVtable.Count; i++)
584584
{
585585
if (baseVtable[i] != vtable[i])
586-
_overriddenMethods.Add(baseVtable[i]);
586+
_overriddenMethods.Add(baseVtable[i].GetCanonMethodTarget(CanonicalFormKind.Specific));
587587
}
588588
}
589589
}

src/tests/nativeaot/SmokeTests/UnitTests/Devirtualization.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class Devirtualization
1212
internal static int Run()
1313
{
1414
RegressionBug73076.Run();
15+
RegressionGenericHierarchy.Run();
1516
DevirtualizationCornerCaseTests.Run();
1617
DevirtualizeIntoUnallocatedGenericType.Run();
1718

@@ -53,6 +54,36 @@ public static void Run()
5354
}
5455
}
5556

57+
class RegressionGenericHierarchy
58+
{
59+
class Base<T>
60+
{
61+
public virtual string Print() => "Base<T>";
62+
}
63+
64+
class Mid : Base<Atom>
65+
{
66+
public override string Print() => "Mid";
67+
public override string ToString() => Print();
68+
}
69+
70+
class Derived : Mid
71+
{
72+
public override string Print() => "Derived";
73+
}
74+
75+
class Atom { }
76+
77+
public static void Run()
78+
{
79+
if (Get().ToString() != "Derived")
80+
throw new Exception();
81+
82+
[MethodImpl(MethodImplOptions.NoInlining)]
83+
static object Get() => new Derived();
84+
}
85+
}
86+
5687
class DevirtualizationCornerCaseTests
5788
{
5889
interface IIntf1

0 commit comments

Comments
 (0)