This repository has been archived by the owner on Nov 1, 2020. It is now read-only.
Implement ldvirtftn in shared generic contexts #2339
Merged
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.
This is to unblock #2308.
Requires JIT changes from dotnet/coreclr#8608.
This is two commits: first one is a bugfix uncovered by #2308. The second implements a missing feature required to support ldvirtftn in shared generic context.
I'm also restricting the number of cases when we actually need a generic lookup to calls/ldftn of interfaces and generic virtual methods. Normal virtual method calls/ldvirtfn go through the vtable slots that are the same between various canonically equivalent instantiations and don't need a runtime lookup.
On Project N, generic dictionaries point to the interface dispatch cell. The process of invoking/loading-the-address-of an interface method consists of a generic lookup for the interface dispatch cell, followed by a call to RhpResolveInterfaceMethod. This doesn't map great to what RyuJIT does (generic lookup followed by "mov rcx, this / call resultOfGenericLookup"). I'm making the dictionary point to a stub to do the dispatch for now. This has obvious delegate equivalency problems. I'll be filing an issue for the feature work to fix this.