Skip to content

Commit

Permalink
Fix devirtualization across genericness in the hierarchy
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MichalStrehovsky committed Oct 1, 2024
1 parent 2917e51 commit 1cec738
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ static List<MethodDesc> BuildVTable(NodeFactory factory, TypeDesc currentType, T
if (currentType == null)
return vtable;

BuildVTable(factory, currentType.BaseType?.ConvertToCanonForm(CanonicalFormKind.Specific), implType, vtable);
BuildVTable(factory, currentType.BaseType, implType, vtable);

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

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

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

for (int i = 0; i < baseVtable.Count; i++)
{
if (baseVtable[i] != vtable[i])
_overriddenMethods.Add(baseVtable[i]);
_overriddenMethods.Add(baseVtable[i].GetCanonMethodTarget(CanonicalFormKind.Specific));
}
}
}
Expand Down
31 changes: 31 additions & 0 deletions src/tests/nativeaot/SmokeTests/UnitTests/Devirtualization.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class Devirtualization
internal static int Run()
{
RegressionBug73076.Run();
RegressionGenericHierarchy.Run();
DevirtualizationCornerCaseTests.Run();
DevirtualizeIntoUnallocatedGenericType.Run();

Expand Down Expand Up @@ -53,6 +54,36 @@ public static void Run()
}
}

class RegressionGenericHierarchy
{
class Base<T>
{
public virtual string Print() => "Base<T>";
}

class Mid : Base<Atom>
{
public override string Print() => "Mid";
public override string ToString() => Print();
}

class Derived : Mid
{
public override string Print() => "Derived";
}

class Atom { }

public static void Run()
{
if (Get().ToString() != "Derived")
throw new Exception();

[MethodImpl(MethodImplOptions.NoInlining)]
static object Get() => new Derived();
}
}

class DevirtualizationCornerCaseTests
{
interface IIntf1
Expand Down

0 comments on commit 1cec738

Please sign in to comment.