-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
JIT: Add support for SwiftSelf<T> in Swift calling convention #103576
Conversation
This adds support to allow SwiftSelf<T> with a frozen struct as T. Swift allows enregistration of 'self' in these cases, but the 'self' must still be passed in the dedicated context register when the frozen struct is not enregistered, which makes this support necessary to handle as part of the calling convention. A few notes: - If `T` is not a value class we `BADCODE` - If the signature includes `SwiftSelf<T>`, then it must be the first argument of the signature, which matches how 'self' gets enregistered on the Swift side, otherwise we `BADCODE` - There is not support for reverse pinvokes for `SwiftSelf<T>`. That's because the context passed to function pointers is always a pointer, so I do not see any use case for this in reverse pinvokes. - Some care must be taken since `SwiftSelf<T>` is a generic struct whose layout generally is not going to match `T` without tail padding (until we get something like dotnet#100896).
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
cc @dotnet/jit-contrib PTAL @amanasifkhalid |
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.
LGTM, thanks!
If the signature includes SwiftSelf, then it must be the first argument of the signature, which matches how 'self' gets enregistered on the Swift side, otherwise we BADCODE
Is this codified in the API proposal? It seems odd that we enforce this for SwiftSelf<T>
but not for SwiftSelf
, nor do we enforce any ordering constraints for SwiftError
, SwiftIndirectResult
, etc.
I agree it's a bit odd. If we decide we do not want this constraint then we can do the reordering within the JIT relatively easily, but I suspect having this constraint is going to make things easier for Mono as well.
Documenting exact expected positions for all the special types seems like goodness to me. It would make sure that different libraries end up with compatible signatures for the same Swift calls. Not sure if that matters or not, but seems like a good thing. |
This adds support to allow SwiftSelf with a frozen struct as T. Swift allows enregistration of 'self' in these cases, but the 'self' must still be passed in the dedicated context register when the frozen struct is not enregistered, which makes this support necessary to handle as part of the calling convention.
A few notes:
T
is not a value class weBADCODE
SwiftSelf<T>
, then it must be the first argument of the signature, which matches how 'self' gets enregistered on the Swift side, otherwise weBADCODE
SwiftSelf<T>
. That's because the context passed to function pointers is always a pointer, so I do not see any use case for this in reverse pinvokes.SwiftSelf<T>
is a generic struct whose layout generally is not going to matchT
without tail padding (until we get something like [API Proposal]: New attribute for interop-specific struct concerns #100896).