Skip to content

wasm-opt missing support for table.fill #5939

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

Closed
kg opened this issue Sep 14, 2023 · 3 comments · Fixed by #5949
Closed

wasm-opt missing support for table.fill #5939

kg opened this issue Sep 14, 2023 · 3 comments · Fixed by #5949

Comments

@kg
Copy link

kg commented Sep 14, 2023

While wasm-opt appears to support table.grow correctly, any compiled module containing a table.fill instruction will fail to parse. This is true even if you pass the appropriate --enable-reference-types flag.

Sample code:

void
mono_jiterp_grow_function_table (int count, int fill_value) {
	__asm(
		// table.grow expects stack layout [fill_ref] [count]
		// start by loading fill_value, then fetching that reference from the indirect fn table
		"local.get 1;"
		"table.get __indirect_function_table;"
		// now fill_ref is on the stack, load count and perform the actual table grow
		"local.get 0;"
		"table.grow __indirect_function_table;"
		// table.grow returns either the new size or an error code. i'm too lazy to deal with it
		"drop;"
	);
}

#if 0
void
mono_jiterp_fill_function_table (int offset, int count, int fill_value) {
	__asm(
		// table.fill expects stack layout [offset] [fill_ref] [count]
		// load offset
		"local.get 0;"
		// load fill_value, then fetch that reference from the indirect fn table
		"local.get 2;"
		"table.get __indirect_function_table;"
		// now fill_ref is on the stack, load count and perform the actual table grow
		"local.get 1;"
		"table.fill __indirect_function_table;"
	);
}
#endif

change the #if 0 to #if 1 and the resulting built module will cause wasm-opt to fail with something like:

[parse exception: invalid code after misc prefix: 17 (at 0:266808)]
  Fatal: error parsing wasm (try --debug for more info)
emcc : error : '/home/kate/Projects/dotnet-runtime-wasm/src/mono/wasm/emsdk/upstream/bin/wasm-opt --strip-dwarf --post-emscripten -O2 --low-memory-unused --zero-filled-memory --pass-arg=directize-initial-contents-immutable --strip-debug --strip-producers dotnet.native.wasm -o dotnet.native.wasm -g --mvp-features --enable-mutable-globals --enable-reference-types --enable-sign-ext --enable-simd' failed (returned 1) [/home/kate/Projects/dotnet-runtime-wasm/src/mono/wasm/wasm.proj]

From checking the spec, 17 is table.fill. Looking through the repo it seems like support for table.init and table.copy might be missing too, but I'm not planning to use them...

@tlively
Copy link
Member

tlively commented Sep 14, 2023

You're the first person to ever ask for table.fill support! I can add it today.

tlively added a commit that referenced this issue Sep 16, 2023
This instruction was standardized as part of the bulk memory proposal, but we
never implemented it until now. Leave similar instructions like table.copy as
future work.

Fixes #5939.
tlively added a commit that referenced this issue Sep 18, 2023
This instruction was standardized as part of the bulk memory proposal, but we
never implemented it until now. Leave similar instructions like table.copy as
future work.

Fixes #5939.
@csjh
Copy link

csjh commented May 20, 2024

Any chance table.init is being worked on? Seems like it's the lonely last table instruction left

@tlively
Copy link
Member

tlively commented May 20, 2024

There's no active work at the moment, but we might implement table.init soon as part of #6612.

radekdoulik pushed a commit to dotnet/binaryen that referenced this issue Jul 12, 2024
This instruction was standardized as part of the bulk memory proposal, but we
never implemented it until now. Leave similar instructions like table.copy as
future work.

Fixes WebAssembly#5939.
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 a pull request may close this issue.

3 participants