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

[mono][interp] Fix unboxing during CALLI #59747

Merged
merged 2 commits into from
Oct 9, 2021
Merged

Conversation

BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Sep 29, 2021

On interp we were storing function pointers as a simple InterpMethod. The problem is that, when loading an InterpMethod with a ldvirtftn opcode, the resulting method might need to have its this pointer unboxed, so this information needs to be embedded into the function pointer. We achieve this by passing function pointers in the interpreter via an InterpFtnDesc, which contains the need_unbox information. Each InterpMethod caches the function descriptors for both unboxing scenarios.

In order to interop with llvmonly we must use a MonoFtnDesc as a function pointer so this must point to point to an InterpFtnDesc in order to resolve normal interp calls.

Contributes to #54374

@ghost
Copy link

ghost commented Sep 29, 2021

Tagging subscribers to this area: @BrzVlad
See info in area-owners.md if you want to be subscribed.

Issue Details

On interp we were storing function pointers as a simple InterpMethod. The problem is that, when loading an InterpMethod with a ldvirtftn opcode, the resulting method might need to have its this pointer unboxed, so this information needs to be embedded into the function pointer. We achieve this by passing function pointers in the interpreter via an InterpFtnDesc, which contains the need_unbox information. Each InterpMethod caches the function descriptors for both unboxing scenarios.

In order to interop with llvmonly we must use a MonoFtnDesc as a function pointer so this must point to point to an InterpFtnDesc in order to resolve normal interp calls.

Contributes to #54374

Author: BrzVlad
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@BrzVlad
Copy link
Member Author

BrzVlad commented Oct 6, 2021

Some observations about this change:

@lambdageek
Copy link
Member

lambdageek commented Oct 6, 2021

Some observations about this change:

So do you want to keep working on it, or take it as-is and do follow-up PRs?

  • I think we could avoid allocating InterpFtnDesc by just storing the InterpMethod pointer tagged whether it needs unboxing or not.

Yea that would work too.

we could also add more mint opcodes - ldvirtftn.managed that pushes 2 values to the stack (InterpMethod, need_unbox) and calli.managed that reads 2 operands. and ldvirtftn+ldftn and calli would only work for the unmanaged calling conventions. (or alternately, add new opcodes for the unmanaged function pointers and indirect calls).

@BrzVlad BrzVlad force-pushed the fix-interp-calli branch 3 times, most recently from 0e9aa2e to e5ce21b Compare October 8, 2021 12:50
On interp we were storing function pointers as a simple InterpMethod. The problem is that, when loading an InterpMethod with a ldvirtftn opcode, the resulting method might need to have its this pointer unboxed, so this information needs to be embedded into the function pointer. We achieve this by tagging the InterpMethod whether we need to perform unboxing as part of calling it.
@BrzVlad BrzVlad merged commit 9fbc728 into dotnet:main Oct 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 8, 2021
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.

2 participants