Stream: git-wasmtime

Topic: wasmtime / PR #9085 Pulley: Add memory access instruction...

Wasmtime GitHub notifications bot (Aug 07 2024 at 19:13):

fitzgen requested alexcrichton for a review on PR #9085.

Wasmtime GitHub notifications bot (Aug 07 2024 at 19:13):

fitzgen requested wasmtime-default-reviewers for a review on PR #9085.

Wasmtime GitHub notifications bot (Aug 07 2024 at 19:13):

fitzgen opened PR #9085 from fitzgen:pulley-memory-access-big-offset to bytecodealliance:main:

I had trimmed these instructions from the original upstreaming of the Pulley interpreter because I had mistakenly believed that they were unused. Turns out they are needed for Cranelift's Pulley backend to allow for lowering certain address modes to a single instruction. The alternative, lowering the address modes to a sequence of instructions, would be a bit annoying and these instructions seem generally useful.

<!--
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 (Aug 07 2024 at 19:27):

fitzgen updated PR #9085.

Wasmtime GitHub notifications bot (Aug 07 2024 at 20:44):

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

Subscribe to Label Action

cc @fitzgen

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

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

• fitzgen: pulley

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 09 2024 at 18:31):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 09 2024 at 18:31):

alexcrichton created PR review comment:

Would it perhaps make sense to add a generlized helper load32_u_offset which takes offset: usize which no-offset/offset8/offset64 all call? (and would presumably get inlined and not actually matter at runtime)

Wasmtime GitHub notifications bot (Aug 09 2024 at 18:48):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Aug 09 2024 at 18:48):

fitzgen created PR review comment:

Mayyybe. Right now some of the offset-to-isize conversions are .into() and some are as isize which is a small hiccup, but probably inconsequential. All said, I think I'd prefer to just leave it as-is for now and see what the future brings.

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

fitzgen updated PR #9085.

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

fitzgen has enabled auto merge for PR #9085.

Wasmtime GitHub notifications bot (Aug 14 2024 at 18:01):

fitzgen merged PR #9085.

Last updated: Nov 22 2024 at 16:03 UTC