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

rustc: Add a new wasm ABI #83763

Merged
merged 1 commit into from
Apr 8, 2021
Merged

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Apr 1, 2021

This commit implements the idea of a new ABI for the WebAssembly target,
one called "wasm". This ABI is entirely of my own invention
and has no current precedent, but I think that the addition of this ABI
might help solve a number of issues with the WebAssembly targets.

When wasm32-unknown-unknown was first added to Rust I naively
"implemented an abi" for the target. I then went to write wasm-bindgen
which accidentally relied on details of this ABI. Turns out the ABI
definition didn't match C, which is causing issues for C/Rust interop.
Currently the compiler has a "wasm32 bindgen compat" ABI which is the
original implementation I added, and it's purely there for, well,
wasm-bindgen.

Another issue with the WebAssembly target is that it's not clear to me
when and if the default C ABI will change to account for WebAssembly's
multi-value feature (a feature that allows functions to return multiple
values). Even if this does happen, though, it seems like the C ABI will
be guided based on the performance of WebAssembly code and will likely
not match even what the current wasm-bindgen-compat ABI is today. This
leaves a hole in Rust's expressivity in binding WebAssembly where given
a particular import type, Rust may not be able to import that signature
with an updated C ABI for multi-value.

To fix these issues I had the idea of a new ABI for WebAssembly, one
called wasm. The definition of this ABI is "what you write
maps straight to wasm". The goal here is that whatever you write down in
the parameter list or in the return values goes straight into the
function's signature in the WebAssembly file. This special ABI is for
intentionally matching the ABI of an imported function from the
environment or exporting a function with the right signature.

With the addition of a new ABI, this enables rustc to:

  • Eventually remove the "wasm-bindgen compat hack". Once this multivalue
    ABI is stable wasm-bindgen can switch to using it everywhere.
    Afterwards the wasm32-unknown-unknown target can have its default ABI
    updated to match C.

  • Expose the ability to precisely match an ABI signature for a
    WebAssembly function, regardless of what the C ABI that clang chooses
    turns out to be.

  • Continue to evolve the definition of the default C ABI to match what
    clang does on all targets, since the purpose of that ABI will be
    explicitly matching C rather than generating particular function
    imports/exports.

Naturally this is implemented as an unstable feature initially, but it
would be nice for this to get stabilized (if it works) in the near-ish
future to remove the wasm32-unknown-unknown incompatibility with the C
ABI. Doing this, however, requires the feature to be on stable because
wasm-bindgen works with stable Rust.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 1, 2021
@rust-log-analyzer

This comment has been minimized.

@alexcrichton alexcrichton force-pushed the wasm-multivalue-abi branch from f6c4a2a to 676b8fe Compare April 1, 2021 23:23
@rust-log-analyzer

This comment has been minimized.

@nagisa
Copy link
Member

nagisa commented Apr 1, 2021

Are there any alternatives to adding a temporarily useful (AFAICT? Or is the plan that bindgen continues to use this ABI for a long time to come?) ABI string for effectively forever?

The goal here is that whatever you write down in
the parameter list or in the return values goes straight into the
function's signature in the WebAssembly file.

This sounds a lot like the existing "unadjusted" ABI, though as far as I can tell for wasm specifically it sets the +multivalue feature as well. Is there any reason multivalue shouldn't be a part of the target_feature system instead?

@alexcrichton
Copy link
Member Author

Sorry if this wasn't clear from the description, but my intention is that this is eventually a long-lasting and stable ABI. If your desire is to match C you'd use extern "C", but if your desire is to match or conform to a WebAssembly specification you'd use this new ABI.

After I posted this I realized that the name "wasm-multivalue" is probably too wordy and too temporary for this concept, and I might actually propose "wasm" instead. The multivalue part is here because LLVM hasn't enabled multivalue by default, but AFAIK almost all other runtimes have implemented it. Eventually wasm targets need to move towards using multivalue returns and I figured that this is a good place to start doing that. The "wasm" abi (if we change the name) would be defined in terms of the online specification, which has support for multiple returns.

Note that multivalue is part of the target_feature system already. I figured that if a new ABI is being added it would be good to unconditionally use it, so I went ahead and automatically enabled it for this new ABI.

@nagisa
Copy link
Member

nagisa commented Apr 2, 2021

I see. extern "wasm" makes much more sense to me then, yes.

There's a conflicting interaction between extern "wasm" unconditionally enabling multivalue and user explicitly requesting #[target_feature(disable="multivalue")]. Should error in that case or something.

Otherwise no other concerns from me and I'm happy to approve landing this.

@alexcrichton alexcrichton force-pushed the wasm-multivalue-abi branch from 676b8fe to 631aef1 Compare April 2, 2021 16:42
@alexcrichton alexcrichton changed the title rustc: Add a new wasm-multivalue ABI rustc: Add a new wasm ABI Apr 2, 2021
@alexcrichton
Copy link
Member Author

Ok sounds good! I've renamed the ABI to "wasm", added tests, updated existing tests, and filed a tracking issue.

AFAIK target_feature(disable) isn't possible today, and I don't think there's too many users of -multivalue (since that's already the effective default in LLVM), so I'm not sure if it's worth adding more checking for -Ctarget-feature=-multivalue

@rust-log-analyzer

This comment has been minimized.

@alexcrichton alexcrichton force-pushed the wasm-multivalue-abi branch from 631aef1 to 0665766 Compare April 2, 2021 17:25
@nagisa
Copy link
Member

nagisa commented Apr 2, 2021

r=me

@alexcrichton
Copy link
Member Author

@bors: r=nagisa

@bors
Copy link
Contributor

bors commented Apr 2, 2021

📌 Commit 0665766 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 2, 2021
@joshtriplett joshtriplett added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Apr 2, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 3, 2021
…=nagisa

rustc: Add a new `wasm` ABI

This commit implements the idea of a new ABI for the WebAssembly target,
one called `"wasm"`. This ABI is entirely of my own invention
and has no current precedent, but I think that the addition of this ABI
might help solve a number of issues with the WebAssembly targets.

When `wasm32-unknown-unknown` was first added to Rust I naively
"implemented an abi" for the target. I then went to write `wasm-bindgen`
which accidentally relied on details of this ABI. Turns out the ABI
definition didn't match C, which is causing issues for C/Rust interop.
Currently the compiler has a "wasm32 bindgen compat" ABI which is the
original implementation I added, and it's purely there for, well,
`wasm-bindgen`.

Another issue with the WebAssembly target is that it's not clear to me
when and if the default C ABI will change to account for WebAssembly's
multi-value feature (a feature that allows functions to return multiple
values). Even if this does happen, though, it seems like the C ABI will
be guided based on the performance of WebAssembly code and will likely
not match even what the current wasm-bindgen-compat ABI is today. This
leaves a hole in Rust's expressivity in binding WebAssembly where given
a particular import type, Rust may not be able to import that signature
with an updated C ABI for multi-value.

To fix these issues I had the idea of a new ABI for WebAssembly, one
called `wasm`. The definition of this ABI is "what you write
maps straight to wasm". The goal here is that whatever you write down in
the parameter list or in the return values goes straight into the
function's signature in the WebAssembly file. This special ABI is for
intentionally matching the ABI of an imported function from the
environment or exporting a function with the right signature.

With the addition of a new ABI, this enables rustc to:

* Eventually remove the "wasm-bindgen compat hack". Once this multivalue
  ABI is stable wasm-bindgen can switch to using it everywhere.
  Afterwards the wasm32-unknown-unknown target can have its default ABI
  updated to match C.

* Expose the ability to precisely match an ABI signature for a
  WebAssembly function, regardless of what the C ABI that clang chooses
  turns out to be.

* Continue to evolve the definition of the default C ABI to match what
  clang does on all targets, since the purpose of that ABI will be
  explicitly matching C rather than generating particular function
  imports/exports.

Naturally this is implemented as an unstable feature initially, but it
would be nice for this to get stabilized (if it works) in the near-ish
future to remove the wasm32-unknown-unknown incompatibility with the C
ABI. Doing this, however, requires the feature to be on stable because
wasm-bindgen works with stable Rust.
@bors
Copy link
Contributor

bors commented Apr 3, 2021

☔ The latest upstream changes (presumably #81866) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 3, 2021
@alexcrichton alexcrichton force-pushed the wasm-multivalue-abi branch from 0665766 to 940323e Compare April 5, 2021 20:47
@Dylan-DPC-zz
Copy link

failed in rollup

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 7, 2021
@bors
Copy link
Contributor

bors commented Apr 8, 2021

☔ The latest upstream changes (presumably #83981) made this pull request unmergeable. Please resolve the merge conflicts.

This commit implements the idea of a new ABI for the WebAssembly target,
one called `"wasm"`. This ABI is entirely of my own invention
and has no current precedent, but I think that the addition of this ABI
might help solve a number of issues with the WebAssembly targets.

When `wasm32-unknown-unknown` was first added to Rust I naively
"implemented an abi" for the target. I then went to write `wasm-bindgen`
which accidentally relied on details of this ABI. Turns out the ABI
definition didn't match C, which is causing issues for C/Rust interop.
Currently the compiler has a "wasm32 bindgen compat" ABI which is the
original implementation I added, and it's purely there for, well,
`wasm-bindgen`.

Another issue with the WebAssembly target is that it's not clear to me
when and if the default C ABI will change to account for WebAssembly's
multi-value feature (a feature that allows functions to return multiple
values). Even if this does happen, though, it seems like the C ABI will
be guided based on the performance of WebAssembly code and will likely
not match even what the current wasm-bindgen-compat ABI is today. This
leaves a hole in Rust's expressivity in binding WebAssembly where given
a particular import type, Rust may not be able to import that signature
with an updated C ABI for multi-value.

To fix these issues I had the idea of a new ABI for WebAssembly, one
called `wasm`. The definition of this ABI is "what you write
maps straight to wasm". The goal here is that whatever you write down in
the parameter list or in the return values goes straight into the
function's signature in the WebAssembly file. This special ABI is for
intentionally matching the ABI of an imported function from the
environment or exporting a function with the right signature.

With the addition of a new ABI, this enables rustc to:

* Eventually remove the "wasm-bindgen compat hack". Once this
  ABI is stable wasm-bindgen can switch to using it everywhere.
  Afterwards the wasm32-unknown-unknown target can have its default ABI
  updated to match C.

* Expose the ability to precisely match an ABI signature for a
  WebAssembly function, regardless of what the C ABI that clang chooses
  turns out to be.

* Continue to evolve the definition of the default C ABI to match what
  clang does on all targets, since the purpose of that ABI will be
  explicitly matching C rather than generating particular function
  imports/exports.

Naturally this is implemented as an unstable feature initially, but it
would be nice for this to get stabilized (if it works) in the near-ish
future to remove the wasm32-unknown-unknown incompatibility with the C
ABI. Doing this, however, requires the feature to be on stable because
wasm-bindgen works with stable Rust.
@alexcrichton alexcrichton force-pushed the wasm-multivalue-abi branch from fa02940 to 482a3d0 Compare April 8, 2021 15:03
@alexcrichton
Copy link
Member Author

Ah yes I forgot that node needed an update to support multivalue returns by default!

@alexcrichton
Copy link
Member Author

@bors: r=nagisa

@bors
Copy link
Contributor

bors commented Apr 8, 2021

📌 Commit 482a3d0 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 8, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 8, 2021
…=nagisa

rustc: Add a new `wasm` ABI

This commit implements the idea of a new ABI for the WebAssembly target,
one called `"wasm"`. This ABI is entirely of my own invention
and has no current precedent, but I think that the addition of this ABI
might help solve a number of issues with the WebAssembly targets.

When `wasm32-unknown-unknown` was first added to Rust I naively
"implemented an abi" for the target. I then went to write `wasm-bindgen`
which accidentally relied on details of this ABI. Turns out the ABI
definition didn't match C, which is causing issues for C/Rust interop.
Currently the compiler has a "wasm32 bindgen compat" ABI which is the
original implementation I added, and it's purely there for, well,
`wasm-bindgen`.

Another issue with the WebAssembly target is that it's not clear to me
when and if the default C ABI will change to account for WebAssembly's
multi-value feature (a feature that allows functions to return multiple
values). Even if this does happen, though, it seems like the C ABI will
be guided based on the performance of WebAssembly code and will likely
not match even what the current wasm-bindgen-compat ABI is today. This
leaves a hole in Rust's expressivity in binding WebAssembly where given
a particular import type, Rust may not be able to import that signature
with an updated C ABI for multi-value.

To fix these issues I had the idea of a new ABI for WebAssembly, one
called `wasm`. The definition of this ABI is "what you write
maps straight to wasm". The goal here is that whatever you write down in
the parameter list or in the return values goes straight into the
function's signature in the WebAssembly file. This special ABI is for
intentionally matching the ABI of an imported function from the
environment or exporting a function with the right signature.

With the addition of a new ABI, this enables rustc to:

* Eventually remove the "wasm-bindgen compat hack". Once this multivalue
  ABI is stable wasm-bindgen can switch to using it everywhere.
  Afterwards the wasm32-unknown-unknown target can have its default ABI
  updated to match C.

* Expose the ability to precisely match an ABI signature for a
  WebAssembly function, regardless of what the C ABI that clang chooses
  turns out to be.

* Continue to evolve the definition of the default C ABI to match what
  clang does on all targets, since the purpose of that ABI will be
  explicitly matching C rather than generating particular function
  imports/exports.

Naturally this is implemented as an unstable feature initially, but it
would be nice for this to get stabilized (if it works) in the near-ish
future to remove the wasm32-unknown-unknown incompatibility with the C
ABI. Doing this, however, requires the feature to be on stable because
wasm-bindgen works with stable Rust.
@bors
Copy link
Contributor

bors commented Apr 8, 2021

⌛ Testing commit 482a3d0 with merge 1255053...

@bors
Copy link
Contributor

bors commented Apr 8, 2021

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 1255053 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 8, 2021
@bors bors merged commit 1255053 into rust-lang:master Apr 8, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 8, 2021
@alexcrichton alexcrichton deleted the wasm-multivalue-abi branch April 16, 2021 16:52
@theduke
Copy link
Contributor

theduke commented May 10, 2021

@alexcrichton this is not directly related, but I'm not quite sure where else to ask.

Does or will this new ABI support anyref ?

As I understand it there currently is no mechanism that allows rustc to directly produce a signature with externrefs.
(I think wasm-bindgen manually patches the wasm when externrefs are enabled?)

@alexcrichton
Copy link
Member Author

No this doesn't help along the externref story for Rust at all, currently that's basically entirely unsupported. I'm not sure myself how externref could be added natively, if ever, to Rust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants