Skip to content

Commit

Permalink
Restore EETypeNode to the state before we deleted reflection blocking
Browse files Browse the repository at this point in the history
Contributes to dotnet#91704.

When we deleted reflection blocking in dotnet#85810 we had to update `EETypeNode`/`ConstructedEETypeNode` to ensure any `MethodTable` also has metadata (constructed or not). This was needed because of how reflection was structured - we couldn't create a `RuntimeType` without knowing about the metadata. After the refactor in dotnet#93440, metadata is no longer a prerequisite to constructing a `RuntimeType`.

The compiler can go back to optimizing away the metadata. This affects things like `typeof(Foo) == bar.GetType()`. No metadata is needed for this (and we do optimized the `Foo` MethodTable to unconstructed one) but we still had to generate metadata.

Besides the rollback of EEType to the previous shape, this also has a bugfix for dotnet#91988 that was found later - interface types used in cast/dispatch should be considered constructed.

I'm seeing 0.1 - 0.7% size saving.
  • Loading branch information
MichalStrehovsky committed Nov 2, 2023
1 parent 655b177 commit 9c97097
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,16 @@ public ISymbolNode ComputeConstantLookup(ReadyToRunHelperId lookupKind, object t
case ReadyToRunHelperId.TypeHandleForCasting:
{
var type = (TypeDesc)targetOfLookup;

// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable.
// If this cast happens with an object that implements IDynamicIntefaceCastable, user code will
// see a RuntimeTypeHandle representing this interface.
if (type.IsInterface)
return NodeFactory.MaximallyConstructableType(type);

if (type.IsNullable)
targetOfLookup = type.Instantiation[0];
return NecessaryTypeSymbolIfPossible((TypeDesc)targetOfLookup);
type = type.Instantiation[0];
return NecessaryTypeSymbolIfPossible(type);
}
case ReadyToRunHelperId.MethodDictionary:
return NodeFactory.MethodGenericDictionary((MethodDesc)targetOfLookup);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
// relocs to nodes we emit.
dependencyList.Add(factory.NecessaryTypeSymbol(_type), "NecessaryType for constructed type");

if (_type is MetadataType mdType)
ModuleUseBasedDependencyAlgorithm.AddDependenciesDueToModuleUse(ref dependencyList, factory, mdType.Module);

DefType closestDefType = _type.GetClosestDefType();

if (_type.IsArray)
Expand Down Expand Up @@ -66,6 +69,9 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
}
}

// Ask the metadata manager if we have any dependencies due to the presence of the EEType.
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencyList, factory, _type);

factory.InteropStubManager.AddInterestingInteropConstructedTypeDependencies(ref dependencyList, factory, _type);

return dependencyList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -633,12 +633,16 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
}
}

// Ask the metadata manager
// if we have any dependencies due to presence of the EEType.
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencies, factory, _type);
if (!ConstructedEETypeNode.CreationAllowed(_type))
{
// If necessary MethodTable is the highest load level for this type, ask the metadata manager
// if we have any dependencies due to presence of the EEType.
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencies, factory, _type);

if (_type is MetadataType mdType)
ModuleUseBasedDependencyAlgorithm.AddDependenciesDueToModuleUse(ref dependencies, factory, mdType.Module);
// If necessary MethodTable is the highest load level, consider this a module use
if (_type is MetadataType mdType)
ModuleUseBasedDependencyAlgorithm.AddDependenciesDueToModuleUse(ref dependencies, factory, mdType.Module);
}

if (_type.IsFunctionPointer)
FunctionPointerMapNode.GetHashtableDependencies(ref dependencies, factory, (FunctionPointerType)_type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
result.Add(factory.ExternSymbol("RhpInitialDynamicInterfaceDispatch"), "Initial interface dispatch stub");
}

result.Add(factory.NecessaryTypeSymbol(_targetMethod.OwningType), "Interface type");
// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable.
// If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will
// see a RuntimeTypeHandle representing this interface.
result.Add(factory.ConstructedTypeSymbol(_targetMethod.OwningType), "Interface type");

return result;
}
Expand All @@ -88,7 +91,10 @@ public override void EncodeData(ref ObjectDataBuilder objData, NodeFactory facto
objData.EmitPointerReloc(factory.ExternSymbol("RhpInitialDynamicInterfaceDispatch"));
}

IEETypeNode interfaceType = factory.NecessaryTypeSymbol(_targetMethod.OwningType);
// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable.
// If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will
// see a RuntimeTypeHandle representing this interface.
IEETypeNode interfaceType = factory.ConstructedTypeSymbol(_targetMethod.OwningType);
if (factory.Target.SupportsRelativePointers)
{
if (interfaceType.RepresentsIndirectionCell)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ public static void Run()
Console.WriteLine(s_type == typeof(Never));

#if !DEBUG
ThrowIfPresentWithUsableMethodTable(typeof(TestTypeEquals), nameof(Never));
ThrowIfPresent(typeof(TestTypeEquals), nameof(Never));
#endif
}
}
Expand Down Expand Up @@ -371,7 +371,7 @@ public static void Run()

// We only expect to be able to get rid of it when optimizing
#if !DEBUG
ThrowIfPresentWithUsableMethodTable(typeof(TestBranchesInGenericCodeRemoval), nameof(Unused));
ThrowIfPresent(typeof(TestBranchesInGenericCodeRemoval), nameof(Unused));
#endif
ThrowIfNotPresent(typeof(TestBranchesInGenericCodeRemoval), nameof(Used));

Expand Down

0 comments on commit 9c97097

Please sign in to comment.