Add `thread.spawn_indirect` by abrown · Pull Request #447 · WebAssembly/component-model

abrown · 2025-02-07T18:27:35Z

This change codifies the conclusions we arrived to in #89. It adds a new way to spawn threads, thread.spawn_indirect, which retrieves the thread start function from a table. This prompted me to rename thread.spawn to thread.spawn_ref.

This encoding change is necessary due to recent additions to the component model; see [bytecodealliance#447]. [bytecodealliance#447]: WebAssembly/component-model#447

[bytecodealliance#447] tries to make the built-in naming a bit more consistent; this change propagates that here. [bytecodealliance#447]: WebAssembly/component-model#447

This encoding change is necessary due to recent additions to the component model; see [bytecodealliance#447]. [bytecodealliance#447]: WebAssembly/component-model#447

[bytecodealliance#447] tries to make the built-in naming a bit more consistent; this change propagates that here. [bytecodealliance#447]: WebAssembly/component-model#447

lukewagner

Looks generally good. One initial suggestion:

This change codifies the conclusions we arrived to in [WebAssembly#89]. It adds a new way to spawn threads, `thread.spawn_indirect`, which retrieves the thread start function from a table. This prompted me to rename `thread.spawn` to `thread.spawn_ref`. [WebAssembly#89]: WebAssembly/shared-everything-threads#89

abrown · 2025-02-27T23:09:01Z

@lukewagner, I had to rebase due to some conflicts but the follow-on commits should have the changes we discussed.

lukewagner

Great, thanks! This looks good to me with these suggestions:

Co-authored-by: Luke Wagner <mail@lukewagner.name>

lukewagner

Last tiny nits:

Co-authored-by: Luke Wagner <mail@lukewagner.name>

rossberg · 2025-03-05T06:12:40Z

+* `$tbl` must refer to a table with type `(table (ref null (shared func))
+  shared)`


The table type doesn't seem to follow any syntax defined in the proposal. More importantly, we have subtyping. So I think this ought to say:

* `$tbl` must refer to a shared table whose element type matches `(ref null (shared func))`

Ah, thanks, good point!

Updated in #462.

This uses the language suggested [here] to document the table type needed for the `thread.spawn_indirect` canonical builtin. [here]: WebAssembly#447 (comment)

This uses the language suggested [here] to document the table type needed for the `thread.spawn_indirect` canonical builtin. [here]: #447 (comment)

[bytecodealliance#447] tries to make the built-in naming a bit more consistent; this change propagates that here. [bytecodealliance#447]: WebAssembly/component-model#447

This encoding change is necessary due to recent additions to the component model; see [bytecodealliance#447]. [bytecodealliance#447]: WebAssembly/component-model#447

[bytecodealliance#447] tries to make the built-in naming a bit more consistent; this change propagates that here. [bytecodealliance#447]: WebAssembly/component-model#447

This encoding change is necessary due to recent additions to the component model; see [bytecodealliance#447]. [bytecodealliance#447]: WebAssembly/component-model#447

* threads: add `thread.spawn_indirect` As discussed in [#89], this adds support for a new intrinsic, `thread.spawn_indirect`. This new operation would allow spawning a shared function stored in a table via a table index. This leaves some future work undone: - `thread.spawn` could/should be renamed to `thread.spawn_ref` - `thread.spawn_indirect` could/should take the encoding byte from `thread.hw_concurrency`--swap `0x07` for `0x06` - importantly, `thread.spawn_indirect` should gain a field indicating which type to expect in the indirect table; due to current limitations in `wasm-tools`, the locations to check once this is possible are marked with `TODO: spawn indirect types`. [#89]: WebAssembly/shared-everything-threads#89 * threads: rename `thread.spawn` to `thread.spawn_ref` [#447] tries to make the built-in naming a bit more consistent; this change propagates that here. [#447]: WebAssembly/component-model#447 * threads: move `thread.*` canonical opcodes to `0x40+` This encoding change is necessary due to recent additions to the component model; see [#447]. [#447]: WebAssembly/component-model#447 * threads: add function types to `thread.spawn_indirect` Initially I implemented `thread.spawn_indirect` without the ability to check the type of the function to spawn out of an abundance of caution (see the implementation issues described in [#89]). In the process of writing out the specification, we convinced ourselves that these problems should not apply to `thread.spawn_indirect`. This change adds the function type index necessary for doing some extra validation of `thread.spawn_indirect` and adds some tests related to this. One unimplemented TODO is what to do about shared tables: technically, the table used by a `thread.spawn_indirect` should be shared but we have so far prevented this in the component model; this can be resolved later, though. [#89]: WebAssembly/shared-everything-threads#89 * review: simplify WAST parsing

abrown commented Feb 7, 2025

View reviewed changes

Comment thread design/mvp/CanonicalABI.md Outdated

abrown mentioned this pull request Feb 7, 2025

threads: add thread.spawn_indirect bytecodealliance/wasm-tools#2042

Merged

lukewagner reviewed Feb 17, 2025

View reviewed changes

Comment thread design/mvp/CanonicalABI.md Outdated

abrown added 3 commits February 27, 2025 15:04

Fix shared syntax for ``

dc32595

Fix table type: it contains shared funcrefs

903cdb1

abrown force-pushed the spawn-indirect branch from a69f22e to 903cdb1 Compare February 27, 2025 23:06

abrown commented Feb 27, 2025

View reviewed changes

Comment thread design/mvp/CanonicalABI.md Outdated

abrown marked this pull request as ready for review February 27, 2025 23:08

lukewagner approved these changes Feb 28, 2025

View reviewed changes

abrown and others added 15 commits February 28, 2025 08:50

Update design/mvp/CanonicalABI.md

03d166d

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/CanonicalABI.md

d394e76

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/CanonicalABI.md

90ac9d4

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/CanonicalABI.md

0e4defb

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/CanonicalABI.md

7b9a6ba

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/Explainer.md

1c5f3c9

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/Binary.md

4ac8c1c

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/Explainer.md

34a5579

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/CanonicalABI.md

9e9087e

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/CanonicalABI.md

1588a27

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/CanonicalABI.md

deadb36

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Move thread.* encodings to the 0x40+ space

83f20a7

Fix Markdown links

fbbceb1

Fix line wrapping

cb42911

Update design/mvp/CanonicalABI.md

3189ccc

Co-authored-by: Luke Wagner <mail@lukewagner.name>

abrown commented Feb 28, 2025

View reviewed changes

Comment thread design/mvp/CanonicalABI.md Outdated

Fix up thread.spawn_ref variables

1c558c7

lukewagner reviewed Feb 28, 2025

View reviewed changes

Comment thread design/mvp/Binary.md Outdated

Comment thread design/mvp/Explainer.md Outdated

abrown and others added 2 commits February 28, 2025 11:44

Update design/mvp/Explainer.md

e4ea201

Co-authored-by: Luke Wagner <mail@lukewagner.name>

Update design/mvp/Binary.md

eb4f9c7

Co-authored-by: Luke Wagner <mail@lukewagner.name>

lukewagner merged commit 5c54dcd into WebAssembly:main Feb 28, 2025

abrown deleted the spawn-indirect branch February 28, 2025 20:27

rossberg reviewed Mar 5, 2025

View reviewed changes

abrown mentioned this pull request Mar 5, 2025

Fix language for thread.spawn_indirect table type #462

Merged

lukewagner pushed a commit that referenced this pull request Mar 5, 2025

Fix language for thread.spawn_indirect table type

173de57

This uses the language suggested [here] to document the table type needed for the `thread.spawn_indirect` canonical builtin. [here]: #447 (comment)

		* `$tbl` must refer to a table with type `(table (ref null (shared func))
		shared)`

Uh oh!

Conversation

abrown commented Feb 7, 2025

Uh oh!

Uh oh!

lukewagner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

abrown commented Feb 27, 2025

Uh oh!

lukewagner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukewagner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rossberg Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

lukewagner Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

abrown Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants