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

[Swift Interop] Lowering fixed buffers #107620

Open
Tracked by #108662
jkurdek opened this issue Sep 10, 2024 · 8 comments
Open
Tracked by #108662

[Swift Interop] Lowering fixed buffers #107620

jkurdek opened this issue Sep 10, 2024 · 8 comments

Comments

@jkurdek
Copy link
Member

jkurdek commented Sep 10, 2024

[StructLayout(LayoutKind.Sequential, Size = 16)]
public unsafe struct Data
{
    public fixed byte bytes[16];
}

Running the swift lowering algorithm on the following struct results in incorrect lowering. I did debug on mono and the resulting lowering is a single unsigned byte. I would expect this to be lowered into two int64.

@jkoritzinsky should such constructs be supported by the swift struct lowering algorithm?

cc: @kotlarmilos

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Sep 10, 2024
@jkoritzinsky
Copy link
Member

Yes, I would expect fixed buffers to be supported for Swift lowering. I believe NativeAOT and CoreCLR already implement this correctly, but I may be misremembering.

@jkurdek
Copy link
Member Author

jkurdek commented Sep 10, 2024

I'm pretty sure CoreCLR (preview 7) side doesn't work as well. I tried passing this struct as parameter to swift using coreclr backend and failed, which got me looking at runtime details in the first place. I haven't debuged on coreclr so I can't say whether the behaviour is the same as on mono.

@jkoritzinsky
Copy link
Member

Looking at this again, it looks like we added support for InlineArray types, not the legacy fixed buffers.

I think it's fine say that legacy fixed buffers aren't supported due to the difficulties of recognizing legacy fixed buffers at the runtime level and the existing limitations of fixed buffers in interop scenarios. What do you think?

@agocke agocke added this to AppModel Sep 11, 2024
@jkurdek
Copy link
Member Author

jkurdek commented Sep 12, 2024

I do think that InlineArray should be sufficient. @kotlarmilos we will have to adjust the projection here dotnet/runtimelab#2584

We do lack the support for struct lowering them in mono though. Do we have a tracking issue for adding this to mono, or should I create one?

The only thing which worries me about fixed buffers is that lowering them results in unexpected result. I do not know what should the convention be. Should we report an error when trying to lower unsupported type?

@kotlarmilos
Copy link
Member

We do lack the support for struct lowering them in mono though. Do we have a tracking issue for adding this to mono, or should I create one?

Yes, please create a tracking issue for InlineArray in Mono.

The only thing which worries me about fixed buffers is that lowering them results in unexpected result. I do not know what should the convention be. Should we report an error when trying to lower unsupported type?

Yes, that would be ideal.

@jkurdek
Copy link
Member Author

jkurdek commented Sep 13, 2024

Support for InlineArray in mono was fixed with #107744

@jkurdek jkurdek removed the untriaged New issue has not been triaged by the area owner label Sep 13, 2024
@jkurdek
Copy link
Member Author

jkurdek commented Sep 13, 2024

We can leave this issue to track the effort on handling the fixed buffer. I do think that we should eventually either implement support for it or detect it and raise an appropriate error.

Copy link
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

4 participants