Stream: git-wasmtime

Topic: wasmtime / PR #9051 Winch aarch64 jmp

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:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

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
-->

Wasmtime GitHub notifications bot (Jul 31 2024 at 14:49):

vulc41n requested cfallin for a review on PR #9051.

Wasmtime GitHub notifications bot (Jul 31 2024 at 14:49):

vulc41n requested fitzgen for a review on PR #9051.

Wasmtime GitHub notifications bot (Jul 31 2024 at 14:49):

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

Wasmtime GitHub notifications bot (Jul 31 2024 at 14:49):

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

Wasmtime GitHub notifications bot (Jul 31 2024 at 14:59):

vulc41n updated PR #9051.

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.

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.

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.

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:

• saulecabrera: winch

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Aug 01 2024 at 16:01):

vulc41n updated PR #9051.

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

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.

Wasmtime GitHub notifications bot (Aug 01 2024 at 17:22):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 01 2024 at 17:37):

cfallin merged PR #9051.

Last updated: Oct 23 2024 at 20:03 UTC