-
Notifications
You must be signed in to change notification settings - Fork 505
Add instantiating unboxing stubs #2566
Add instantiating unboxing stubs #2566
Conversation
To support calling canonical interface methods on generic valuetypes, the compiler needs to generate unboxing+instantiating thunks that bridge the difference between two calling conventions. As a refresher: * Instance methods on shared generic valuetypes expect two arguments (aside from the arguments declared in the signature): a ByRef to the first byte of the value of the valuetype (`this`), and a generic context argument (EEType) * Interface calls expect `this` to be a reference type (with the generic context to be inferred from `this` by the callee) Instantiating and unboxing stubs bridge this by extracting a managed pointer out of a boxed valuetype, along with the EEType of the boxed valuetype (to provide the generic context) before dispatching to the instance method. We compile them by: * Pretending the unboxing stub is an instance method on a reference type with the same layout as a boxed valuetype * Having the unboxing stub load the `m_pEEType` field (to get generic context) and a byref to the actual value (to get a `this` expected by valuetype methods) * Generating a call to the instance method on the valuetype through a wrapper that has an explicit generic context parameter in it's signature.
|
|
||
| parameters[0] = Context.GetWellKnownType(WellKnownType.Object).GetKnownField("m_pEEType").FieldType; | ||
| for (int i = 0; i < _methodRepresented.Signature.Length; i++) | ||
| parameters[i + 1] = _methodRepresented.Signature[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.Signature[i]...
|
|
||
| // Does the method have a hidden parameter? | ||
| if (method.RequiresInstArg() && !isFatFunctionPointer) | ||
| if (method.RequiresInstArg() && !isFatFunctionPointer && !_compilation.TypeSystemContext.IsSpecialUnboxingThunkTargetMethod(method)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could get rid of the special casing here by making RequiresInstArg (and likely it's friends too) a virtual method on MethodDesc. Might also help when we make MDArray.Address method shared generic friendly. I'm conflicted about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not a bad idea.
| { | ||
| get | ||
| { | ||
| StringBuilder sb = new StringBuilder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if I should just move this all to NodeFactory so that I have access to NameMangler and can generate stable non-conflicting names. The only reason why this cares about names is to generate a unique symbol.
|
@davidwrighton I think I made this work without special casing in RyuJIT's importer. PTAL |
| /// If the type is not generic, returns the <paramref name="type"/>. | ||
| /// </summary> | ||
| private static InstantiatedType InstantiateAsOpen(this MetadataType type) | ||
| public static TypeDesc InstantiateAsOpen(this TypeDesc type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this terminology for Open is quite confusing. It really means instantiate such that if the type parameters from an open instantiation were substituted in, the result would be the same as the type definition. Which is really bizarre. (The open instantiation is used to refer to the uninstantiated form in a number of documents, and is the same thing as the typical instantiation.)
| /// Creates an open instantiation of a field. Given Foo<T>.Field, returns | ||
| /// Foo<!0>.Field. If the owning type is not generic, returns the <paramref name="field"/>. | ||
| /// </summary> | ||
| public static FieldDesc InstantiateAsOpen(this FieldDesc field) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same naming issue as above. I don't have a good name, but I don't like Open.
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhere in here we need a giant comment describing what's happening here. I understand what you're doing, but it wouldn't be obvious from the pile of code without knowing what the PR is supposed to do.
|
@MichalStrehovsky Yep, I think this is a correct approach. I couldn't do it this way in NUTC, as NUTC couldn't construct new types/methods, but this is viable. |
|
lgtm |
To support calling canonical interface methods on generic valuetypes,
the compiler needs to generate unboxing+instantiating thunks that bridge
the difference between two calling conventions.
As a refresher:
(aside from the arguments declared in the signature): a ByRef to the
first byte of the value of the valuetype (
this), and a generic contextargument (EEType)
thisto be a reference type (with the genericcontext to be inferred from
thisby the callee)Instantiating and unboxing stubs bridge this by extracting a managed
pointer out of a boxed valuetype, along with the EEType of the boxed
valuetype (to provide the generic context) before dispatching to the
instance method.
We compile them by:
with the same layout as a boxed valuetype
m_pEETypefield (to get genericcontext) and a byref to the actual value (to get a
thisexpected byvaluetype methods)
wrapper that has an explicit generic context parameter in it's
signature.
Fixes #2520.