Stream: git-wasmtime

Topic: wasmtime / PR #10887 x64: Migrate `mulx` to the new assem...

Wasmtime GitHub notifications bot (May 31 2025 at 18:24):

alexcrichton opened PR #10887 from alexcrichton:x64-mulx to bytecodealliance:main:

This is an interesting instruction as it has a relatively unique shape compared to many others. The VEX encoding is used to give it a 3-operand form, although it still has an implicit 4th operand as well. The other unique part about this instruction is that if the two write-only operands are the same then that has a different semantic meaning than if they are different.

Modeling the two-output form of the instruction was pretty easy, the only changes needed were to add the r32a and r32b locations as previously only r32 was available. Modeling the one-output form of the instruction led to a "hook" where these instructions specify that they use a custom regalloc function. That skips the auto-generated regalloc entirely and defers to a new custom submodule in the assembler crate. These custom variants handle only having a single write operand on the instruction.

<!--
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 31 2025 at 18:24):

alexcrichton requested fitzgen for a review on PR #10887.

Wasmtime GitHub notifications bot (May 31 2025 at 18:24):

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

Wasmtime GitHub notifications bot (May 31 2025 at 18:24):

alexcrichton requested abrown for a review on PR #10887.

Wasmtime GitHub notifications bot (May 31 2025 at 18:31):

alexcrichton updated PR #10887.

Wasmtime GitHub notifications bot (May 31 2025 at 19:44):

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

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "cranelift:meta", "isle"

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

• cfallin: isle
• fitzgen: isle

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 (Jun 02 2025 at 17:42):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Jun 02 2025 at 17:42):

abrown created PR review comment:

I guess there's no way to pass the same temporary register in... I wonder if eventually we need to allow even more fine-grained control from ISLE — a few new gen_asm.rs rules?

Wasmtime GitHub notifications bot (Jun 02 2025 at 17:42):

abrown created PR review comment:

I really like this custom* idea: I'd been mulling over how to do custom encodings for nop and custom printing for comparisons and felt we needed an escape hatch for very special instructions. And I like that we just expect custom::visit_{inst} to be present, which should be pretty clear in compile errors.

I don't know if you were thinking this way, but maybe in the future we can go to .custom(Visit | Encode | Display) which would result in an appropriate call to custom::visit::{inst}, custom::encode::{inst}, and custom::display::{inst}. Thoughts?

Wasmtime GitHub notifications bot (Jun 02 2025 at 17:49):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 02 2025 at 17:49):

alexcrichton created PR review comment:

I think that's reasonable yeah!

Wasmtime GitHub notifications bot (Jun 02 2025 at 17:50):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 02 2025 at 17:50):

alexcrichton created PR review comment:

I was wondering the same yeah, but I figured that if it's just mulx then there's no need to go to such lengths, so I figured I'd leave it as-is for now. If the number of instructions needing this grows though I can see how making new ISLE helpers would be reasonable

Wasmtime GitHub notifications bot (Jun 02 2025 at 18:03):

alexcrichton updated PR #10887.

Wasmtime GitHub notifications bot (Jun 02 2025 at 18:03):

alexcrichton has enabled auto merge for PR #10887.

Wasmtime GitHub notifications bot (Jun 02 2025 at 20:37):

alexcrichton merged PR #10887.

Last updated: Dec 06 2025 at 06:05 UTC