Skip to content

Implement subtype checking for [return_]call_indirect instructions#9448

Merged
fitzgen merged 2 commits into
bytecodealliance:mainfrom
fitzgen:subtype-checking-for-call-indirect
Oct 10, 2024
Merged

Implement subtype checking for [return_]call_indirect instructions#9448
fitzgen merged 2 commits into
bytecodealliance:mainfrom
fitzgen:subtype-checking-for-call-indirect

Conversation

@fitzgen
Copy link
Copy Markdown
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 alexcrichton and cfallin 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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