Function references implementation for Wasmtime by alexcrichton · Pull Request #6441 · bytecodealliance/wasmtime

alexcrichton · 2023-05-23T20:20:59Z

This is #5288 but with some more commits I've added onto the end to handle some comments I've noted during a "final" review pass. I think that everything should be in order now and there should be FIXME comments on major ticket items still remaining to do after this PR.

I've opened this as a second PR to provide my assistance in pushing this over the finish line. Many of my comments were relatively minor or stuff I would want to do quickly in a follow-up to help future-proof things as well so I figured I'd go ahead and make the changes here. The vast majority of this work is still Luna's and Daniel's!

- Always support Index in blocktypes - Support Index as table type by pretending to be Func - Etc

This will ultimately be reverted when we refer to wasm-tools#function-references, which doesn't have peek, but does have type annotations on CallRef

There certainly need to be many more, but this at least catches the bugs fixed in the next commit

This isn't only used for null references

Don't initialize non-nullable locals to null, instead skip initialization entirely and wasm validation will ensure it's always initialized in the scope where it's used.

Use a `SignatureIndex` instead of a `u32` which while not 100% correct should be more correct. This additionally renames the `Index` variant to `TypedFunc` to leave space for future types which aren't functions to not all go into an `Index` variant. This required updates to Winch because `wasmtime_environ` types can no longer be converted back to their `wasmparser` equivalents. Additionally this means that all type translation needs to go through some form of context to resolve indices which is now encapsulated in a `TypeConvert` trait implemented in various locations.

Reduce some duplication and simplify some data structures to have a more direct form of table initialization and a bit more graceful handling of element-initialized tables. Additionally element-initialize tables are now treated the same as if there's a large element segment initializing them.

github-actions · 2023-05-23T20:44:55Z

Subscribe to Label Action

cc @peterhuene

Details

This issue or pull request has been labeled: "cranelift", "cranelift:wasm", "wasmtime:api", "wasmtime:config"

Thus the following users have been cc'd because of the following labels:

peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

github-actions · 2023-05-23T20:45:15Z

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

If you added a new Config method, you wrote extensive documentation for
it.

Details

Our documentation should be of the following form:

Short, simple summary sentence.

More details. These details can be multiple paragraphs. There should be
information about not just the method, but its parameters and results as
well.

Is this method fallible? If so, when can it return an error?

Can this method panic? If so, when does it panic?

# Example

Optional example here.

If you added a new Config method, or modified an existing one, you
ensured that this configuration is exercised by the fuzz targets.

Details

For example, if you expose a new strategy for allocating the next instance
slot inside the pooling allocator, you should ensure that at least one of our
fuzz targets exercises that new strategy.

Often, all that is required of you is to ensure that there is a knob for this
configuration option in wasmtime_fuzzing::Config (or one
of its nested structs).

Rarely, this may require authoring a new fuzz target to specifically test this
configuration. See our docs on fuzzing for more details.
If you are enabling a configuration option by default, make sure that it
has been fuzzed for at least two weeks before turning it on by default.

Details

To modify this label's message, edit the .github/label-messager/wasmtime-config.md file.

To add new label messages or remove existing label messages, edit the
.github/label-messager.json configuration file.

Learn more.

alexcrichton · 2023-05-23T20:49:17Z

Actually, on second though, I'm going to do my PR-the-PR strategy instead of this.

dhil and others added 30 commits November 16, 2022 22:40

Make wasmtime-types type check

3799e10

Make wasmtime-environ type check.

7a4bc7e

Make wasmtime-runtime type check

6e73ec4

Make cranelift-wasm type check

cb0fa63

Make wasmtime-cranelift type check

11acbe2

Make wasmtime type check

37c32fc

Make wasmtime-wast type check

5af12e6

Make testsuite compile

98b26b0

Address Luna's comments

1c6eb7e

Restore compatibility with effect-handlers/wasm-tools#func-ref-2

0c69cba

Add function refs feature flag; support testing

931feeb

Provide function references support in helpers

4897c59

- Always support Index in blocktypes - Support Index as table type by pretending to be Func - Etc

Implement ref.as_non_null

fda8182

Add br_on_null

1959304

Update Cargo.lock to use wasm-tools with peek

e14fde5

This will ultimately be reverted when we refer to wasm-tools#function-references, which doesn't have peek, but does have type annotations on CallRef

Add call_ref

6e31760

Support typed function references in ref.null

aa71b96

Implement br_on_non_null

1f411e6

Remove extraneous flag; default func refs false

a589b99

Use IndirectCallToNull trap code for call_ref

b6d81e2

Factor common call_indirect / call_ref into a fn

99fba53

Remove copypasta clippy attribute / format

c199f2d

Add a some more tests for typed table instructions

8e33f18

There certainly need to be many more, but this at least catches the bugs fixed in the next commit

Fix missing typed cases for table_grow, table_fill

8482369

Document trap code; remove answered question

d30c76e

Mark wasm-tools to wasmtime reftype infallible

20907e8

Fix reversed conditional

9d0482f

Scope externref/funcref shorthands within WasmRefType

71a7e96

Merge with upstream

6765ecb

Make wasmtime compile again

15eb6fa

dhil and others added 14 commits May 23, 2023 13:50

Ignore type-equivalence test

c04a013

Replace TODOs with descriptive comments.

6f66bb5

Clarify a comment

ed97ce8

This isn't only used for null references

Resolve a TODO in local init

dda6670

Don't initialize non-nullable locals to null, instead skip initialization entirely and wasm validation will ensure it's always initialized in the scope where it's used.

Clarify a comment and skipping the null check.

45b5b4b

Remove a stray comment

02e8172

Clean up some unrelated chagnes

39dbc0c

Simplify Table bindings slightly

90a490f

Remove a no-longer-needed TODO

bf67211

Add a FIXME for SignatureIndex in WasmHeapType

9d20d39

Add a FIXME for panicking on exposing function-references types

f45c492

Fix a warning on nightly

fab54f0

alexcrichton requested review from a team as code owners May 23, 2023 20:21

alexcrichton requested review from fitzgen and removed request for a team May 23, 2023 20:21

Fix tests for winch and cranelift

24afb04

github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:wasm wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime labels May 23, 2023

alexcrichton closed this May 23, 2023

alexcrichton mentioned this pull request May 23, 2023

Various changes found during review effect-handlers/wasmtime#21

Merged

alexcrichton deleted the function-references branch May 24, 2023 14:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Function references implementation for Wasmtime#6441

Function references implementation for Wasmtime#6441
alexcrichton wants to merge 108 commits intobytecodealliance:mainfrom
alexcrichton:function-references

alexcrichton commented May 23, 2023

Uh oh!

github-actions bot commented May 23, 2023

Uh oh!

github-actions bot commented May 23, 2023

Uh oh!

alexcrichton commented May 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

alexcrichton commented May 23, 2023

Uh oh!

github-actions bot commented May 23, 2023

Subscribe to Label Action

Uh oh!

github-actions bot commented May 23, 2023

Label Messager: wasmtime:config

Uh oh!

alexcrichton commented May 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants