Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't report interfaces with optimized-away MT as "never instantiated" #111493

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,28 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType
/// </summary>
public virtual bool CanReferenceConstructedMethodTable(TypeDesc type) => true;

/// <summary>
/// Gets a value indicating whether it might be possible that the interface type is implemented by a constructed type data structure
/// in this compilation. Note that the MethodTable for the interface itself could still be optimized away.
/// </summary>
public virtual bool CanInterfaceBeImplementedByConstructedMethodTable(TypeDesc type) => true;

/// <summary>
/// Gets a value indicating whether a (potentially canonically-equlivalent) constructed MethodTable could
/// exist. This is similar to <see cref="CanReferenceConstructedMethodTable"/>, but will return true
/// for List&lt;__Canon&gt; if a constructed MethodTable for List&lt;object&gt; exists.
/// </summary>
public virtual bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc type) => true;

/// <summary>
/// Gets a value indicating whether it might be possible that the interface type is implemented by a constructed type data structure
/// in this compilation. Note that the MethodTable for the interface itself could still be optimized away.
/// Similar to <see cref="CanInterfaceBeImplementedByConstructedMethodTable"/> but will return true for
/// IInterface&lt;__Canon&gt; if IInterface&lt;String&gt; could be implemented.
/// </summary>
public virtual bool CanInterfaceOrCanonicalFormOfItBeImplementedByConstructedMethodTable(TypeDesc type) => true;


public virtual TypeDesc[] GetImplementingClasses(TypeDesc type) => null;
#endif
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,21 @@ public bool CanReferenceConstructedMethodTable(TypeDesc type)
return NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type.NormalizeInstantiation());
}

public bool CanInterfaceBeImplementedByConstructedMethodTable(TypeDesc type)
{
return NodeFactory.DevirtualizationManager.CanInterfaceBeImplementedByConstructedMethodTable(type.NormalizeInstantiation());
}

public bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc type)
{
return NodeFactory.DevirtualizationManager.CanReferenceConstructedTypeOrCanonicalFormOfType(type.NormalizeInstantiation());
}

public bool CanInterfaceOrCanonicalFormOfItBeImplementedByConstructedMethodTable(TypeDesc type)
{
return NodeFactory.DevirtualizationManager.CanInterfaceOrCanonicalFormOfItBeImplementedByConstructedMethodTable(type.NormalizeInstantiation());
}

public DelegateCreationInfo GetDelegateCtor(TypeDesc delegateType, MethodDesc target, TypeDesc constrainedType, bool followVirtualDispatch)
{
// If we're creating a delegate to a virtual method that cannot be overridden, devirtualize.
Expand Down
23 changes: 23 additions & 0 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,8 @@ private sealed class ScannedDevirtualizationManager : DevirtualizationManager
private HashSet<TypeDesc> _constructedMethodTables = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _canonConstructedMethodTables = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _canonConstructedTypes = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _implementedInterfaces = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _canonImplementedInterfaces = new HashSet<TypeDesc>();
private HashSet<TypeDesc> _unsealedTypes = new HashSet<TypeDesc>();
private Dictionary<TypeDesc, HashSet<TypeDesc>> _implementators = new();
private HashSet<TypeDesc> _disqualifiedTypes = new();
Expand Down Expand Up @@ -507,6 +509,13 @@ public ScannedDevirtualizationManager(NodeFactory factory, ImmutableArray<Depend
{
RecordImplementation(baseInterface, type);
}

if (_implementedInterfaces.Add(baseInterface))
{
TypeDesc canonBaseInterface = baseInterface.ConvertToCanonForm(CanonicalFormKind.Specific);
if (baseInterface != canonBaseInterface)
_canonImplementedInterfaces.Add(canonBaseInterface);
}
}

// Record all base types of this class
Expand Down Expand Up @@ -729,13 +738,27 @@ public override bool CanReferenceConstructedMethodTable(TypeDesc type)
return _constructedMethodTables.Contains(type);
}

public override bool CanInterfaceBeImplementedByConstructedMethodTable(TypeDesc type)
{
Debug.Assert(type.NormalizeInstantiation() == type);
Debug.Assert(type.IsInterface);
return _implementedInterfaces.Contains(type);
}

public override bool CanReferenceConstructedTypeOrCanonicalFormOfType(TypeDesc type)
{
Debug.Assert(type.NormalizeInstantiation() == type);
Debug.Assert(ConstructedEETypeNode.CreationAllowed(type));
return _constructedMethodTables.Contains(type) || _canonConstructedMethodTables.Contains(type);
}

public override bool CanInterfaceOrCanonicalFormOfItBeImplementedByConstructedMethodTable(TypeDesc type)
{
Debug.Assert(type.NormalizeInstantiation() == type);
Debug.Assert(type.IsInterface);
return _implementedInterfaces.Contains(type) || _canonImplementedInterfaces.Contains(type);
}

public override TypeDesc[] GetImplementingClasses(TypeDesc type)
{
if (_disqualifiedTypes.Contains(type))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2241,8 +2241,25 @@ private bool CanNeverHaveInstanceOfSubclassOf(TypeDesc type)
if (!ConstructedEETypeNode.CreationAllowed(type))
return false;

// TODO: rather conservative
if (type.HasVariance)
return false;

TypeDesc canonType = type.ConvertToCanonForm(CanonicalFormKind.Specific);

// Interface optimization may be able to remove MethodTable of interfaces if they were never actually
// used to call/cast. Since getExactClasses can be called for things like "is a location of this
// type ever going to be non-null", we need to make sure the ability to optimize the interface MethodTable away
// doesn't lead to saying "this will always be null".
if (type.IsInterface)
{
if (_compilation.CanInterfaceOrCanonicalFormOfItBeImplementedByConstructedMethodTable(type)
|| (type != canonType && _compilation.CanInterfaceBeImplementedByConstructedMethodTable(canonType)))
{
return false;
}
}

// If we don't have a constructed MethodTable for the exact type or for its template,
// this type or any of its subclasses can never be instantiated.
return !_compilation.CanReferenceConstructedTypeOrCanonicalFormOfType(type)
Expand Down
Loading