Stream: git-wasmtime

Topic: wasmtime / PR #10814 x64: Migrate `SignExtendData` to new...

Wasmtime GitHub notifications bot (May 20 2025 at 13:17):

alexcrichton requested abrown for a review on PR #10814.

Wasmtime GitHub notifications bot (May 20 2025 at 13:17):

alexcrichton requested wasmtime-compiler-reviewers for a review on PR #10814.

Wasmtime GitHub notifications bot (May 20 2025 at 13:17):

alexcrichton opened PR #10814 from alexcrichton:migrate-another-instruction to bytecodealliance:main:

I woke up early today and figure how better to start the day than to frob some assembler instructions. I'd never seen the SignExtendData before and it was trickier and more subtle than I thought. The end result of this PR is to remove this pseudo-instruction and replace it with the per-instruction equivalents that it can generate.

The first thing surprising about this instruction is that the Capstone and Intel names disagree. For example what Intel calls CBW Capstone calls cbtw. This is true for all the various instructions here so the names are all changing subtly.

The second surprising thing was that the behavior of the instruction depended on the operand size. This instruction was tasked with sign-extending data just before a div instruction which requires the operand to be in AX (sign-extended) for 8-bit division, but otherwise requires DX:AX or wider for wider division. That meant that the 8-bit version of SignExtendData would read AL and write AX, but the wider variants would read AX and write DX for example.

This involved writing a specialized helper in ISLE for handling the 16-bits-or-higher width and otherwise customizing each of the various cases for 8-bit or wider already in ISLE. Winch received appropriate minor updates as well, and in the end we've now got a few more instructions bound which Cranelift doesn't currently used but should be easier to do so in the future.

<!--
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 (May 20 2025 at 14:47):

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

Subscribe to Label Action

cc @saulecabrera

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "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 (May 20 2025 at 15:55):

abrown submitted PR review:

This is great! A note on coordination: @rahulchaphalkar is working on comparisons and I have a branch converting all the moves.

Wasmtime GitHub notifications bot (May 20 2025 at 15:55):

abrown created PR review comment:

        // Note that the Intel manual calls has different names for these

Wasmtime GitHub notifications bot (May 20 2025 at 15:55):

abrown created PR review comment:

You found a case where some extra spaces made this not match?

Wasmtime GitHub notifications bot (May 20 2025 at 16:07):

alexcrichton updated PR #10814.

Wasmtime GitHub notifications bot (May 20 2025 at 16:11):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (May 20 2025 at 16:11):

alexcrichton created PR review comment:

The failure I ran into was:

> cwtl  ;; implicit: %eax
  debug: cwtl_zo(cwtl_zo { eax: Fixed(FuzzReg(0)) })
  assembled: 98
  expected (capstone): cwtl
  actual (to_string):  cwtl  ;; implicit: %eax

thread '<unnamed>' panicked at cranelift/assembler-x64/src/fuzz.rs:36:9:
assertion `left == right` failed
  left: "cwtl "
 right: "cwtl  ;; implicit: %eax"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

so I think what's happening here may be a bug in Capstone and/or just an artifact of Capstone instructions having no operands?

Wasmtime GitHub notifications bot (May 20 2025 at 16:21):

alexcrichton has enabled auto merge for PR #10814.

Wasmtime GitHub notifications bot (May 20 2025 at 16:41):

alexcrichton merged PR #10814.

Last updated: Dec 06 2025 at 06:05 UTC