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

NativeAOT-LLVM: Change name of Wasm Import function name to include the module prefix #2410

Closed

Conversation

yowl
Copy link
Contributor

@yowl yowl commented Oct 4, 2023

This PR solves a problem creating a Wasm Component that defines a WIT world where the same name function is both imported and exported, directly or via an interface, e.g.

world many-arguments {
  import imports: interface {
    many-arguments: func(
      a1: u64,
      a2: u64,
      a3: u64,
      a4: u64,
      a5: u64,
      a6: u64,
      a7: u64,
      a8: u64,
      a9: u64,
      a10: u64,
      a11: u64,
      a12: u64,
      a13: u64,
      a14: u64,
      a15: u64,
      a16: u64,
    )
  }
  export many-arguments: func(
    a1: u64,
    a2: u64,
    a3: u64,
    a4: u64,
    a5: u64,
    a6: u64,
    a7: u64,
    a8: u64,
    a9: u64,
    a10: u64,
    a11: u64,
    a12: u64,
    a13: u64,
    a14: u64,
    a15: u64,
    a16: u64,
  )
}

Previously this would have created and extern with the name many-arguments and an alias for the export with the same name. This resulted in the linker making calls to the extern call back into the export and not the actual imported function.

I have added the module name to the extern (the Wasm import) where a WasmImport is defined. Maybe not the best strategy?

cc @dotnet/nativeaot-llvm

@yowl yowl marked this pull request as ready for review October 4, 2023 02:43
@SingleAccretion
Copy link

How would you define such a component in C/C++ (or Rust I guess)?

So far the model has been that all statically resolvable symbols are statically linked. So its expected that code below will always do PI -> RPI_Impl:

void PI()
{
    [DllImport("*")]
    [WasmImport] // Stand-in syntax for specifying the method in the imports list
    static void RPI();
    RPI();
}

[UnamangedCallersOnly(EntryPoint = "RPI")]
static void RPI_Impl() { }

What this change effectively does is that if RPI is specified in the Wasm imports list, it becomes statically unresolvable, mandating that it should be resolved "dynamically" (made into a import).

Do we want that behavior (I am not sure myself)? How does this interact with #2383?

@yowl
Copy link
Contributor Author

yowl commented Oct 4, 2023

How would you define such a component in C/C++ (or Rust I guess)?

// import
__attribute__((__import_module__("imports"), __import_name__("many-arguments")))
void __wasm_import_imports_many_arguments(int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t, int64_t);

// export
__attribute__((__export_name__("many-arguments")))
void __wasm_export_many_arguments_many_arguments(int64_t arg, int64_t arg0, int64_t arg1, int64_t arg2, int64_t arg3, int64_t arg4, int64_t arg5, int64_t arg6, int64_t arg7, int64_t arg8, int64_t arg9, int64_t arg10, int64_t arg11, int64_t arg12, int64_t arg13, int64_t arg14) {
  many_arguments_many_arguments((uint64_t) (arg), (uint64_t) (arg0), (uint64_t) (arg1), (uint64_t) (arg2), (uint64_t) (arg3), (uint64_t) (arg4), (uint64_t) (arg5), (uint64_t) (arg6), (uint64_t) (arg7), (uint64_t) (arg8), (uint64_t) (arg9), (uint64_t) (arg10), (uint64_t) (arg11), (uint64_t) (arg12), (uint64_t) (arg13), (uint64_t) (arg14));
}

@yowl
Copy link
Contributor Author

yowl commented Oct 4, 2023

Do we want that behavior (I am not sure myself)? How does this interact with #2383?

Yes, I think we do. WasmImport exists to allow the import to be satisfied by a different module which we only have use for from components and that happens non-statically. Regards #2383 this PR doesn't progress that subject much except to reinforce that we would benefit from a way to define the module name in the source code, via an attribute probably..

@SingleAccretion
Copy link

Are there scenarios where the current behavior of "try static linking, fall back to Wasm imports" valuable / desirable?

Say, someone shipping a managed library that depends on some native library, and that native library is available as a component in some environments but not others (where it'd be linked in statically).

@yowl
Copy link
Contributor Author

yowl commented Oct 4, 2023

If I understand correctly, you are suggesting a scenario where module A imports from module B function F, but if we can find F statically from anywhere, i.e. any module, then link it regardless of the mismatch in module name?

@yowl
Copy link
Contributor Author

yowl commented Nov 29, 2023

Marking as draft while I check if #2444 means we can solve the same export/import name in wit codegen.

@yowl
Copy link
Contributor Author

yowl commented Dec 3, 2023

superceded by #2444

@yowl yowl closed this Dec 3, 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.

2 participants