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

[wasm] Add @_expose(wasm) attribute for top-level functions #68524

Merged
merged 2 commits into from
Sep 28, 2023

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Sep 14, 2023

Today, there is no way to create an "export" from Swift. (The current workaround is using @_cdecl and linker option, or linking C source1).

This PR adds a new variant of @_expose attribute with wasm parameter (and optionally with custom name).
This attribute instructs the compiler that this function declaration should be "export"ed from this .wasm module. It's equivalent of Clang's __attribute__((export_name("name")))

The attribute doesn't have any effect on to function signature and calling convention but only adds LLVM meta attribute to the function. If it's applied with @_cdecl at the same time, only C thunk is exported.

Radar: rdar://115502069

Footnotes

  1. https://book.swiftwasm.org/examples/exporting-function.html

@kateinoigakukun
Copy link
Member Author

@swift-ci Please smoke test

@MaxDesiatov MaxDesiatov added the attributes Feature: Declaration and type attributes label Sep 14, 2023
@MaxDesiatov
Copy link
Member

@swift-ci build toolchain

@ahoppen ahoppen removed their request for review September 14, 2023 21:26
@xedin xedin removed their request for review September 15, 2023 14:45
@rintaro rintaro removed their request for review September 15, 2023 23:46
Copy link
Collaborator

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

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

I'm not familiar with Wasm but this looks like a good feature to me!
It's probably worth for @hyp to take a look as well, since he's the author of @_expose(Cxx) attribute.

One comment I have is that we should probably support multiple expose attributes on a single decl, in case someone would like to e.g. expose a Swift function to C++ and Wasm.

lib/AST/SwiftNameTranslation.cpp Outdated Show resolved Hide resolved
lib/SIL/IR/SILFunctionBuilder.cpp Outdated Show resolved Hide resolved
test/attr/attr_expose.swift Show resolved Hide resolved
Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

This overall is looking really solid. I agree with Egor. I would like to see the behavior for a function that is exported both for WASM and for C.

Another question, is the export of the function based on target triple? Like does it make sense for a C static archive to have WASM entries if we're not compiling for WASM? Or in the other direction, does it make sense to generate C entrypoints when we are compiling for WASM?

@MaxDesiatov
Copy link
Member

MaxDesiatov commented Sep 19, 2023

Another question, is the export of the function based on target triple?

I'd assume in LLVM this is a no-op for non-Wasm triples.

Like does it make sense for a C static archive to have WASM entries if we're not compiling for WASM?

Not in my understanding, Wasm exports are quite specific to Wasm object format, I don't think they'd be valid in any way in other object formats.

Or in the other direction, does it make sense to generate C entrypoints when we are compiling for WASM?

@_cdecl does make sense on Wasm, as its calling convention is quite trivial for C (at least with Clang), it doesn't have registers.

@kateinoigakukun
Copy link
Member Author

@egorzhdan Thank you for reviewing! It makes sense to me to unlock the combination of Cxx and Wasm exposure on a single decl (while we cannot accept multiple @_expose with the same exposure kind).

@etcwilde Thanks too :) The exposed entrypoint is generated only when lowering LLVM IR to Wasm object file, so the metadata attribute added in this PR is no-op on other targets. I'll leave a comment on the place where the metadata attribute is added regardless of the target triple.

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Sep 19, 2023

@egorzhdan @etcwilde
In the case of C++ and Wasm exposures, I thought it makes sense to expose a Wasm entrypoint for a function with C++ signature, but it looks like the C++ interface itself is defined in a header, so there is no definition in the compilation file of swiftc. So the exported wasm entrypoint in this way is only available when the C++ interface header is included by someone.

It's quite different from the combination of cdecl and Wasm exposure because the C interface function is defined within the compilation unit. I'm not sure whether we should expose C++ interface function as Wasm entrypoint as we do in cdecl and Wasm combination, or we should expose Swift interface as Wasm entrypoint in C++ and Wasm case? (Or just prohibit the case for now?) WDYT?

@egorzhdan
Copy link
Collaborator

What I meant by exposing a function for both C++ and Wasm is: imagine you have a multiplatform Swift module that can be compiled for both Wasm (so the function is annotated with @_expose(wasm)) and macOS, and you'd like to make a function from this module available to C++ projects on macOS (so you'd like to add @_expose(Cxx) as well).

I don't know a use case for having multiple @_expose attr on a single decl that take effect for the same target architecture, but I think we should support multiple @_expose attrs for distinct architectures.

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Sep 25, 2023

@egorzhdan Thank you for your elaboration. So if we have the following definition,

@_expose(Cxx)
func exposeToCxxWasm(input: String) {}

We have the following interop header:

namespace foo SWIFT_PRIVATE_ATTR SWIFT_SYMBOL_MODULE("foo") {

SWIFT_INLINE_THUNK void exposeToCxxWasm(const swift::String& input) noexcept SWIFT_SYMBOL("s:3foo15exposeToCxxWasm5inputySS_tF") {
  return _impl::$s3foo15exposeToCxxWasm5inputySS_tF(_impl::swift_interop_passDirect_foo_uint64_t_0_8_void_ptr_8_16(swift::_impl::_impl_String::getOpaquePointer(input)));
}

} // namespace foo

If we simply add @_expose(wasm) on the exposeToCxxWasm decl, it adds wasm-export-name metadata to the LLVM IR emitted by this compilation unit, but it exposes a Wasm entrypoint function with Swift calling convention because C++ thunk is generated in a header. This behavior is inconsistent with @_cdecl and @_expose(wasm), which exposes a Wasm entrypoint with C calling convention.

So I tried exposing a Wasm entrypoint with C++ calling convention, but I found adding __attribute__ on the generated thunk function does not solve the issue and Wasm export entry won't be included in the generated object file because the thunk in the header is not included within compilation unit unless it will be included by someone.

@hyp
Copy link
Contributor

hyp commented Sep 25, 2023

@_expose(Cxx) is mostly irrelevant - C++ interop no longer keys off it by default, so any supported public function can be exposed to C++.
However, it would still be good to support C++ interop and WASM correctly. For that you need to make sure that __attribute__((export_name(""))) is applied to every Swift function prototype declaration that has _expose(wasm) attribute in the generated header, like in your example that would be the function declaration $s3foo15exposeToCxxWasm5inputySS_tF in the generated header (Although I'm not sure if that's required by WASM, it sounds like it would be best to do it to make sure the Swift function definition and its declaration used in C++ match at the LLVM IR stage). This attribute should only be added when generating the header the WASM target. You don't need to add anything to C++ inline thunks like exposeToCxxWasm, they're not meant to be exported.

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Sep 25, 2023

@hyp Thank you Alex!

For that you need to make sure that attribute((export_name(""))) is applied to every Swift function prototype declaration that has _expose(wasm) attribute in the generated header

Actually, Wasm has two-level namespace to reference a function in an external image. In Clang, it's expressed by __attribute__ ((import_module)) and __attribute__((import_name)).
So import-side has to have different attributes than export-side.
The name of import_name can be determined at compiling exposeToCxxWasm because it should have the same name as export_name, but the Wasm module name (note that the module name is irrelevant from Swift's module name) cannot because it can be changed at instantiating modules just before execution.

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Sep 25, 2023

Can we leave C++ and Wasm combination as TODO until we come up with better solutions?

@hyp
Copy link
Contributor

hyp commented Sep 25, 2023

Can we leave C++ and Wasm combination as TODO until we come up with better solutions?

That's fine with me! It sounds like this combination should just work without extra annotations, unless C++ and Swift pieces live in different WASM modules.

The name of import_name can be determined at compiling exposeToCxxWasm because it should have the same name as export_name, but the Wasm module name (note that the module name is irrelevant from Swift's module name) cannot because it can be changed at instantiating modules just before execution.

Is there a potential solution there? How does another Swift module know how to import another Wasm module if it can be changed dynamically?

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Sep 25, 2023

That's fine with me! It sounds like this combination should just work without extra annotations, unless C++ and Swift pieces live in different WASM modules.

Yes! Linking within a single image works well as it is now!

Is there a potential solution there? How does another Swift module know how to import another Wasm module if it can be changed dynamically?

Currently, we don't support Swift's import across different images. All Swift modules should be linked at static-link time for now. One of the potential solutions here is to use "env" as module name, which is conventionally used for dynamic linking in C+Wasm environments: https://github.com/WebAssembly/tool-conventions/blob/main/DynamicLinking.md
Also, another solution would be embedding some linkage info in swiftmodule like -module-link-name especially for wasm import, just as Swift-specific convention.

However, both of them can limit future evolutions around dynamic linking, unfortunately. So I think it would be great to support it after we support dynamic linking in wasm.

@kateinoigakukun
Copy link
Member Author

@swift-ci Please smoke test

This attribute instructs the compiler that this function declaration
should be "export"ed from this .wasm module. It's equivalent of Clang's
`__attribute__((export_name("name")))`
But multiple @_expose with the same exposure kind are still invalid.
@kateinoigakukun
Copy link
Member Author

@swift-ci Please test

@kateinoigakukun kateinoigakukun merged commit 8cfcc24 into apple:main Sep 28, 2023
5 checks passed
@kateinoigakukun kateinoigakukun deleted the katei/expose-wasm-sym branch September 28, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attributes Feature: Declaration and type attributes WebAssembly Platform: WebAssembly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants