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

Implement subtype checking for [return_]call_indirect instructions #9448

Merged

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Oct 10, 2024

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).

@fitzgen fitzgen requested review from a team as code owners October 10, 2024 18:10
@fitzgen fitzgen requested review from cfallin and alexcrichton and removed request for a team October 10, 2024 18:10
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).
@fitzgen fitzgen force-pushed the subtype-checking-for-call-indirect branch from 70bfce2 to b89dcb6 Compare October 10, 2024 18:11
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

funcref_ptr,
crate::TRAP_INDIRECT_CALL_TO_NULL,
);
if features.gc() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe just gc

@fitzgen
Copy link
Member Author

fitzgen commented Oct 10, 2024

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 #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.

@fitzgen fitzgen added this pull request to the merge queue Oct 10, 2024
@fitzgen fitzgen removed this pull request from the merge queue due to a manual request Oct 10, 2024
@fitzgen fitzgen added this pull request to the merge queue Oct 10, 2024
Merged via the queue into bytecodealliance:main with commit 7c96aa1 Oct 10, 2024
39 checks passed
@fitzgen fitzgen deleted the subtype-checking-for-call-indirect branch October 10, 2024 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants