Skip to content

Commit

Permalink
Runtime support for static virtual interface methods (#52173)
Browse files Browse the repository at this point in the history
This change implements initial CoreCLR runtime support for static virtual interface methods and adds over 1700 test cases covering various aspects of this new runtime feature.

1) In the JIT, we are relaxing the assumption that ".constrained" must be always followed by a "callvirt" by allowing two more IL instructions to be constrained, "call" and "ldftn".

2) In the JIT interface, we're adding bits of code to CEEInfo::getCallInfo to properly handle the new static virtual methods. We're extending constrained method resolution to cater for the static virtual method case in a new method "ResolveStaticVirtualMethod".

3) In our work on the implementation we found a pre-existing JIT interface issue - the interplay between getCallInfo and embedGenericHandle doesn't work well in case of constrained calls as the existing API surface doesn't provide any means for communicating compile-time resolution of the constraint from getCallInfo to embedGenericHandle. In this change we're working around this by deferring to runtime resolution of the constraint in the presence of shared generics.

4) In the method table builder, we're newly tracking a flag saying whether we're implementing an interface declaring static virtual methods (fHasVirtualStaticMethods) that is used to speed up the runtime checks by skipping irrelevant interfaces.

5) For Crossgen / Crossgen2, this change only adds minimalistic support in the form of bailing out in getCallInfo whenever we hit a call to a static virtual method, cancelling AOT compilation of the caller and deferring it to runtime JIT.
  • Loading branch information
trylek committed May 14, 2021
1 parent 268d951 commit 88211b9
Show file tree
Hide file tree
Showing 58 changed files with 56,277 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
using System.Runtime.InteropServices;
using System.Runtime.Loader;
using System.Runtime.Serialization;
using System.Runtime.Versioning;
using System.Threading;
using Internal.Runtime.CompilerServices;

namespace System
{
[NonVersionable]
public unsafe struct RuntimeTypeHandle : ISerializable
{
// Returns handle for interop with EE. The handle is guaranteed to be non-null.
Expand Down Expand Up @@ -798,6 +800,7 @@ RuntimeMethodHandleInternal Value
}
}

[NonVersionable]
public unsafe struct RuntimeMethodHandle : ISerializable
{
// Returns handle for interop with EE. The handle is guaranteed to be non-null.
Expand Down Expand Up @@ -1122,6 +1125,7 @@ internal sealed class RuntimeFieldInfoStub : IRuntimeFieldInfo
RuntimeFieldHandleInternal IRuntimeFieldInfo.Value => m_fieldHandle;
}

[NonVersionable]
public unsafe struct RuntimeFieldHandle : ISerializable
{
// Returns handle for interop with EE. The handle is guaranteed to be non-null.
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/dlls/mscorrc/mscorrc.rc
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ BEGIN
IDS_CLASSLOAD_MI_MISSING_SIG_BODY "Signature for the body in a method implementation cannot be found. Type: '%1'. Assembly: '%2'."
IDS_CLASSLOAD_MI_MISSING_SIG_DECL "Signature for the declaration in a method implementation cannot be found. Type: '%1'. Assembly: '%2'."
IDS_CLASSLOAD_MI_BADRETURNTYPE "Return type in method '%1' on type '%2' from assembly '%3' is not compatible with base type method '%4'."
IDS_CLASSLOAD_STATICVIRTUAL_NOTIMPL "Virtual static method '%3' is not implemented on type '%1' from assembly '%2'."

IDS_CLASSLOAD_EQUIVALENTSTRUCTMETHODS "Could not load the structure '%1' from assembly '%2'. The structure is marked as eligible for type equivalence, but it has a method."
IDS_CLASSLOAD_EQUIVALENTSTRUCTFIELDS "Could not load the structure '%1' from assembly '%2'. The structure is marked as eligible for type equivalence, but it has a static or non-public field."
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/dlls/mscorrc/resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@
#define IDS_CLASSLOAD_MI_MISSING_SIG_BODY 0x17a6
#define IDS_CLASSLOAD_MI_MISSING_SIG_DECL 0x17a7
#define IDS_CLASSLOAD_MI_BADRETURNTYPE 0x17a8
#define IDS_CLASSLOAD_STATICVIRTUAL_NOTIMPL 0x17a9

#define IDS_CLASSLOAD_TOOMANYGENERICARGS 0x17ab
#define IDS_ERROR 0x17b0
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1143,9 +1143,9 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
{
OPCODE actualOpcode = impGetNonPrefixOpcode(codeAddr, codeEndp);

if (actualOpcode != CEE_CALLVIRT)
if (actualOpcode != CEE_CALLVIRT && actualOpcode != CEE_CALL && actualOpcode != CEE_LDFTN)
{
BADCODE("constrained. has to be followed by callvirt");
BADCODE("constrained. has to be followed by callvirt, call or ldftn");
}
}
goto OBSERVE_OPCODE;
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13903,7 +13903,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)

JITDUMP(" %08X", resolvedToken.token);

eeGetCallInfo(&resolvedToken, nullptr /* constraint typeRef*/,
eeGetCallInfo(&resolvedToken, (prefixFlags & PREFIX_CONSTRAINED) ? &constrainedResolvedToken : nullptr,
addVerifyFlag(combine(CORINFO_CALLINFO_SECURITYCHECKS, CORINFO_CALLINFO_LDFTN)),
&callInfo);

Expand Down Expand Up @@ -14074,9 +14074,9 @@ void Compiler::impImportBlockCode(BasicBlock* block)

{
OPCODE actualOpcode = impGetNonPrefixOpcode(codeAddr, codeEndp);
if (actualOpcode != CEE_CALLVIRT)
if (actualOpcode != CEE_CALLVIRT && actualOpcode != CEE_CALL && actualOpcode != CEE_LDFTN)
{
BADCODE("constrained. has to be followed by callvirt");
BADCODE("constrained. has to be followed by callvirt, call or ldftn");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1818,6 +1818,13 @@ private void VerifyMethodSignatureIsStable(MethodSignature methodSig)

private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, CORINFO_METHOD_STRUCT_* callerHandle, CORINFO_CALLINFO_FLAGS flags, CORINFO_CALL_INFO* pResult)
{
if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) == 0 && pConstrainedResolvedToken != null)
{
// Defer constrained call / ldftn instructions used for static virtual methods
// to runtime resolution.
throw new RequiresRuntimeJitException("SVM");
}

MethodDesc methodToCall;
MethodDesc targetMethod;
TypeDesc constrainedType;
Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/vm/genmeth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,10 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD,
pExactMT->GetCanonicalMethodTable(),
FALSE,
Instantiation(repInst, methodInst.GetNumArgs()),
TRUE);
/* allowInstParam */ TRUE,
/* forceRemotableMethod */ FALSE,
/* allowCreate */ TRUE,
/* level */ level);

_ASSERTE(pWrappedMD->IsSharedByGenericInstantiations());
_ASSERTE(!methodInst.IsEmpty() || !pWrappedMD->IsSharedByGenericMethodInstantiations());
Expand Down
64 changes: 56 additions & 8 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5124,7 +5124,7 @@ void CEEInfo::getCallInfo(
TypeHandle exactType = TypeHandle(pResolvedToken->hClass);

TypeHandle constrainedType;
if ((flags & CORINFO_CALLINFO_CALLVIRT) && (pConstrainedResolvedToken != NULL))
if (pConstrainedResolvedToken != NULL)
{
constrainedType = TypeHandle(pConstrainedResolvedToken->hClass);
}
Expand Down Expand Up @@ -5352,13 +5352,37 @@ void CEEInfo::getCallInfo(
// Direct calls to abstract methods are not allowed
if (IsMdAbstract(dwTargetMethodAttrs) &&
// Compensate for always treating delegates as direct calls above
!(((flags & CORINFO_CALLINFO_LDFTN) && (flags & CORINFO_CALLINFO_CALLVIRT) && !resolvedCallVirt)))
!(((flags & CORINFO_CALLINFO_LDFTN) && (flags & CORINFO_CALLINFO_CALLVIRT) && !resolvedCallVirt))
&& !(IsMdStatic(dwTargetMethodAttrs) && fForceUseRuntimeLookup))
{
COMPlusThrowHR(COR_E_BADIMAGEFORMAT, BFA_BAD_IL);
}

bool allowInstParam = (flags & CORINFO_CALLINFO_ALLOWINSTPARAM);

// If the target method is resolved via constrained static virtual dispatch
// And it requires an instParam, we do not have the generic dictionary infrastructure
// to load the correct generic context arg via EmbedGenericHandle.
// Instead, force the call to go down the CORINFO_CALL_CODE_POINTER code path
// which should have somewhat inferior performance. This should only actually happen in the case
// of shared generic code calling a shared generic implementation method, which should be rare.
//
// An alternative design would be to add a new generic dictionary entry kind to hold the MethodDesc
// of the constrained target instead, and use that in some circumstances; however, implementation of
// that design requires refactoring variuos parts of the JIT interface as well as
// TryResolveConstraintMethodApprox. In particular we would need to be abled to embed a constrained lookup
// via EmbedGenericHandle, as well as decide in TryResolveConstraintMethodApprox if the call can be made
// via a single use of CORINFO_CALL_CODE_POINTER, or would be better done with a CORINFO_CALL + embedded
// constrained generic handle, or if there is a case where we would want to use both a CORINFO_CALL and
// embedded constrained generic handle. Given the current expected high performance use case of this feature
// which is generic numerics which will always resolve to exact valuetypes, it is not expected that
// the complexity involved would be worth the risk. Other scenarios are not expected to be as performance
// sensitive.
if (IsMdStatic(dwTargetMethodAttrs) && constrainedType != NULL && pResult->exactContextNeedsRuntimeLookup)
{
allowInstParam = FALSE;
}

// Create instantiating stub if necesary
if (!allowInstParam && pTargetMD->RequiresInstArg())
{
Expand Down Expand Up @@ -5401,9 +5425,20 @@ void CEEInfo::getCallInfo(
{
pResult->kind = CORINFO_CALL_CODE_POINTER;

// For reference types, the constrained type does not affect method resolution
DictionaryEntryKind entryKind = (!constrainedType.IsNull() && constrainedType.IsValueType()) ? ConstrainedMethodEntrySlot : MethodEntrySlot;

DictionaryEntryKind entryKind;
if (constrainedType.IsNull() || ((flags & CORINFO_CALLINFO_CALLVIRT) && !constrainedType.IsValueType()))
{
// For reference types, the constrained type does not affect method resolution on a callvirt, and if there is no
// constraint, it doesn't effect it either
entryKind = MethodEntrySlot;
}
else
{
// constrained. callvirt case where the constraint type is a valuetype
// OR
// constrained. call or constrained. ldftn case
entryKind = ConstrainedMethodEntrySlot;
}
ComputeRuntimeLookupForSharedGenericToken(entryKind,
pResolvedToken,
pConstrainedResolvedToken,
Expand Down Expand Up @@ -5706,7 +5741,19 @@ void CEEInfo::getCallInfo(

pResult->methodFlags = getMethodAttribsInternal(pResult->hMethod);

SignatureKind signatureKind = flags & CORINFO_CALLINFO_CALLVIRT && !(pResult->kind == CORINFO_CALL) ? SK_VIRTUAL_CALLSITE : SK_CALLSITE;
SignatureKind signatureKind;
if (flags & CORINFO_CALLINFO_CALLVIRT && !(pResult->kind == CORINFO_CALL))
{
signatureKind = SK_VIRTUAL_CALLSITE;
}
else if ((pResult->kind == CORINFO_CALL_CODE_POINTER) && IsMdVirtual(dwTargetMethodAttrs) && IsMdStatic(dwTargetMethodAttrs))
{
signatureKind = SK_STATIC_VIRTUAL_CODEPOINTER_CALLSITE;
}
else
{
signatureKind = SK_CALLSITE;
}
getMethodSigInternal(pResult->hMethod, &pResult->sig, (pResult->hMethod == pResolvedToken->hMethod) ? pResolvedToken->hClass : NULL, signatureKind);

if (flags & CORINFO_CALLINFO_VERIFICATION)
Expand Down Expand Up @@ -8639,9 +8686,10 @@ CEEInfo::getMethodSigInternal(
// Otherwise we would end up with two secret generic dictionary arguments (since the stub also provides one).
//
BOOL isCallSiteThatGoesThroughInstantiatingStub =
signatureKind == SK_VIRTUAL_CALLSITE &&
(signatureKind == SK_VIRTUAL_CALLSITE &&
!ftn->IsStatic() &&
ftn->GetMethodTable()->IsInterface();
ftn->GetMethodTable()->IsInterface()) ||
signatureKind == SK_STATIC_VIRTUAL_CODEPOINTER_CALLSITE;
if (!isCallSiteThatGoesThroughInstantiatingStub)
sigRet->callConv = (CorInfoCallConv) (sigRet->callConv | CORINFO_CALLCONV_PARAMTYPE);
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/jitinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum SignatureKind
SK_NOT_CALLSITE,
SK_CALLSITE,
SK_VIRTUAL_CALLSITE,
SK_STATIC_VIRTUAL_CODEPOINTER_CALLSITE,
};

class Stub;
Expand Down
15 changes: 10 additions & 5 deletions src/coreclr/vm/memberload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,8 @@ FieldDesc * MemberLoader::GetFieldDescFromMemberRefAndType(Module * pModule,
//
MethodDesc* MemberLoader::GetMethodDescFromMethodDef(Module *pModule,
mdToken MethodDef,
BOOL strictMetadataChecks)
BOOL strictMetadataChecks,
ClassLoadLevel owningTypeLoadLevel)
{
CONTRACTL
{
Expand Down Expand Up @@ -623,7 +624,7 @@ MethodDesc* MemberLoader::GetMethodDescFromMethodDef(Module *pModule,
}
}

pMD->CheckRestore();
pMD->CheckRestore(owningTypeLoadLevel);

#if 0
// <TODO> Generics: enable this check after the findMethod call in the Zapper which passes
Expand Down Expand Up @@ -713,7 +714,8 @@ MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec(
BOOL strictMetadataChecks,
// Normally true - the zapper is one exception. Throw an exception if no generic method args
// given for a generic method, otherwise return the 'generic' instantiation
BOOL allowInstParam)
BOOL allowInstParam,
ClassLoadLevel owningTypeLoadLevel)
{
CONTRACTL
{
Expand All @@ -738,7 +740,7 @@ MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec(
switch (TypeFromToken(MemberRef))
{
case mdtMethodDef:
pMD = GetMethodDescFromMethodDef(pModule, MemberRef, strictMetadataChecks);
pMD = GetMethodDescFromMethodDef(pModule, MemberRef, strictMetadataChecks, owningTypeLoadLevel);
th = pMD->GetMethodTable();
break;

Expand Down Expand Up @@ -770,7 +772,10 @@ MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec(
th.GetMethodTable(),
FALSE /* don't get unboxing entry point */,
strictMetadataChecks ? Instantiation() : pMD->LoadMethodInstantiation(),
allowInstParam);
allowInstParam,
/* forceRemotableMethod */ FALSE,
/* allowCreate */ TRUE,
/* level */ owningTypeLoadLevel);
} // MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec

//---------------------------------------------------------------------------------------
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/vm/memberload.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class MemberLoader
mdToken MemberRefOrDefOrSpec,
const SigTypeContext *pTypeContext, // Context for type parameters in any parent TypeSpec and in the instantiation in a MethodSpec
BOOL strictMetadataChecks, // Normally true - the zapper is one exception. Throw an exception if no generic method args given for a generic method, otherwise return the 'generic' instantiation
BOOL allowInstParam);
BOOL allowInstParam,
ClassLoadLevel owningTypeLoadLevel = CLASS_LOADED);

static FieldDesc* GetFieldDescFromMemberDefOrRef(Module *pModule,
mdMemberRef MemberDefOrRef,
Expand All @@ -92,7 +93,8 @@ class MemberLoader

static MethodDesc* GetMethodDescFromMethodDef(Module *pModule,
mdToken MethodDef,
BOOL strictMetadataChecks);
BOOL strictMetadataChecks,
ClassLoadLevel owningTypeLoadLevel = CLASS_LOADED);

static FieldDesc* GetFieldDescFromFieldDef(Module *pModule,
mdToken FieldDef,
Expand Down
Loading

0 comments on commit 88211b9

Please sign in to comment.