-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Skip type checks on tables that don't need it #8172
Skip type checks on tables that don't need it #8172
Conversation
Have the same function with slightly different variations to compare codegen between the possible strategies.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me! I really like the way you've restructured things with the CheckIndirectCallTypeSignature
enum.
Co-authored-by: Jamey Sharp <jamey@minilop.net>
// Otherwise if the types don't match then either (a) this is a | ||
// null pointer or (b) it's a pointer with the wrong type. | ||
// Figure out which and trap here. | ||
// | ||
// Note that the callee may be null in which case this load may | ||
// trap. If so use the `IndirectCallToNull` trap code. | ||
let mem_flags = ir::MemFlags::trusted().with_readonly(); | ||
let callee_sig_id = self.builder.ins().load( | ||
sig_id_type, | ||
mem_flags.with_trap_code(Some(ir::TrapCode::IndirectCallToNull)), | ||
funcref_ptr, | ||
i32::from(self.env.offsets.ptr.vm_func_ref_type_index()), | ||
); | ||
// If it's possible to have a null here then try to load the | ||
// type information. If that fails due to the function being a | ||
// null pointer, then this was a call to null. Otherwise if it | ||
// succeeds then we know it won't match, so trap anyway. | ||
if table.table.wasm_ty.nullable { | ||
let mem_flags = ir::MemFlags::trusted().with_readonly(); | ||
self.builder.ins().load( | ||
sig_id_type, | ||
mem_flags.with_trap_code(Some(ir::TrapCode::IndirectCallToNull)), | ||
funcref_ptr, | ||
i32::from(self.env.offsets.ptr.vm_func_ref_type_index()), | ||
); | ||
} | ||
self.builder.ins().trap(ir::TrapCode::BadSignature); | ||
return CheckIndirectCallTypeSignature::StaticTrap; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that wouldn't be a bad idea.
This commit implements an optimization to skip type checks in
call_indirect
for tables that don't require it. With thefunction-references proposal it's possible to have tables of a single
type of function as opposed to today's default
funcref
which is aheterogenous set of functions. In this situation it's possible that a
call_indirect
's type tag matches the type tag of atable
-of-typed-funcref
-values, meaning that it's impossible for thetype check to fail.
The type check of a function pointer in
call_indirect
is refactoredhere 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.