Support for `multivalue`

[CryZe]

Motivation

Multivalue is now finally properly supported by LLVM 17, which made its way into recent Rust nightlies.

Proposed Solution

It seems like a bunch of bindings that wasm-bindgen generates are using multivalue, but wasm-bindgen doesn't generate the JS side correctly to return multiple values.

Alternatives

The bindings could be adjusted to just not use multivalue at all, then the JS side doesn't need to be changed.

Additional Context

None so far

[Liamolucko]

Huh, I didn't realise that enabling multivalue affected the C ABI. That... kind of seems like a bug to me? Even though it's currently wrong, the C ABI is still supposed to be a fixed ABI that matches C, not change depending on what target features are enabled.

The bindings could be adjusted to just not use multivalue at all, then the JS side doesn't need to be changed.

That was already pretty much the plan anyway, we're trying to make it so that wasm-bindgen no longer relies on Rust's incorrect C ABI (#3454).

[fornwall]

When it comes to potentially enabling multivalue in LLVM by default (trying to get it into LLVM 18 or 19 perhaps? see WebAssembly/tool-conventions#158), is there any ballpark timeline estimate for multivalue support in wasm-bindgen (not something to be rushed and certainly not demanding anything, just trying to get a sense of what is realistic and desirable for LLVM:s defaults next year)?

[Liamolucko]

The ideal solution would be to wait for rustc to switch to the standard C ABI (rust-lang/rust#71871), since that ABI never uses multivalue anyway and so wasm-bindgen doesn't have to worry about it. But I don't know how long that will take, since it needs a future-incompatibility lint to tell people to upgrade from old versions of wasm-bindgen that will break with the standard ABI, and then we'll need to wait for everyone to upgrade before we can actually make the change.

Now that I think about it though, it should actually be fine to switch only return types to match the standard C ABI right away. The two ABIs are identical in how they handle return types, as long as multivalue is disabled, and wasm-bindgen is already broken when multivalue is enabled anyway. That would be enough to fix this.

As for trying to fix it on wasm-bindgen's end: it's possible, but annoying. The obvious solution is to test for cfg!(target_feature = "multivalue") and generate different JS when it's enabled, except that that would break when rustc switches to the standard C ABI, since that'll go back to not using multivalue even when the target feature is enabled. Any solution would need to be forwards-compatible with the standard C ABI as well as the current one so that rustc is able to switch without breaking wasm-bindgen.

The only solution like that that I can think of is to forcibly use the standard C ABI's handling of return values no matter what (using retptrs whenever the return type takes up more than one value). I'm not sure if that's possible though; see the second half of #3595's description for why. So if that's impossible, we'd have to always use retptrs no matter what, which isn't ideal.

[daxpedda]

The ideal solution would be to wait for rustc to switch to the standard C ABI (rust-lang/rust#71871), since that ABI never uses multivalue anyway and so wasm-bindgen doesn't have to worry about it. But I don't know how long that will take, since it needs a future-incompatibility lint to tell people to upgrade from old versions of wasm-bindgen that will break with the standard ABI, and then we'll need to wait for everyone to upgrade before we can actually make the change.

For reference: rust-lang/rust#117918.

[RReverser]

the C ABI is still supposed to be a fixed ABI that matches C, not change depending on what target features are enabled

Right, but C ABI that Rust is supposed to match also changed depending on multivalue. It's intentional, because the whole point of multivalue as a feature is to change the ABI of passing/returning complex types like structs and fixed-size arrays by value from "a pointer allocated on the shadow stack" to "an actual tuple of values that is up to the engine to pass around efficiently".

[Liamolucko]

Right, but C ABI that Rust is supposed to match also changed depending on multivalue.

Oh, this is the first I've heard of that - it isn't mentioned in tool-conventions. I assume you mean the one clang uses with -target-abi experimental-mv?

It's intentional, because the whole point of multivalue as a feature is to change the ABI of passing/returning complex types like structs and fixed-size arrays by value from "a pointer allocated on the shadow stack" to "an actual tuple of values that is up to the engine to pass around efficiently".

I'm not so sure about that - it only happens on wasm32-unknown-unknown, not other targets like wasm32-wasi. I suspect it might just be another accidental result of Rust using PassMode::Direct where it shouldn't in wasm32-unknown-unknown's C ABI.

If it is intentional though, the rest of Rust's Wasm targets need to be updated to do the same thing. Supporting it in wasm-bindgen wouldn't be too hard, that just means we can use cfg!(target_feature = "multivalue") after all.

Even then, it seems a bit questionable to me to do it automatically when -Ctarget-feature=+multivalue is enabled. clang doesn't do it automatically with -mmultivalue, it requires an extra -target-abi experimental-mv as well, and I don't think enabling multivalue on both compilers should result in their outputs becoming incompatible.

[RReverser]

it only happens on wasm32-unknown-unknown, not other targets like wasm32-wasi. I suspect it might just be another accidental result of Rust using PassMode::Direct where it shouldn't in wasm32-unknown-unknown's C ABI.

Ah yeah Rust-specific incompatibility of wasm32-unknown-unknown still needs to be fixed separately. (long overdue, really)

Interesting that in Clang ABI change still requires experimental opt-in... For some reason I thought it's already enabled automatically when this feature is used. I must have misremembered some previous discussions on changing external C ABI for multivalue.

[CryZe]

It seems like build-std and -C target-feature=+multivalue -Z wasm-c-abi=spec makes multivalue work perfectly fine, even with wasm-bindgen.

[CryZe]

I guess this can be closed now then, considering multivalue works without the multivalue ABI if you just switch to the spec compliant C ABI, which is intended to be the default soon anyway.