Skip to content

Fix handling of array constructors in Crossgen2 #48557

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.

Sign up for GitHub

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

Merged
trylek merged 3 commits into from
Mar 30, 2021
Merged

Fix handling of array constructors in Crossgen2 #48557

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e7ad90
Fix handling of array constructors in Crossgen2
trylek Feb 20, 2021
9ddf834
Ramping up on the method descriptor handle in CoreCLR
trylek Feb 21, 2021
652ca4d
Improve the change based on Michal's PR feedback
trylek Feb 21, 2021
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
40 changes: 29 additions & 11 deletions src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1334,7 +1334,7 @@ private void ceeInfoGetCallInfo(
// Its basic meaning is that shared generic methods always need instantiating
// stubs as the shared generic code needs the method dictionary parameter that cannot
// be provided by other means.
useInstantiatingStub = originalMethod.GetCanonMethodTarget(CanonicalFormKind.Specific).RequiresInstMethodDescArg();
useInstantiatingStub = originalMethod.OwningType.IsArray || originalMethod.GetCanonMethodTarget(CanonicalFormKind.Specific).RequiresInstMethodDescArg();

callerMethod = HandleToObject(callerHandle);

Expand Down Expand Up @@ -1534,7 +1534,8 @@ private void ceeInfoGetCallInfo(
}

methodToCall = targetMethod;
MethodDesc canonMethod = targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific);
bool isArrayConstructor = targetMethod.OwningType.IsArray && targetMethod.IsConstructor;
MethodDesc canonMethod = (isArrayConstructor ? null : targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific));

if (directCall)
{
Expand All @@ -1548,7 +1549,7 @@ private void ceeInfoGetCallInfo(

bool allowInstParam = (flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_ALLOWINSTPARAM) != 0;

if (!allowInstParam && canonMethod.RequiresInstArg())
if (!allowInstParam && canonMethod != null && canonMethod.RequiresInstArg())
{
useInstantiatingStub = true;
}
Expand All @@ -1572,7 +1573,17 @@ private void ceeInfoGetCallInfo(
const CORINFO_CALLINFO_FLAGS LdVirtFtnMask = CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN | CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT;
bool unresolvedLdVirtFtn = ((flags & LdVirtFtnMask) == LdVirtFtnMask) && !resolvedCallVirt;

if ((pResult->exactContextNeedsRuntimeLookup && useInstantiatingStub && (!allowInstParam || resolvedConstraint)) || forceUseRuntimeLookup)
if (isArrayConstructor)
{
// Constructors on arrays are special and don't actually have entrypoints.
// That would be fine by itself and wouldn't need special casing. But
// constructors on SzArray have a weird property that causes them not to have canonical forms.
// int[][] has a .ctor(int32,int32) to construct the jagged array in one go, but its canonical
// form of __Canon[] doesn't have the two-parameter constructor. The canonical form would need
// to have an unlimited number of constructors to cover stuff like "int[][][][][][]..."
pResult->kind = CORINFO_CALL_KIND.CORINFO_CALL;
}
else if ((pResult->exactContextNeedsRuntimeLookup && useInstantiatingStub && (!allowInstParam || resolvedConstraint)) || forceUseRuntimeLookup)
{
if (unresolvedLdVirtFtn)
{
Expand All @@ -1597,7 +1608,7 @@ private void ceeInfoGetCallInfo(
if (allowInstParam)
{
useInstantiatingStub = false;
methodToCall = canonMethod;
methodToCall = canonMethod ?? methodToCall;
}

pResult->kind = CORINFO_CALL_KIND.CORINFO_CALL;
Expand Down Expand Up @@ -1824,12 +1835,19 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO
nonUnboxingMethod = rawPinvoke.Target;
}

// READYTORUN: FUTURE: Direct calls if possible
pResult->codePointerOrStubLookup.constLookup = CreateConstLookupToSymbol(
_compilation.NodeFactory.MethodEntrypoint(
ComputeMethodWithToken(nonUnboxingMethod, ref pResolvedToken, constrainedType, unboxing: isUnboxingStub),
isInstantiatingStub: useInstantiatingStub,
isPrecodeImportRequired: (flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0));
if (methodToCall.OwningType.IsArray && methodToCall.IsConstructor)
{
pResult->codePointerOrStubLookup.constLookup = default;
}
else
{
// READYTORUN: FUTURE: Direct calls if possible
pResult->codePointerOrStubLookup.constLookup = CreateConstLookupToSymbol(
_compilation.NodeFactory.MethodEntrypoint(
ComputeMethodWithToken(nonUnboxingMethod, ref pResolvedToken, constrainedType, unboxing: isUnboxingStub),
isInstantiatingStub: useInstantiatingStub,
isPrecodeImportRequired: (flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0));
}

// If the abi of the method isn't stable, this will cause a usage of the RequiresRuntimeJitSymbol, which will trigger a RequiresRuntimeJitException
UpdateConstLookupWithRequiresRuntimeJitSymbolIfNeeded(ref pResult->codePointerOrStubLookup.constLookup, targetMethod);
Expand Down