Stream: git-wasmtime

Topic: wasmtime / PR #8161 Wasm ICs / typed-funcrefs test: switc...


view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:07):

cfallin requested alexcrichton for a review on PR #8161.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:07):

cfallin requested wasmtime-core-reviewers for a review on PR #8161.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:07):

cfallin opened PR #8161 from cfallin:typed-funcref-ics-test-update to bytecodealliance:main:

…e table.

This is based on discussion in #8158:

(Ignore the part where this module doesn't actually have any update logic that would set non-null refs anywhere; it's a compile-test, not a runtest!)

Once #8159 is merged and #8160 is implemented, we should see zero branches in this test.

Of note here is that we do seem to be doing a dynamic signature check again, even though the table has a typed funcref element. Perhaps this path isn't implemented yet either?

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:07):

cfallin edited PR #8161:

This is based on discussion in #8158:

(Ignore the part where this module doesn't actually have any update logic that would set non-null refs anywhere; it's a compile-test, not a runtest!)

Once #8159 is merged and #8160 is implemented, we should see zero branches in this test.

Of note here is that we do seem to be doing a dynamic signature check again, even though the table has a typed funcref element. Perhaps this path isn't implemented yet either?

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:07):

cfallin edited PR #8161.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:23):

cfallin updated PR #8161.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:24):

cfallin edited PR #8161.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:24):

cfallin edited PR #8161:

This is based on discussion in #8158: as noted in #8160, if we use a nullable typed funcref table instead (and given that we know we'll initialize a particular slot before use on the application side, so we won't actually call a null ref), and if we have a null-ref default value, we should be able to avoid the lazy table-init mechanism entirely.

(Ignore the part where this module doesn't actually have any update logic that would set non-null refs anywhere; it's a compile-test, not a runtest!)

Once #8159 is merged and #8160 is implemented, we should see zero branches in this test.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:24):

cfallin commented on PR #8161:

(I switched back from call_indirect to table.get + call_ref because it seems the former doesn't actually omit callee signature checks even when the callee is a typed funcref)

view this post on Zulip Wasmtime GitHub notifications bot (Mar 17 2024 at 21:27):

cfallin edited PR #8161:

This is based on discussion in #8158: as noted in #8160, if we use a nullable typed funcref table instead (and given that we know we'll initialize a particular slot before use on the application side, so we won't actually call a null ref), and if we have a null-ref default value, we should be able to avoid the lazy table-init mechanism entirely.

(Ignore the part where this module doesn't actually have any update logic that would set non-null refs anywhere; it's a compile-test, not a runtest!)

Once #8159 is merged and #8160 is implemented (and table-at-constant-index is fully optimized), we should see zero branches in this test.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 08:46):

jameysharp submitted PR review:

Looks great. The collection of optimizations we're discussing should be easy and should show up very well on this test. I'm looking forward to seeing each one shrink this!

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 14:55):

alexcrichton commented on PR #8161:

it seems the former doesn't actually omit callee signature checks even when the callee is a typed funcref

Ah yes I didn't mean to imply that we already did it, only that it's not too hard to add. I've got a commit after https://github.com/bytecodealliance/wasmtime/pull/8159 which implements the optimization and the codegen for call_indirect vs table.get + call_ref is the same after that.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 15:25):

alexcrichton merged PR #8161.


Last updated: Dec 23 2024 at 13:07 UTC