cmd/compile: replace CallImport with go:wasmimport directive #38248
Comments
I'm confused about there being two different ABIs and also about what is optional and not. It would be nice if the optional fields were last, not in the middle of the list. Then you can tell what things mean by looking at just the argument count - you don't have to know that "go" is apparently a magic word and that when it is arg[2] it is not the real arg[2]. Could you please write the suggested docs describing the syntax in full? And maybe rotate the optional words to the end? It would be even better if there were not two different optional ABI words. Can it be reduced to one? |
Right, I forgot to include a clear explanation of the arguments of the directive. The usage is like this: //go:wasmimport localname importmodule importname [abi0]
Lines 261 to 262 in aa3413c
Hope this helps. |
//go:linkname is kind of the odd one out for compiler directives (see go doc compile) in that it doesn't just implicitly apply to the next function declaration. Instead of adding another like that, it would probably be better to attach it to a function implicitly, like the others (example: //go:noinline). Then instead of:
you'd have
Is there always a function declaration to attach to? The syntax would be down to:
I'm skeptical about exposing the name abi0. If the default is likely to be Go code, maybe use [wasi] as the optional mode? Or maybe if the importmodule is go, assume the Go ABI? |
I fully agree that a version without However, all the current directives that apply to the next function are just flags without any parameters. They get processed by the This is why I opted for not doing a large change to Go's lexer/parser and instead copied a solution that already exists: About the abi flag: The default should be the abi that wasi uses, since it is a more native way to pass arguments to a WebAssembly function. The |
That was true a couple weeks ago but is no longer true (I just changed that, in preparation for some other possible So now we're just down to the ABI flag. The problem with I'm also a little confused about "The abi0 flag only needs to exist so I don't have to change everything in a massive CL." since the @neelance is it the case that long term nothing will say /cc @aclements how do you feel about exposing the name |
Great!
They should not.
Yes, that's the long term plan, but I can't say when this will be the case. It depends a lot on how fast the development of WASI goes until it has enough features to completely replace the custom interface that wasm_exec.js currently uses. I don't mind picking a different flag name than |
I'm actually fine with exposing the name abi0. The intent of calling them ABI0 and ABIInternal was that we could expose the name "ABI0" if we ever froze another, faster ABI as ABI1. I'm not sure if we'd want to spell it "abi0" or "ABI0" given that it is an initialism. However, it actually seems more reasonable to me to assume everything in the "go" module follows the "Go ABI" and omit the ABI from the directive. If I understand correctly, this is effectively a "closed" module for wasm_exec.js's exports and users can't define new abi0 interfaces in either this module or any other module, so the set of things that use abi0 is fixed and bounded and exactly those in the "go" module. |
All right, I agree that it is good to discourage anyone else from using abi0 by exposing it less. Shall I edit the proposal in the first post or add a new version below? |
@neelance, please add a new comment below, so it's easier to see the history. Thanks. |
Here is the new full proposal text: The wasm architecture currently uses a special The WebAssembly ecosystem is converging on using WASI (WebAssembly System Interface) as a standardized interface between the WebAssembly binary and its host (similar to system calls). I am working on adding support for WASI to Go. As a first step, Go needs to be able to use function imports with the ABI used by WASI. For this reason I propose to replace //go:wasmimport importmodule importname Concrete example: //go:wasmimport wasi_unstable proc_exit
func __wasi_proc_exit(code int32)
Lines 261 to 262 in aa3413c By default The |
From the TinyGo side, I fully support this proposal. Right now we use something like this (following your example): //go:wasm-module wasi_unstable
//export proc_exit
func __wasi_proc_exit(code int32) It works as a stopgap, but if this proposal is accepted I'll make sure TinyGo will switch over to the new format. I have one comment:
What is this ABI? Is it specified somewhere? It seems to me that it's really just the C ABI and if so, I think it should be specified as such. Some related discussions: |
On the lowest level the ABI consists of passing data via arguments (on the WebAssembly stack) to WebAssembly's |
Nitpick: ABI is much more than a calling convention! It also involves struct layout, alignment, etc. (relevant when passing pointers around). But in general, because you can use this facility for more than just WASI (e.g. |
My goal with I do not want to define some "WebAssembly interface for Go" here, since I believe this is too early and Go should rather follow the WebAssembly ecosystem (namely WASI) than to diverge with its own interface. So if there is some concern about other people using this interface, then I would be okay with restricting |
Everyone seems in favor of what we've converged on. Thanks for the good discussion. Based on the discussion, this seems like a likely accept. |
Sorry about being late. Does A bit bikeshedding: is "go" the best name for wasm_exec.js? From the Go code side, everything is go except the imported functions. It is true that the module is currently named "go", but we can change that. Maybe "jsgo" or "gojs"? |
There is no list. The parameter types need to be supported. Like I said above, I would be okay with restricting it to
I am fine with changing this name. No preference on my end. |
Replying to the previous two comments:
Leaving as likely accept for another week but we seem to have converged. |
Thanks. I also think it is reasonable to start with restricting it to runtime and syscall/js. We can make it more general later if necessary. |
No change in consensus, so accepted. |
Change https://golang.org/cl/243677 mentions this issue: |
Change https://golang.org/cl/248739 mentions this issue: |
Change https://golang.org/cl/252828 mentions this issue: |
I wonder, can we do the same for exports? Starting from the patch https://golang.org/cl/252828, it should be quite trivial to implement a
This might be a first step towards #25612 and #41715. I can take care of filing a PR for it, I already have a prototype on my machine |
@slinkydeveloper As you can see in this issue, the addition of |
#25612 is not a proper proposal, since there are a lot of aspects of the intended behavior that are not clear. I see some difficulties around |
The wasm architecture currently uses a special
CallImport
assembler instruction to call function imports. This approach is quite inflexible and only compatible with the ABI used by wasm_exec.js.The WebAssembly ecosystem is converging on using WASI (WebAssembly System Interface) as a standardized interface between the WebAssembly binary and its host (similar to system calls). I am working on adding support for WASI to Go. As a first step, Go needs to be able to use function imports with the ABI used by WASI.
For this reason I propose to replace
CallImport
with a new compiler directivego:wasmimport
. It can be used like this:By default
go:wasmimport
will use the ABI used by WASI. An optional parameter can be used for backwards compatibility withwasm_exec.js
. This parameter is calledabi0
becausewasm_exec.js
is reading the parameters form memory with Go's normal ABI0 layout:I do not plan to add other ABIs. On the contrary, the
abi0
mode is supposed to go away once WASI can be used as the primary interface (it is still missing certain features).The
go:wasmimport
directive will not be covered by Go's compatibility promise as long as the wasm architecture itself is not considered stable.The text was updated successfully, but these errors were encountered: