Skip to content

Conversation

@yowl
Copy link
Collaborator

@yowl yowl commented Dec 15, 2023

This PR removes the WasmImport tag from the project file now that the WasmImportLinkageAttribute has been accepted upstream and merged into Ilc. Note that it is not in the published dotnet SDK yet so we need to add the attribute to the generated code until we update to dotnet 9 SDK (and tooling which supports it).

Also moves cabi_realloc to c code, fixing #777 . I did try a few things to avoid adding C code and invoking emcc to compile that code to a wasm object file.

cc @jsturtevant @silesmo

yowl added 2 commits December 14, 2023 20:23
Remove WasmImport from csproj
Move cabi-realloc to c
redo import name
namespace {namespace}.{name};
// temporarily add this attribute until it is available in dotnet 9
namespace System.Runtime.InteropServices
Copy link
Collaborator

@jsturtevant jsturtevant Dec 15, 2023

Choose a reason for hiding this comment

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

This is in a for loop so gets printed twice in some cases (which is why CI is failing)

I think we could move put this where internal static class Intrinsics was so it only gets printed once

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, Yes I have some more changes pending, but until the packages are published dotnet/runtimelab#2462, then I've left this as draft

csproj.generate()?;

// generate the cabi_realloc (and TODO: release)
let mut cmd_emcc = Command::new("emcc.bat");
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this can be a task in the csproj.cs that builds if that c file is present?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So that it builds as part of the user dotnet publish ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

that's what I was thinking, since its a file we generate and it needs to be compiled to be usable component

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, added to the csproj.

@jsturtevant
Copy link
Collaborator

do we need bump the dotnet AOT compiler version anywhere?

@yowl
Copy link
Collaborator Author

yowl commented Dec 15, 2023

do we need bump the dotnet AOT compiler version anywhere?

Yes we do. Incoming, when the packages publish.

@yowl yowl closed this Dec 18, 2023
@yowl yowl reopened this Dec 18, 2023
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