Stream: git-wasmtime

Topic: wasmtime / PR #8172 Skip type checks on tables that don'...

Wasmtime GitHub notifications bot (Mar 18 2024 at 20:23):

alexcrichton requested jameysharp for a review on PR #8172.

Wasmtime GitHub notifications bot (Mar 18 2024 at 20:23):

alexcrichton requested wasmtime-compiler-reviewers for a review on PR #8172.

Wasmtime GitHub notifications bot (Mar 18 2024 at 20:23):

alexcrichton opened PR #8172 from alexcrichton:remove-call-indirect-typecheck to bytecodealliance:main:

This commit implements an optimization to skip type checks in
call_indirect for tables that don't require it. With the
function-references proposal it's possible to have tables of a single
type of function as opposed to today's default funcref which is a
heterogenous set of functions. In this situation it's possible that a
call_indirect's type tag matches the type tag of a
table-of-typed-funcref-values, meaning that it's impossible for the
type check to fail.

The type check of a function pointer in call_indirect is refactored
here to take the table's type into account. Various things are shuffled
around to ensure that the right traps still show up in the right places
but the important part is that, when possible, the type check is omitted
entirely.

Wasmtime GitHub notifications bot (Mar 18 2024 at 20:23):

alexcrichton requested wasmtime-core-reviewers for a review on PR #8172.

Wasmtime GitHub notifications bot (Mar 18 2024 at 21:28):

jameysharp submitted PR review:

This makes sense to me! I really like the way you've restructured things with the CheckIndirectCallTypeSignature enum.

Wasmtime GitHub notifications bot (Mar 18 2024 at 21:28):

jameysharp submitted PR review:

This makes sense to me! I really like the way you've restructured things with the CheckIndirectCallTypeSignature enum.

Wasmtime GitHub notifications bot (Mar 18 2024 at 21:28):

jameysharp created PR review comment:

Extreme nit-pick:

                // succeeds then we know it won't match, so trap anyway.

Wasmtime GitHub notifications bot (Mar 18 2024 at 21:32):

alexcrichton updated PR #8172.

Wasmtime GitHub notifications bot (Mar 18 2024 at 21:32):

alexcrichton has enabled auto merge for PR #8172.

Wasmtime GitHub notifications bot (Mar 18 2024 at 22:08):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Mar 18 2024 at 22:08):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Mar 18 2024 at 22:08):

fitzgen created PR review comment:

FWIW, when the GC proposal is enabled, the static immediate type on the call_indirect will be allowed to be a subtype of the actual function's type.

We don't implement this yet, but will need to.

https://webassembly.github.io/gc/core/exec/instructions.html#exec-call-indirect

Wasmtime GitHub notifications bot (Mar 18 2024 at 22:15):

alexcrichton merged PR #8172.

Wasmtime GitHub notifications bot (Mar 19 2024 at 03:16):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Mar 19 2024 at 03:16):

alexcrichton created PR review comment:

Oh good point! I keep consistently forgetting that contra/covariance applies to params/results...

This makes me think we should remove the PartialEq implementation for types in wasmtime-types like we did in the embedder API as well

Wasmtime GitHub notifications bot (Mar 19 2024 at 15:54):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Mar 19 2024 at 15:54):

fitzgen created PR review comment:

Yeah that wouldn't be a bad idea.

Last updated: Dec 23 2024 at 12:05 UTC