-
Notifications
You must be signed in to change notification settings - Fork 5.4k
JIT: Devirtualize shared generic virtual methods #123323
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
Open
hez2010
wants to merge
52
commits into
dotnet:main
Choose a base branch
from
hez2010:gvm-devirt-shared
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+431
−216
Open
Changes from all commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
789055d
Support for shared GVM devirt that doesn't require a runtime lookup
hez2010 555ba83
Update src/coreclr/vm/jitinterface.cpp
hez2010 25c0967
A better way to check runtime lookup
hez2010 65a61bb
Add a TODO
hez2010 8d74fdb
Merge branch 'main' into gvm-devirt-shared
hez2010 c8b7f69
Add support for runtime lookup as well
hez2010 0aea180
Try another approach
hez2010 5fbd527
Nit
hez2010 a3d71c0
Merge branch 'main' into gvm-devirt-shared
hez2010 eb692c5
Introduce a new kind of dictionary entry and switch to instParamLookup
hez2010 abd5db2
Save resolved token
hez2010 2eff218
Merge branch 'main' into gvm-devirt-shared
hez2010 2178332
Also assert isArrayInterface
hez2010 d4c9f91
Always use impRuntimeLookupToTree
hez2010 e96d509
Nit
hez2010 21623ad
Fix inlining in case of runtime lookup not available (R2R)
hez2010 bc7d210
Address a potential AV
hez2010 aeca172
Always call getInstantiatedEntry
hez2010 aa460d5
Include instantiation in the method name
hez2010 263a6ae
Add more information to the dump
hez2010 eaf4493
Remove duplicated class name
hez2010 264058e
Merge branch 'main' into gvm-devirt-shared
hez2010 19da409
Use getLookupTree
hez2010 9c5faa7
Disable runtime lookup support in R2R
hez2010 ecdf97f
Merge branch 'main' into gvm-devirt-shared
hez2010 2fb9e79
Remove unused ctxTree
hez2010 53a52d4
Get rid of getInstantiatedEntry and prevent computing a runtime looku…
hez2010 264dc62
Merge branch 'gvm-devirt-shared' of github.com:hez2010/runtime into g…
hez2010 d994cb1
Guard lookupToken
hez2010 3994537
More fixes
hez2010 014fb46
Fix
hez2010 fc995e5
Bail NativeAOT
hez2010 583277b
Make runtime lookup tests really do runtime lookup
hez2010 1aea555
Fix R2R const lookup
hez2010 d5e3485
Bail on runtime lookup for R2R
hez2010 267e187
R2R runtime lookup support
hez2010 ca8f876
Merge branch 'main' into gvm-devirt-shared
hez2010 757043d
Use the correct impl method token
hez2010 2906173
Remove the no longer necessary assertion
hez2010 4b2a1e7
Merge branch 'main' into gvm-devirt-shared
hez2010 b7a9ce9
Merge branch 'main' into gvm-devirt-shared
hez2010 a183861
Rework runtime lookup computation for devirtualized shared generics i…
hez2010 63df39e
Bail on non-exact array interface devirt
hez2010 d9f2476
Nit
hez2010 ee80835
Merge branch 'main' into gvm-devirt-shared
hez2010 d87e421
Build helperArg from templateMethod
hez2010 d326492
Pass the correct token
hez2010 c90906c
Cleanup lookupToken
hez2010 ac7c3e2
JIT format
hez2010 154f832
Disable runtime lookup support in R2R
hez2010 df87b84
Add one more test case
hez2010 ae46173
Bail on shared method table
hez2010 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1289,6 +1289,7 @@ var_types Compiler::impImportCall(OPCODE opcode, | |
| info->methodHnd = callInfo->hMethod; | ||
| info->exactContextHnd = exactContextHnd; | ||
| info->inlinersContext = compInlineContext; | ||
| info->resolvedToken = *pResolvedToken; | ||
| call->AsCall()->gtLateDevirtualizationInfo = info; | ||
| } | ||
| } | ||
|
|
@@ -7530,6 +7531,29 @@ bool Compiler::isCompatibleMethodGDV(GenTreeCall* call, CORINFO_METHOD_HANDLE gd | |
| return true; | ||
| } | ||
|
|
||
| //------------------------------------------------------------------------ | ||
| // impDevirtualizedCallHasConstInstParam: check if the instantiation argument | ||
| // is a compile-time lookup. | ||
| // | ||
| // Arguments: | ||
| // dvInfo - Devirtualization information returned by resolveVirtualMethod. | ||
| // | ||
| // Returns: | ||
| // true if instParamLookup describes a valid constant handle/address lookup. | ||
| // | ||
| bool Compiler::impDevirtualizedCallHasConstInstParam(const CORINFO_DEVIRTUALIZATION_INFO& dvInfo) | ||
| { | ||
| if (dvInfo.instParamLookup.lookupKind.needsRuntimeLookup) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return ((dvInfo.instParamLookup.constLookup.accessType == IAT_VALUE && | ||
| dvInfo.instParamLookup.constLookup.handle != nullptr) || | ||
| (dvInfo.instParamLookup.constLookup.accessType == IAT_PVALUE && | ||
| dvInfo.instParamLookup.constLookup.addr != nullptr)); | ||
| } | ||
|
|
||
| //------------------------------------------------------------------------ | ||
| // considerGuardedDevirtualization: see if we can profitably guess at the | ||
| // class involved in an interface or virtual call. | ||
|
|
@@ -7649,6 +7673,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, | |
| CORINFO_CONTEXT_HANDLE exactContext = dvInfo.exactContext; | ||
| CORINFO_METHOD_HANDLE exactMethod = dvInfo.devirtualizedMethod; | ||
| uint32_t exactMethodAttrs = info.compCompHnd->getMethodAttribs(exactMethod); | ||
| const bool exactNeedsMethodContext = | ||
| ((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD; | ||
|
|
||
| // NOTE: This is currently used only with NativeAOT. In theory, we could also check if we | ||
| // have static PGO data to decide which class to guess first. Presumably, this is a rare case. | ||
|
|
@@ -7663,8 +7689,9 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, | |
| } | ||
|
|
||
| addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs, | ||
| clsAttrs, likelyHood, dvInfo.needsMethodContext, | ||
| dvInfo.isInstantiatingStub, baseMethod, originalContext); | ||
| clsAttrs, likelyHood, exactNeedsMethodContext, | ||
| impDevirtualizedCallHasConstInstParam(dvInfo), baseMethod, | ||
| originalContext); | ||
| } | ||
|
|
||
| if (call->GetInlineCandidatesCount() == numExactClasses) | ||
|
|
@@ -7736,8 +7763,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, | |
|
|
||
| likelyContext = dvInfo.exactContext; | ||
| likelyMethod = dvInfo.devirtualizedMethod; | ||
| needsMethodContext = dvInfo.needsMethodContext; | ||
| instantiatingStub = dvInfo.isInstantiatingStub; | ||
| needsMethodContext = ((size_t)likelyContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD; | ||
| instantiatingStub = impDevirtualizedCallHasConstInstParam(dvInfo); | ||
| } | ||
| else | ||
| { | ||
|
|
@@ -8767,25 +8794,23 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, | |
| const bool objClassIsFinal = (objClassAttribs & CORINFO_FLG_FINAL) != 0; | ||
|
|
||
| #if defined(DEBUG) | ||
| const char* callKind = isInterface ? "interface" : "virtual"; | ||
| const char* objClassNote = "[?]"; | ||
| const char* objClassName = "?objClass"; | ||
| const char* baseClassName = "?baseClass"; | ||
| const char* baseMethodName = "?baseMethod"; | ||
| const char* callKind = isInterface ? "interface" : "virtual"; | ||
| const char* objClassNote = "[?]"; | ||
| const char* objClassName = "?objClass"; | ||
| const char* baseMethodFullName = "?baseMethod"; | ||
|
|
||
| if (verbose || doPrint) | ||
| { | ||
| objClassNote = isExact ? " [exact]" : objClassIsFinal ? " [final]" : ""; | ||
| objClassName = eeGetClassName(objClass); | ||
| baseClassName = eeGetClassName(baseClass); | ||
| baseMethodName = eeGetMethodName(baseMethod); | ||
| objClassNote = isExact ? " [exact]" : objClassIsFinal ? " [final]" : ""; | ||
| objClassName = eeGetClassName(objClass); | ||
| baseMethodFullName = eeGetMethodFullName(baseMethod); | ||
|
|
||
| if (verbose) | ||
| { | ||
| printf("\nimpDevirtualizeCall: Trying to devirtualize %s call:\n" | ||
| " class for 'this' is %s%s (attrib %08x)\n" | ||
| " base method is %s::%s\n", | ||
| callKind, objClassName, objClassNote, objClassAttribs, baseClassName, baseMethodName); | ||
| " base method is %s\n", | ||
| callKind, objClassName, objClassNote, objClassAttribs, baseMethodFullName); | ||
| } | ||
| } | ||
| #endif // defined(DEBUG) | ||
|
|
@@ -8834,26 +8859,30 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, | |
|
|
||
| info.compCompHnd->resolveVirtualMethod(&dvInfo); | ||
|
|
||
| CORINFO_METHOD_HANDLE derivedMethod = dvInfo.devirtualizedMethod; | ||
| CORINFO_CONTEXT_HANDLE exactContext = dvInfo.exactContext; | ||
| CORINFO_CLASS_HANDLE derivedClass = NO_CLASS_HANDLE; | ||
| CORINFO_RESOLVED_TOKEN* pDerivedResolvedToken = &dvInfo.resolvedTokenDevirtualizedMethod; | ||
| CORINFO_METHOD_HANDLE derivedMethod = dvInfo.devirtualizedMethod; | ||
| CORINFO_CONTEXT_HANDLE exactContext = dvInfo.exactContext; | ||
| CORINFO_CLASS_HANDLE derivedClass = NO_CLASS_HANDLE; | ||
| CORINFO_RESOLVED_TOKEN* pDerivedResolvedToken = &dvInfo.resolvedTokenDevirtualizedMethod; | ||
| const bool needsRuntimeLookup = dvInfo.instParamLookup.lookupKind.needsRuntimeLookup; | ||
| const bool needsCompileTimeLookup = impDevirtualizedCallHasConstInstParam(dvInfo); | ||
| const bool needsInstParam = needsRuntimeLookup || needsCompileTimeLookup; | ||
| const bool isArrayInterfaceDevirt = (objClassAttribs & CORINFO_FLG_ARRAY) != 0; | ||
|
|
||
| if (derivedMethod != nullptr) | ||
| { | ||
| assert(exactContext != nullptr); | ||
|
|
||
| if (((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS) | ||
| { | ||
| assert(!dvInfo.needsMethodContext); | ||
| assert(!needsInstParam); | ||
| derivedClass = (CORINFO_CLASS_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK); | ||
| } | ||
| else | ||
| { | ||
| // Array interface devirt can return a nonvirtual generic method of the non-generic SZArrayHelper class. | ||
| // Generic virtual method devirt also returns a generic method. | ||
| // | ||
| assert(call->IsGenericVirtual(this) || dvInfo.needsMethodContext); | ||
| assert(call->IsGenericVirtual(this) || isArrayInterfaceDevirt); | ||
| assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); | ||
| derivedClass = info.compCompHnd->getMethodClass(derivedMethod); | ||
| } | ||
|
|
@@ -8864,54 +8893,39 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, | |
| bool canDevirtualize = false; | ||
|
|
||
| #if defined(DEBUG) | ||
| const char* derivedClassName = "?derivedClass"; | ||
| const char* derivedMethodName = "?derivedMethod"; | ||
| const char* note = "inexact or not final"; | ||
| const char* instArg = ""; | ||
| const char* derivedMethodFullName = "?derivedMethod"; | ||
| const char* note = "inexact or not final"; | ||
| const char* instArg = ""; | ||
| #endif | ||
|
|
||
| CORINFO_METHOD_HANDLE instantiatingStub = NO_METHOD_HANDLE; | ||
|
|
||
| if (dvInfo.isInstantiatingStub) | ||
| if (derivedMethod != nullptr && needsInstParam) | ||
| { | ||
| // We should only end up with generic methods that needs a method context (eg. array interface, GVM). | ||
| // | ||
| assert(dvInfo.needsMethodContext); | ||
| if (needsCompileTimeLookup) | ||
| { | ||
| // We should only end up with generic methods that need a method context (eg. array interface, GVM). | ||
| // | ||
| assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); | ||
|
|
||
| // We don't expect NAOT to end up here, since it has Array<T> | ||
| // and normal devirtualization. | ||
| // | ||
| assert(!IsTargetAbi(CORINFO_NATIVEAOT_ABI)); | ||
| // If we don't know the array type exactly we may have the wrong interface type here. | ||
| // Bail out. | ||
| // | ||
| if (isArrayInterfaceDevirt && !isExact) | ||
| { | ||
| JITDUMP("Array interface devirt: array type is inexact, sorry.\n"); | ||
| return; | ||
| } | ||
|
|
||
| // We don't expect R2R to end up here, since it does not (yet) support | ||
| // array interface devirtualization. | ||
| // | ||
| assert(!IsAot()); | ||
| // We don't expect R2R/NAOT to end up here for array interface devirtualization. | ||
| // For NAOT, it has Array<T> and normal devirtualization. | ||
| // For R2R, we don't (yet) support array interface devirtualization. | ||
| assert(call->IsGenericVirtual(this) || !IsAot()); | ||
|
|
||
| // We don't expect there to be an existing inst param arg. | ||
| // | ||
| CallArg* const instParam = call->gtArgs.FindWellKnownArg(WellKnownArg::InstParam); | ||
| if (instParam != nullptr) | ||
| { | ||
| assert(!"unexpected inst param in virtual/interface call"); | ||
| return; | ||
| instantiatingStub = (CORINFO_METHOD_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK); | ||
| } | ||
|
|
||
| // If we don't know the array type exactly we may have the wrong interface type here. | ||
| // Bail out. | ||
| // | ||
| if (!isExact) | ||
| { | ||
| JITDUMP("Array interface devirt: array type is inexact, sorry.\n"); | ||
| return; | ||
| } | ||
|
|
||
| // We want to inline the instantiating stub. Fetch the relevant info. | ||
| // | ||
| CORINFO_CLASS_HANDLE ignored = NO_CLASS_HANDLE; | ||
| derivedMethod = info.compCompHnd->getInstantiatedEntry(derivedMethod, &instantiatingStub, &ignored); | ||
| assert(ignored == NO_CLASS_HANDLE); | ||
| assert((derivedMethod == NO_METHOD_HANDLE) || (instantiatingStub != NO_METHOD_HANDLE)); | ||
| assert(!needsCompileTimeLookup || (instantiatingStub != NO_METHOD_HANDLE)); | ||
| } | ||
|
|
||
| // If we failed to get a method handle, we can't directly devirtualize. | ||
|
|
@@ -8942,18 +8956,18 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, | |
| { | ||
| note = "final method"; | ||
| } | ||
| if (dvInfo.isInstantiatingStub) | ||
| if (needsInstParam) | ||
| { | ||
| instArg = " [instantiating stub]"; | ||
| instArg = needsCompileTimeLookup ? eeGetMethodFullName(instantiatingStub) : "runtime lookup"; | ||
| } | ||
|
|
||
| if (verbose || doPrint) | ||
| { | ||
| derivedMethodName = eeGetMethodName(derivedMethod); | ||
| derivedClassName = eeGetClassName(derivedClass); | ||
| derivedMethodFullName = eeGetMethodFullName(derivedMethod); | ||
| if (verbose) | ||
| { | ||
| printf(" devirt to %s::%s -- %s%s\n", derivedClassName, derivedMethodName, note, instArg); | ||
| printf(" devirt to %s -- %s%s%s\n", derivedMethodFullName, note, | ||
| needsInstParam ? ", instantiating stub: " : "", instArg); | ||
| gtDispTree(call); | ||
| } | ||
| } | ||
|
|
@@ -8992,24 +9006,36 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, | |
| return; | ||
| } | ||
|
|
||
| // Insert the instantiation argument when necessary. | ||
| if (needsInstParam) | ||
| { | ||
| assert(call->gtArgs.FindWellKnownArg(WellKnownArg::InstParam) == nullptr); | ||
|
|
||
| CORINFO_METHOD_HANDLE compileTimeHandle = derivedMethod; | ||
| if (needsCompileTimeLookup) | ||
| { | ||
| compileTimeHandle = instantiatingStub; | ||
| } | ||
|
|
||
| GenTree* instParam = getLookupTree(&dvInfo.instParamLookup, GTF_ICON_METHOD_HDL, compileTimeHandle); | ||
|
|
||
| if (instParam == nullptr) | ||
| { | ||
| // If we're inlining, impLookupToTree can return nullptr after recording a fatal observation. | ||
| JITDUMP("Failed to produce the lookup for devirtualized call, sorry.\n"); | ||
| return; | ||
| } | ||
|
|
||
| call->gtArgs.InsertInstParam(this, instParam); | ||
| } | ||
|
Comment on lines
9009
to
9030
|
||
|
|
||
| // All checks done. Time to transform the call. | ||
| // | ||
| assert(canDevirtualize); | ||
| Metrics.DevirtualizedCall++; | ||
|
|
||
| JITDUMP(" %s; can devirtualize\n", note); | ||
|
|
||
| if (dvInfo.isInstantiatingStub) | ||
| { | ||
| // Pass the instantiating stub method desc as the inst param arg. | ||
| // | ||
| // Note different embedding would be needed for NAOT/R2R, | ||
| // but we have ruled those out above. | ||
| // | ||
| GenTree* const instParam = gtNewIconEmbMethHndNode(instantiatingStub); | ||
| call->gtArgs.InsertInstParam(this, instParam); | ||
| } | ||
|
|
||
| // Make the updates. | ||
| call->gtFlags &= ~GTF_CALL_VIRT_VTABLE; | ||
| call->gtFlags &= ~GTF_CALL_VIRT_STUB; | ||
|
|
@@ -9039,8 +9065,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, | |
|
|
||
| if (doPrint) | ||
| { | ||
| printf("Devirtualized %s call to %s:%s; now direct call to %s:%s [%s]\n", callKind, baseClassName, | ||
| baseMethodName, derivedClassName, derivedMethodName, note); | ||
| printf("Devirtualized %s call to %s; now direct call to %s [%s]%s%s\n", callKind, baseMethodFullName, | ||
| derivedMethodFullName, note, needsInstParam ? ", instantiating stub: " : "", instArg); | ||
| } | ||
|
|
||
| // If we successfully devirtualized based on an exact or final class, | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.