Skip to content
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

Add support for creating delegates to static virtual methods #66936

Merged
merged 2 commits into from
Mar 29, 2022

Conversation

MichalStrehovsky
Copy link
Member

Requires a JitInterface change because we need to be able to pass information about constraints to getReadyToRunDelegateCtorHelper

Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JIT part looks ok to me.

@jkotas
Copy link
Member

jkotas commented Mar 24, 2022

Ok, done. It's a bit of a misuse but I couldn't find a better spot to transfer this information within RyuJIT.

Can you change typeInfo to store a new MethodPointerInfo structure instead CORINFO_RESOLVED_TOKEN that contains the resolved token and anything else that is convenient to keep around for later? I think it would be much cleaner than attaching the information to CORINFO_RESOLVED_TOKEN.

In general, directly reusing JIT/EE interface structures for bookkeeping in the JIT tends to work poorly. It is best to decouple the two purposes whenever we run into problems.

@MichalStrehovsky
Copy link
Member Author

Can you change typeInfo to store a new MethodPointerInfo structure instead CORINFO_RESOLVED_TOKEN

Looks like that's going to be much cleaner, thanks! Looks like typeInfo tried to be too general purpose in this sense and that made things confusing:

  • typeInfo::IsToken() will only be true for the loading of a function pointer case. typeInfo::m_token is only used for the function pointers as well.
  • typeInfo::m_method is dead code.

I'll try to clean that up as part of this.

@jkotas
Copy link
Member

jkotas commented Mar 25, 2022

Formatting...

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@MichalStrehovsky MichalStrehovsky merged commit 370f160 into dotnet:main Mar 29, 2022
@MichalStrehovsky MichalStrehovsky deleted the delStaticVirt branch March 29, 2022 03:44
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
…66936)

Requires a JitInterface change because we need to be able to pass information about constraints to `getReadyToRunDelegateCtorHelper`
@ghost ghost locked as resolved and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants