Skip to content

add a set of apis to configure wasi-nn via InstantiationArgs2#4764

Open
zhanheng1 wants to merge 36 commits intobytecodealliance:mainfrom
dongsheng28849455:InstantiationArgs2-wasinn
Open

add a set of apis to configure wasi-nn via InstantiationArgs2#4764
zhanheng1 wants to merge 36 commits intobytecodealliance:mainfrom
dongsheng28849455:InstantiationArgs2-wasinn

Conversation

@zhanheng1
Copy link
Copy Markdown
Contributor

add a set of apis to configure wasi-nn via InstantiationArgs2:

  1. Add a new command line option for wasi-nn: --wasi-nn-graph=encoding:target:model_path1:model_path2:...:model_pathN;
  2. Add new structs to manage this cmd option: WASINNGlobalContext and wasi_nn_graph_registry;
  3. Add 2 new parameters for load_by_name(encoding and target). load_by_name() can get encoding and target from WASINNGlobalContext and set into backend.
  4. Update test codes.

@zhanheng1 zhanheng1 force-pushed the InstantiationArgs2-wasinn branch from 77794ae to c73e4aa Compare December 19, 2025 07:07
@zhanheng1 zhanheng1 marked this pull request as ready for review December 23, 2025 01:52
@zhanheng1 zhanheng1 force-pushed the InstantiationArgs2-wasinn branch from 302f32b to 9d8e31a Compare March 20, 2026 07:06

#if WASM_ENABLE_WASI_NN != 0 || WASM_ENABLE_WASI_EPHEMERAL_NN != 0
wasm_runtime_wasi_nn_registry_create(&nn_registry);
wasi_nn_set_inst_args(inst_args, nn_registry, &wasi_nn_parse_ctx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this be done before inst_args is used? ie. before wasm_runtime_instantiate_ex2

Copy link
Copy Markdown
Contributor Author

@zhanheng1 zhanheng1 Mar 25, 2026

Choose a reason for hiding this comment

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

If I recall correctly, we previously discussed this issue.
Since wasm_runtime_malloc() requires wasm_runtime_instantiate_ex2() to initialize the memory first, this logic should either be implemented inside wasm_runtime_instantiate_ex2(), or extracted as a separate function to be called after wasm_runtime_instantiate_ex2().
Originally, the function was placed in the middle of wasm_runtime_instantiate_ex2(), but it was later suggested that extracting it as a standalone function would be better, which is why the current version was adopted.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't remember the discussion. was i involved?

wasm_runtime_malloc doesn't require instance memory. do you mean wasm_module_malloc?
i think it's fine to use wasm_runtime_malloc to allocate the registry. after all, the registry doesn't necessarily belong to a specific module or instance. i vaguely remember we had a brief discussion about possible use cases to shared a registry among instances.

i remember i suggested to make wasm_runtime_instantiation_args_set_wasi_nn_registry just assign a pointer.
that is,

struct InstantiationArgs2 {
   ....
#if WASM_ENABLE_WASI_NN != 0 || WASM_ENABLE_WASI_EPHEMERAL_NN != 0
    WASINNRegistry *nn_registry;
#endif
}

void
wasm_runtime_instantiation_args_set_wasi_nn_registry(
    struct InstantiationArgs2 *p, WASINNRegistry *registry)
{
    p->nn_registry = registry;
}

and make wasm_instantiate and aot_instantiate call wasm_runtime_set_wasi_nn_registry internally to assign args->nn_registry to the new instance.

#if WASM_ENABLE_WASI_NN != 0 || WASM_ENABLE_WASI_EPHEMERAL_NN != 0
wasm_runtime_wasi_nn_registry_create(&nn_registry);
wasi_nn_set_inst_args(inst_args, nn_registry, &wasi_nn_parse_ctx);
wasm_runtime_set_wasi_nn_registry(wasm_module_inst, nn_registry);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wasm_runtime_set_wasi_nn_registry is not supposed to be used by the embedder, is it?

i suppose the registry is passed via inst_args.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I remember correctly, we've also discussed this issue before.
Since inst_args is destroyed immediately in the next line of code, the WASM application cannot actually use it.
The current version stores the registry in a slot within each WASM module's context, allowing the WASM app to retrieve the registry via the module's context.
Therefore, wasm_runtime_set_wasi_nn_registry should be explicitly called in main.c.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

inst_args is, as its name suggests, only for wasm_runtime_instantiate_ex2.
it's expected necessary info (in this case a pointer to the registry object)
is copied during instantiation.

…d graph_paths.

Add check for some wasm_runtime_malloc() calls.
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.

3 participants