Stream: git-wasmtime

Topic: wasmtime / PR #9051 Winch aarch64 jmp


view this post on Zulip Wasmtime GitHub notifications bot (Jul 31 2024 at 14:49):

vulc41n opened PR #9051 from vulc41n:winch-aarch64-jmp to bytecodealliance:main:

Hey :wave:

This PR implements jmp, jmp_table and branch instructions for winch targeting aarch64.

#8321

<!--
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 (Jul 31 2024 at 14:49):

vulc41n requested cfallin for a review on PR #9051.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 31 2024 at 14:49):

vulc41n requested fitzgen for a review on PR #9051.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 31 2024 at 14:49):

vulc41n requested wasmtime-core-reviewers for a review on PR #9051.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 31 2024 at 14:49):

vulc41n requested wasmtime-compiler-reviewers for a review on PR #9051.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 31 2024 at 14:59):

vulc41n updated PR #9051.

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

cfallin submitted PR review:

Thanks! I think we need to add a little logic to handle large branch offsets and islands here (aarch64-specific issue, x64 doesn't need this) but otherwise LGTM.

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

cfallin submitted PR review:

Thanks! I think we need to add a little logic to handle large branch offsets and islands here (aarch64-specific issue, x64 doesn't need this) but otherwise LGTM.

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

cfallin created PR review comment:

We'll need an EmitIsland instruction here too; see here and here for more details on how and why.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 31 2024 at 19:44):

github-actions[bot] commented on PR #9051:

Subscribe to Label Action

cc @saulecabrera

<details>
This issue or pull request has been labeled: "winch"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2024 at 16:01):

vulc41n updated PR #9051.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2024 at 16:04):

vulc41n commented on PR #9051:

Thanks for reviewing this work :smile:

Should I push the tests ? They are obviously large (13 and 12Mb).

Please tell me if there is anything else that I can improve

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2024 at 17:19):

cfallin commented on PR #9051:

Should I push the tests ? They are obviously large (13 and 12Mb).

We have tests of the underlying parts (MachBuffer and its island-insertion logic); that is probably fine for coverage here, vs. checking in a very large test, IMHO. If we wanted to do better in the future we could add the equivalent of Cranelift's chaos mode choice to sometimes artificially lower the island threshold, to avoid the need for very large inputs to trigger this; but no need to do that in this PR.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2024 at 17:22):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2024 at 17:37):

cfallin merged PR #9051.


Last updated: Dec 23 2024 at 12:05 UTC