Stream: git-wasmtime

Topic: wasmtime / PR #9435 Implement the Wasm GC instructions fo...

Wasmtime GitHub notifications bot (Oct 09 2024 at 23:43):

fitzgen opened PR #9435 from fitzgen:any-extern-conversions to bytecodealliance:main:

This commit implements two instructions:

1. any.convert_extern
2. extern.convert_any

These instructions are used to convert between anyref and externref values. The any.convert_extern instruction takes an anyref value and converts it to an externref value. The extern.convert_any instruction takes an externref value and converts it to an anyref value.

Rather than implementing wrapper objects -- for example an struct AnyOfExtern(ExternRef) type that is a subtype of AnyRef -- we instead reuse the same representation converted references as their unconverted reference. For example, (any.convert_extern my_externref) is identical to the original my_externref value. This means that we don't actually emit any clif instructions to implement these Wasm instructions; they are no-ops!

Wasm code remains none-the-wiser because it cannot directly test for the difference between, for example, a my_anyref and the result of (extern.convert_any my_anyref) because they are in two different type hierarchies, so any direct ref.test would be invalid. The Wasm code would have to convert one into the other's type hierarchy, at which point it doesn't know whether wrapping/unwrapping took place.

We did need some changes at the host API and host API implementation levels, however:

• We needed to relax the requirement that a wasmtime::AnyRef only wraps a VMGcRef that points to an object that is a subtype of anyref and similar for wasmtime::ExternRef.

• We needed to make the wasmtime::ExternRef::data[_mut] methods return an option of their associated host data, since any externref resulting from (extern.convert_any ...) does not have any associated host data. (This change would have been required either way, even if we used wrapper objects.)

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Oct 09 2024 at 23:43):

fitzgen requested alexcrichton for a review on PR #9435.

Wasmtime GitHub notifications bot (Oct 09 2024 at 23:43):

fitzgen requested wasmtime-core-reviewers for a review on PR #9435.

Wasmtime GitHub notifications bot (Oct 09 2024 at 23:43):

fitzgen edited PR #9435:

This commit implements two instructions:

1. any.convert_extern
2. extern.convert_any

These instructions are used to convert between anyref and externref values. The any.convert_extern instruction takes an externref value and converts it to an anyref value. The extern.convert_any instruction takes an anyref value and converts it to an externref value.

Rather than implementing wrapper objects -- for example an struct AnyOfExtern(ExternRef) type that is a subtype of AnyRef -- we instead reuse the same representation converted references as their unconverted reference. For example, (any.convert_extern my_externref) is identical to the original my_externref value. This means that we don't actually emit any clif instructions to implement these Wasm instructions; they are no-ops!

Wasm code remains none-the-wiser because it cannot directly test for the difference between, for example, a my_anyref and the result of (extern.convert_any my_anyref) because they are in two different type hierarchies, so any direct ref.test would be invalid. The Wasm code would have to convert one into the other's type hierarchy, at which point it doesn't know whether wrapping/unwrapping took place.

We did need some changes at the host API and host API implementation levels, however:

• We needed to relax the requirement that a wasmtime::AnyRef only wraps a VMGcRef that points to an object that is a subtype of anyref and similar for wasmtime::ExternRef.

• We needed to make the wasmtime::ExternRef::data[_mut] methods return an option of their associated host data, since any externref resulting from (extern.convert_any ...) does not have any associated host data. (This change would have been required either way, even if we used wrapper objects.)

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Oct 10 2024 at 00:32):

fitzgen requested wasmtime-fuzz-reviewers for a review on PR #9435.

Wasmtime GitHub notifications bot (Oct 10 2024 at 00:32):

fitzgen updated PR #9435.

Wasmtime GitHub notifications bot (Oct 10 2024 at 02:08):

github-actions[bot] commented on PR #9435:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:c-api", "wasmtime:ref-types"

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

• fitzgen: fuzzing, wasmtime:ref-types

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Oct 10 2024 at 02:57):

fitzgen updated PR #9435.

Wasmtime GitHub notifications bot (Oct 10 2024 at 14:34):

alexcrichton created PR review comment:

Given the footgun of using as_externref_unchecked would it make sense to remove that method entirely?

Wasmtime GitHub notifications bot (Oct 10 2024 at 14:34):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 10 2024 at 14:41):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Oct 10 2024 at 14:41):

fitzgen created PR review comment:

Sure, I can make a follow up PR to do that, don't think it will be hard at this point.

Wasmtime GitHub notifications bot (Oct 10 2024 at 15:03):

fitzgen merged PR #9435.

Last updated: Nov 22 2024 at 16:03 UTC