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

adds new WasmImportLinkAttribute for C# and initial support for mono … #776

Merged
merged 6 commits into from
Jan 11, 2024

Conversation

silesmo
Copy link
Member

@silesmo silesmo commented Nov 30, 2023

WIP

@jsturtevant
Copy link
Collaborator

can I try this out with the nightly wasi-experimental workload? I think CI install scripts will need to be updated.

@silesmo
Copy link
Member Author

silesmo commented Dec 5, 2023

can I try this out with the nightly wasi-experimental workload? I think CI install scripts will need to be updated.

Yes the CI scripts needs to be updated. I will do that once it works as expected.

@silesmo
Copy link
Member Author

silesmo commented Dec 5, 2023

Dependent upon dotnet/runtime#95084

pub struct CSProject {
pub struct CSProject;

pub struct CSProjectLLVMBuilder {
Copy link
Collaborator

@yowl yowl Dec 5, 2023

Choose a reason for hiding this comment

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

Somewhere in here we will need to add the DIrectPInvoke instructions. The WasmImportLinkageAttribute tells the compiler to emit the wasm import attributes to LLVM, but these methods also need to be "Direct PInvokes" from a NAOT perspective https://learn.microsoft.com/en-us/dotnet/core/deploying/native-aot/interop#direct-pinvoke-calls. This means adding someting like

 <ItemGroup>
   <DirectPInvoke Include="xyz!SimpleDirectPInvokeTestFunc" />
 </ItemGroup>

where xyz is the Wasm import module name, and SimpleDirectPInvokeTestFunc is the Wasm import function name
or perhaps better, more concise

 <ItemGroup>
   <DirectPInvoke Include="xyz" />
 </ItemGroup>

which will mark all exports in the xyz module as direct P/invokes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't actually need this now, as Naot for Wasm treats all WasmImportLinkage marked methods as direct pInvoke

@silesmo silesmo marked this pull request as ready for review January 10, 2024 16:39
@silesmo
Copy link
Member Author

silesmo commented Jan 10, 2024

There are still some issues with the generated components as they can't be ran. There seems to be something incorrect with the types. But I think we can merge this for now as it doesn't break anything already existing and it allows us to actually build using mono as well and test the code gen with mono.

Copy link
Collaborator

@yowl yowl 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. The formatting changes in the top Cargo.toml are they intentional?

@silesmo silesmo merged commit 08394ce into bytecodealliance:main Jan 11, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants