-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/compile: relax wasm/wasm32 function import signature type constraints #66984
Comments
"This follows the C struct value semantics" is just a hair vague; are 8-byte quantities (float64, int64, uint64) stored at a 4-byte or 8-byte alignment? It was my understanding (and the purpose of #66408) to specify a (edited to note error, the host alignment for 8-byte integers and floats is 8 bytes). |
Ideally 8-byte values would always be 8-byte aligned in the |
@dr2chase Looking at what clang does, it uses 8-byte alignment on 64bit quantities so we'd match that. |
You are right, I got it backwards. But that is what you are expecting for anything that has pointers-to-it passed to the wasm host platform, yes? |
Thanks for the proposal! A few questions:
Besides, for structs, arrays of structs, and pointer to structs, I would suggest we allow only structs with Thanks. |
Two other questions, first:
Is this laid out Second, passing pointers to 8-byte primitive types to the host will be tricky unless those references come from fields in structures tagged with HostLayout -- otherwise, they may not be aligned. So
|
Thanks for the quick feedback! I've tried to answer each question:
The specification falls out of the table of transformations (I think?). There current plan isn't to introduce any sort of magic around large structs or field packing. Structs fields are added as call parameters, from the first field to the last, according to the conversion rules for the type of the field. Examples: type foo struct {
a int
b string
c [2]float32
} With a function signature of //go:wasmimport some_module some_function
func wasmFunc(in foo) int Would roughly translate to (in WAT format) // $a is of type `i32` holding the value of `a`
// $b_addr is of type `i32` and is a pointer to the start of the bytes for the Go string `b`
// $b_len is of type `i32` and is the length in bytes to read from `$b_addr` to get the whole string
// $c_0 is of type `f32` and is the value of `c[0]`
// $c_1 is of type `f32` and is the value of `c[1]`
call $some_function (local.get $a) (local.get $b_addr) (local.get $b_len) (local.get $c_0) (local.get $c_1) Struct fields would be expanded into call parameters before subsequent fields at the same level.
For
This sounds like a great idea, and we should also extend it to pointers to 8 byte sized primitive types to guarantee alignment, as suggested by @dr2chase's last question. This would avoid any question around alignment issues for pointers. It hurts the ergonomics a little bit but that's a price worth paying, I think.
I'm a little confused by the question to be honest. If this type was used as an input to a Wasm call, it would look like this: // $a is of type `i32`
// $b is of type `i32`
call $some_function (local.get $a) (local.get $b) I suppose that might mean the memory looks like this: |
Thanks for the response!
This sounds like a reasonable choice. Is this ABI specified anywhere in Wasm/WASI docs? Or the Wasm side has to define the function taking parameters element-wise?
This sounds reasonable as well. Is it specified anywhere in Wasm/WASI docs? Thanks. |
I don't know about this being an official ABI so much as just a consequence of the Wasm spec around function calls and how we can apply Go semantics to it. We're limited to the
Not sure there's a doc anywhere, but practically, definitions like |
Addresses PR feedback in tinygo-org#4027.
I guess my question is whether a pointer-to-struct is ever passed from Go to the WASM platform, and therefore, what expectations the WASM side has about the layout of the fields of that structure. I don't think this is for specifying the layout that gets passed to a WASM call if the struct is passed by value. |
As the ABI doesn't have a way to pass struct by value, do we need to support it? If users on the other (non-Go) side have to define the function as taking arguments element-wise with primitive types and pointers, it would probably be better to define the same way on the Go side. Does any other language have a Wasm/WASI interface that allows passing struct by value? (Same applies for arrays. Pointer to struct/array is fine.) |
I think the biggest concern around this is that all 64 bit values use 8 byte alignment, as you say. We definitely want this, so I think that on its own makes the case for I also don't know that it's an important question for this proposal in particular, since the answer is pretty clear regarding what we should be passing in the
It's fair to say that we can just not support structs and arrays by value, their use are likely to be limited (why not use a pointer?), and it would significantly simplify the implementation. We can come back to it if we need to later. I'll update the proposal. |
On the TinyGo side we're working on an implementation of this proposal, so here's my perspective on it from TinyGo:
Question: what fields would be allowed in these structs? I would assume a struct with a
|
I think this is fine. And we should allow them in Go gc toolchain for the "wasm32" port.
Yeah, something along this line makes sense. And also for arrays. I'd say a struct field or a struct pointed by a field should also have the |
Thanks for your thoughts Ayke, it's always appreciated.
As Cherry says, these values will be allowed since this proposal is restricted to the wasm32 architecture. The wasm architecture will not have these new relaxations applied. I'm not sure I understand the incompatibility?
I've added some clarifying words to the proposal, please take a look! |
The updated proposal looks good to me! However, I have to say that @ydnar has pointed out that the Canonical ABI also allows structs, and it would be nice to have them supported in
Nevermind, TinyGo doesn't even support
Seems like a good idea. It's easier to remove such a restriction in the future (if it turns out to be unnecessary) than it is to introduce it later. But I don't know Go internals well enough to say it is needed. |
To comment quickly on the Canonical ABI: it doesn't relate to this proposal directly as this proposal only targets the |
I'm okay with supprting passing structs by value if there is a widely used ABI that is not too complex (if it is as complex as the ELF C ABI on amd64, I'm not sure). If currently there is no widely agreed ABI for structs, we can wait. We can always add things later. |
Hi, original author of the relaxed type constraints proposed here. The TinyGo PR where this originated depends on LLVM to flatten structs and arrays. This works in practice most of the time, except when it doesn't: namely the Component Model and WASI 0.2 extensively uses tagged unions ( The code generator ( The CABI flattening rules are per-field, so if a Given that the compiler is ignorant of the CABI layout, this strategy cannot correctly represent these @cherrymui: LLVM does correctly flatten structs and arrays consistent with the CABI spec (my sense is the former informed the latter). If we want to start with a more constrained set of types now and relax later, we can make that work. |
Not exactly. If you pass a LLVM struct like I believe this is why the Component Model lowers everything to bare i32/i64/f32/f64/pointer values in function signatures, which have no ambiguity in what ABI they should have on the WebAssembly level. |
Addresses PR feedback in tinygo-org#4027.
This reflects the relaxed type restrictions proposed here (except for structs.HostLayout): golang/go#66984
golang/go#66984 - Remove int and uint as allowed types in params, results, pointers, or struct fields - Only allow small integers in pointers, arrays, or struct fields
golang/go#66984 TODO: regenerate WASI syscall packages for GC shape types including structs.HostLayout compiler: require go1.23 for structs.HostLayout compiler: use an interface to check if GoVersion() exists This permits TinyGo to compile with Go 1.21. compiler: use goenv.Compare instead of WantGoVersion
compiler: align with current wasm types proposal golang/go#66984 - Remove int and uint as allowed types in params, results, pointers, or struct fields - Only allow small integers in pointers, arrays, or struct fields - enforce structs.HostLayout usage per wasm types proposal golang/go#66984 - require go1.23 for structs.HostLayout - use an interface to check if GoVersion() exists This permits TinyGo to compile with Go 1.21. - use goenv.Compare instead of WantGoVersion - testdata/wasmexport: use int32 instead of int - compiler/testdata: add structs.HostLayout - compiler/testdata: improve tests for structs.HostLayout
No change in consensus, so accepted. 🎉 The proposal is #66984 (comment) |
Change https://go.dev/cl/626615 mentions this issue: |
The proposal reads
This sounds like pointer to empty struct is allowed. I'm not sure this is useful, though, as I don't think the host can do anything with a pointer to an empty struct. Should we disallow it (by just removing the "If the struct has any fields" clause)? If one really wants a pointer to a struct with no (real) field, they can still do |
We actually use this quite a bit in the TinyGo implementation of Here’s the implementation in TinyGo, for what it’s worth: https://github.com/tinygo-org/tinygo/blob/2a76ceb7dd5ea5a834ec470b724882564d9681b3/compiler/symbol.go#L466-L491 Edit: where this is used in practice is not typically a pointer to an empty struct, but a struct that contains a field that is an empty struct, e.g. for interface conformance: type S struct {
_ structs.HostLayout
embedded
Name string
}
type embedded struct{}
func (embedded) isEmbedded() {} Edit 2: I suppose we could replace all instances of |
Fair point. Thanks for the example. |
Change https://go.dev/cl/627059 mentions this issue: |
…d results As proposed on #66984, this CL allows more types to be used as wasmimport/wasmexport function parameters and results. Specifically, bool, string, and uintptr are now allowed, and also pointer types that point to allowed element types. Allowed element types includes sized integer and floating point types (including small integer types like uint8 which are not directly allowed as a parameter type), bool, array whose element type is allowed, and struct whose fields are allowed element type and also include a struct.HostLayout field. For #66984. Change-Id: Ie5452a1eda21c089780dfb4d4246de6008655c84 Reviewed-on: https://go-review.googlesource.com/c/go/+/626615 Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
CL 626615 implemented this for wasm. This will be included in Go 1.24. The wasm32 port will adopt this when the port lands. (Move the backlog milestone for wasm32.) |
Now that we support pointer types on wasmimport functions, use them, instead of unsafe.Pointer. This removes unsafe conversions. There is still one unsafe.Pointer argument left. It is actually a *Stat_t, which is an exported type with an int field, which is not allowed as a wasmimport field type. We probably cannot change it at this point. Updates #66984. Change-Id: I445c70b356c3877a5604bee67d19d99a538c682e Reviewed-on: https://go-review.googlesource.com/c/go/+/627059 Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
Background
#59149 removed the package restrictions on the use of
go:wasmimport
, but established strict constraints on the types that can be used as input and result parameters. The motivation for this was that supporting rich types between the host and the client would require sophisticated and expensive runtime type conversions because of the mismatch between the 64 bit architecture of the client and the 32 bit architecture of the host.With the upcoming 32 bit wasm port, this problem goes away, as both client and host will use 32 bit pointers. However, we can also support a limited set of types on 64 bit platforms, where no runtime conversion is necessary.
Proposal
Relax the constraints on types that can be used as input and result parameters with the
go:wasmimport
compiler directive. The exact allowed types would depend on whetherwasm
orwasm32
is used.We define the "small integer types" group as the set of types described by
[u]int[8|16]
. The following types would be allowed as input parameters to anywasmimport
/wasmexport
function:bool
int32
,uint32
,int64
,uint64
float32
,float64
string
uintptr
,unsafe.Pointer
,*T
whereT
is an allowed parameter type or one of the small integer types.*struct
. All fields of the*struct
must be allowed parameter types,struct
,[...]T
, or one of the small integer types.structs.HostLayout
(see structs: add HostLayout "directive" type #66408).struct
fields must also embedstructs.HostLayout
(recursively).*T
andstring
types are only allowed in*struct
onwasm32
.*[...]T
whereT
is an allowed type or one of the small integer types.All input parameter types except
string
are also allowed as result parameter types.The following types would remain disallowed as input and output parameter types:
chan T
complex64
,complex128
func
interface
map[T]U
[]T
struct
[...]T
The conventions established for use of pointers in CGO will be required when using pointers with
wasmimport
/wasmexport
, e.g. the host can read Go memory, can write pointerless data (like the content of a byte buffer) but cannot write Go pointers to Go memory, and cannot hold on to Go pointers unless they are pinned.Discussion
Compatibility guarantees
The Go spec does not specify the struct layout and leaves it up to implementations to decide. As such, we cannot provide a guaranteed ABI without having to change the spec or force future layout changes to provide runtime conversion of data. This proposal suggests making it clear to users through documentation that there are no guarantees of compatibility across versions of the Go compiler.
Type conversion rules
The following conversion rules would be automatically applied by the compiler for the respective parameter type:
bool
i32
int32, uint32
int64, uint64
i32, i32
i64, i64
float32, float64
f32, f64
string
(i32, i32)
tuple of (pointer, len). Only allowed for input parameters.uintptr
,unsafe.Pointer
,*T
,*struct
,*[...]T
i32, i32, i32, i32, i32
Strings
Strings are not allowed as result parameters as Wasm practically does not allow more than 1 result parameter.
Supporting
GOARCH=wasm
The
wasm
architecture uses 64 bit pointers and integer sizes. As the host uses 32 bit pointers, this makes it impossible to allow certain types without costly runtime conversions, such as*struct
types containing pointer fields. Sincestring
types are also pointer types,*struct
types containingstring
fields are also disallowed.Supporting
[u]int
,[u]int8
,[u]int16
as concrete parametersThe
[u]int
types are problematic as the size of them are not precisely defined, and may cause confusion when used with strictly 32 bit or 64 bit integers. The[u]int8
and[u]int16
types are problematic because we would be forced to automatically convert them to/from thei32
wasm representation, with potential loss of precision or overflow. They are still allowed as pointer type, array elements and struct fields.Supporting slices, maps
Both slices and maps are disallowed because of the uncertainty around the memory underlying these types and interactions with struct and array rules. Users who wish to use slices can manually use
(&slice, len(slice))
orunsafe.Pointer
. There is no clear way to support passing or returning map data from the host other than by usingunsafe.Pointer
and making assumptions about the underlying data.Related proposals
struct.Hostlayout
#66408 proposes a way for users to request that struct layout is host compatible. Our proposal depends on the definitions put forward in this proposal for
struct
parameters.Future work
WASI Preview 2 (AKA WASI 0.2)
WASI Preview 2 defines its API in terms of the Component Model, with a rich type system and an IDL language, WIT. The Component Model also defines a Canonical ABI with a specification for lifting and lowering Component Model types into and out of linear memory. This proposal does not attempt to define the ABI for any hypothetical wasip2 target, and would leave such decisions for any future wasip2 proposal.
Supporting
struct
and[...]T
by valueA previous version of this proposal included support for passing
struct
and[...]T
types by value by expanding each field recursively into call parameters. This was removed in favor of a simpler initial implementation but could be re-added if users require it.Contributors
@johanbrandhorst, @evanphx, @achille-roussel, @dgryski, @ydnar
CC @cherrymui @golang/wasm
The text was updated successfully, but these errors were encountered: