Stream: git-wasmtime

Topic: wasmtime / PR #9448 Implement subtype checking for `[retu...

Wasmtime GitHub notifications bot (Oct 10 2024 at 18:10):

fitzgen opened PR #9448 from fitzgen:subtype-checking-for-call-indirect to bytecodealliance:main:

When Wasm GC is enabled, the [return_]call_indirect instructions must do full subtyping checks, rather than simple strict equality type checks.

This adds an additional branch and slow path to indirect calls, so we only emit code for this check when Wasm GC is enabled, even though it would otherwise be correct to always emit it (because the is_subtype check would always fail for non-equal types, since there is no subtyping before Wasm GC).

<!--
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 18:10):

fitzgen requested cfallin for a review on PR #9448.

Wasmtime GitHub notifications bot (Oct 10 2024 at 18:10):

fitzgen requested wasmtime-compiler-reviewers for a review on PR #9448.

Wasmtime GitHub notifications bot (Oct 10 2024 at 18:10):

fitzgen requested alexcrichton for a review on PR #9448.

Wasmtime GitHub notifications bot (Oct 10 2024 at 18:10):

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

Wasmtime GitHub notifications bot (Oct 10 2024 at 18:11):

fitzgen updated PR #9448.

Wasmtime GitHub notifications bot (Oct 10 2024 at 18:38):

fitzgen updated PR #9448.

Wasmtime GitHub notifications bot (Oct 10 2024 at 19:55):

alexcrichton submitted PR review:

Mind filing an issue for this performance issue as well? This to me is going to be a hard blocker for enabling GC by default because a failed subtype check will involve acquiring a global lock even for non-GC modules which may not be appropriate for all embedders.

Wasmtime GitHub notifications bot (Oct 10 2024 at 19:55):

alexcrichton created PR review comment:

Should this be features.function_references() || features.gc()? Or just features.function_references()?

Wasmtime GitHub notifications bot (Oct 10 2024 at 19:55):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 10 2024 at 21:53):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Oct 10 2024 at 21:53):

fitzgen created PR review comment:

I believe just gc

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

fitzgen commented on PR #9448:

Mind filing an issue for this performance issue as well? This to me is going to be a hard blocker for enabling GC by default because a failed subtype check will involve acquiring a global lock even for non-GC modules which may not be appropriate for all embedders.

Added an item to https://github.com/bytecodealliance/wasmtime/issues/9351

I think much of this is equivalent to making ref.test fast (i.e. expose the supertypes arrays to wasm so that we can do the O(1) subtype check inline, rather than behind a lock) but even after we address that, it is another branch to another slow path to do a full subtype check when the types don't exactly match.

Wasmtime GitHub notifications bot (Oct 10 2024 at 22:42):

fitzgen merged PR #9448.

Last updated: Oct 23 2024 at 20:03 UTC