-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Enumerate all host calls in wasmtime_environ::HostCall
#9743
Enumerate all host calls in wasmtime_environ::HostCall
#9743
Conversation
This commit is a continuation of the plan of implementing host calls in Pulley through bytecodealliance#9665, bytecodealliance#9675, and bytecodealliance#9693. Here the `Compiler::call_indirect_host` method is updated to take a new type, `HostCall`, which indicates what type of host call is being performed. This is then serialized to a 32-bit integer which will be present in the pulley instruction being generated. This 32-bit integer will then be used to perform a dispatch (the dispatch is left for a future PR with more Pulley integration). This new `HostCall` structure is defined with `BuiltinFunctionIndex` internally. Additionally a new `ComponentBuiltinFunctionIndex` is added to enumerate the same set of indexes for components as well. Along the way the split between component transcoders/builtins were removed and they're now all lumped together in one macro for builtins. (no need to have two separate macros). This new `HostCall` is used to implement the `call_indirect_host` instruction for Pulley to fill out an unimplemented piece of code.
Yessssss |
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.
LGTM modulo a nitpick around naming
crates/cranelift/src/func_environ.rs
Outdated
} | ||
|
||
impl BuiltinFunctions { | ||
fn new(compiler: &Compiler) -> Self { | ||
Self { | ||
types: BuiltinFunctionSignatures::new(compiler), | ||
builtins: [None; BuiltinFunctionIndex::builtin_functions_total_number() as usize], | ||
builtins: [None; BuiltinFunctionIndex::max() as usize], |
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.
Nitpick: If max
returns the maximum index then this should be max + 1
. If max
returns the number of builtins then we should rename it to len
or count
or something.
Ditto just above.
crates/environ/src/builtin.rs
Outdated
@@ -259,7 +268,7 @@ macro_rules! declare_indexes { | |||
$len:expr; | |||
) => { | |||
/// Returns the total number of builtin functions. | |||
pub const fn builtin_functions_total_number() -> u32 { | |||
pub const fn max() -> u32 { |
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 let's call this count
or len
or something instead of max
.
This commit is a continuation of the plan of implementing host calls in Pulley through #9665, #9675, and #9693. Here the
Compiler::call_indirect_host
method is updated to take a new type,HostCall
, which indicates what type of host call is being performed. This is then serialized to a 32-bit integer which will be present in the pulley instruction being generated. This 32-bit integer will then be used to perform a dispatch (the dispatch is left for a future PR with more Pulley integration).This new
HostCall
structure is defined withBuiltinFunctionIndex
internally. Additionally a newComponentBuiltinFunctionIndex
is added to enumerate the same set of indexes for components as well. Along the way the split between component transcoders/builtins were removed and they're now all lumped together in one macro for builtins. (no need to have two separate macros).This new
HostCall
is used to implement thecall_indirect_host
instruction for Pulley to fill out an unimplemented piece of code.