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 SwiftIndirectResult in Mono JIT and Interpreter #104111

Merged
merged 7 commits into from
Jul 3, 2024

Conversation

kotlarmilos
Copy link
Member

Description

This PR adds Mono support for SwiftIndirectResult in Swift calling convention. This type provides direct access to the return result buffer needed for returning non-frozen types in Swift.

Changes

This PR includes both Mono JIT and Interpreter changes. On arm64, the indirect return buffer x8 is assigned when SwiftIndirectResult is passed as an argument, requiring no additional handling. On amd64, rax is the indirect return buffer, but it is heavily used across the runtime as a scratch register. To handle this, the x10 register is assigned as a proxy, and at the P/Invoke boundary, it is copied to/from rax.

Verification

The runtime tests in Interop/Swift/SwiftIndirectResult should verify the functionality.

Additional notes

Additional testing coverage will be added as we progress with projection tooling changes.

Contributes to #102082

@kotlarmilos
Copy link
Member Author

/azp run runtime-extra-platforms

Copy link
Contributor

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

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek
Copy link
Member

I think I don't understand SwiftIndirectResult and non-frozen structs. Looking at the sample:

 private struct NonFrozenStruct
    {
        public int A;
        public int B;
        public int C;
    }

    [UnmanagedCallConv(CallConvs = new Type[] { typeof(CallConvSwift) })]
    [DllImport(SwiftLib, EntryPoint = "$s19SwiftIndirectResult21ReturnNonFrozenStruct1a1b1cAA0efG0Vs5Int32V_A2ItF")]
    public static extern void ReturnNonFrozenStruct(SwiftIndirectResult result, int a, int b, int c);

 [Fact]
    public static void TestReturnNonFrozenStruct()
    {
        // In normal circumstances this instance would have unknown/dynamically determined size.
        NonFrozenStruct instance;
        ReturnNonFrozenStruct(new SwiftIndirectResult(&instance), 10, 20, 30);
        Assert.Equal(10, instance.A);
        Assert.Equal(20, instance.B);
        Assert.Equal(30, instance.C);
    }

How does Swift know that the caller allocated a large enough buffer so the callee can rely on being able to write into it?

My understanding of non-frozen Swift types is that they allow you to link two different versions of NonFrozenStruct where one of them can have more members than the other?

@jakobbotsch
Copy link
Member

jakobbotsch commented Jun 27, 2024

How does Swift know that the caller allocated a large enough buffer so the callee can rely on being able to write into it?

There is metadata created and exported by the callee module that, among other things, includes the size of the non-frozen struct. The caller is supposed to allocate a buffer of that size. I didn't do that in the test.

@kotlarmilos's original issue has some (pseudo-)code that is more faithful to what the interop generator would end up with: #102082

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@matouskozak matouskozak left a comment

Choose a reason for hiding this comment

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

LGTM!

src/mono/mono/mini/method-to-ir.c Show resolved Hide resolved
Copy link
Member

@jkurdek jkurdek left a comment

Choose a reason for hiding this comment

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

Looks good!

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.

6 participants