Skip to content

Commit

Permalink
Remove conservative generic scanning logic
Browse files Browse the repository at this point in the history
@EgorBo's dotnet#88025 made it possible to undo conservative scanning logic added in dotnet/corert#7618.

Saves 0.4% in size for Stage1 and Stage2 apps.
  • Loading branch information
MichalStrehovsky committed Jul 4, 2023
1 parent 2fbb7eb commit 0b8f094
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 55 deletions.
3 changes: 1 addition & 2 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1202,13 +1202,12 @@ private bool getMethodInfo(CORINFO_METHOD_STRUCT_* ftn, CORINFO_METHOD_INFO* inf
}
}

#if READYTORUN
// Add an early CanInline check to see if referring to the IL of the target methods is
// permitted from within this MethodBeingCompiled, the full CanInline check will be performed
// later.
if (!_compilation.CanInline(MethodBeingCompiled, method))
return false;
#endif

MethodIL methodIL = method.IsUnboxingThunk() ? null : _compilation.GetMethodIL(method);
return Get_CORINFO_METHOD_INFO(method, methodIL, info);
}
Expand Down
53 changes: 0 additions & 53 deletions src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -601,33 +601,6 @@ private void ImportCall(ILOpcode opcode, int token)
_dependencies.Add(instParam, reason);
}

if (instParam == null
&& !targetMethod.OwningType.IsValueType
&& !_factory.TypeSystemContext.IsSpecialUnboxingThunk(_canonMethod))
{
// We have a call to a shared instance method and we're already in a shared context.
// e.g. this is a call to Foo<T>.Method() and we're about to add Foo<__Canon>.Method()
// to the dependency graph).
//
// We will pretend the runtime determined owning type (Foo<T>) got allocated as well.
// This is because RyuJIT might end up inlining the shared method body, making it concrete again,
// without actually having to go through a dictionary.
// (This would require inlining across two generic contexts, but RyuJIT does that.)
//
// If we didn't have a constructed type for this at the scanning time, we wouldn't
// know the dictionary dependencies at the inlined site, leading to a compile failure.
// (Remember that dictionary dependencies of instance methods on generic reference types
// are tied to the owning type.)
//
// This is not ideal, because if e.g. Foo<string> never got allocated otherwise, this code is
// unreachable and we're making the scanner scan more of it.
//
// Technically, we could get away with injecting a RuntimeDeterminedMethodNode here
// but that introduces more complexities and doesn't seem worth it at this time.
Debug.Assert(targetMethod.AcquiresInstMethodTableFromThis());
_dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.TypeHandle, runtimeDeterminedMethod.OwningType), reason + " - inlining protection");
}

_dependencies.Add(_factory.CanonicalEntrypoint(targetMethod), reason);
}
else
Expand Down Expand Up @@ -669,32 +642,6 @@ private void ImportCall(ILOpcode opcode, int token)
_dependencies.Add(instParam, reason);
}

if (instParam == null
&& concreteMethod != targetMethod
&& targetMethod.OwningType.NormalizeInstantiation() == targetMethod.OwningType
&& !targetMethod.OwningType.IsValueType)
{
// We have a call to a shared instance method and we still know the concrete
// type of the generic instance (e.g. this is a call to Foo<string>.Method()
// and we're about to add Foo<__Canon>.Method() to the dependency graph).
//
// We will pretend the concrete type got allocated as well. This is because RyuJIT might
// end up inlining the shared method body, making it concrete again.
//
// If we didn't have a constructed type for this at the scanning time, we wouldn't
// know the dictionary dependencies at the inlined site, leading to a compile failure.
// (Remember that dictionary dependencies of instance methods on generic reference types
// are tied to the owning type.)
//
// This is not ideal, because if Foo<string> never got allocated otherwise, this code is
// unreachable and we're making the scanner scan more of it.
//
// Technically, we could get away with injecting a ShadowConcreteMethod for the concrete
// method, but that's more complex and doesn't seem worth it at this time.
Debug.Assert(targetMethod.AcquiresInstMethodTableFromThis());
_dependencies.Add(_compilation.NodeFactory.MaximallyConstructableType(concreteMethod.OwningType), reason + " - inlining protection");
}

_dependencies.Add(GetMethodEntrypoint(targetMethod), reason);
}
}
Expand Down

0 comments on commit 0b8f094

Please sign in to comment.