Wasmtime GitHub notifications bot (May 15 2025 at 21:28): Wasmtime GitHub notifications bot (May 15 2025 at 21:28):abrown opened PR #10782 from abrown:asm-mul to bytecodealliance:main:
Multiplication on x86 is a bit tricky: there are a few different instruction formats, some gaps, and some implicit registers holding different sets of bits (
rax,rdx). This PR converts all x64 backend multiplications to use the new assembler and, in doing so, adds support for implicit registers: these are operands that need to be tracked during register allocation but may not show up in instruction disassembly. There is more that _could_ be done to refactor these multiplication rules but that could come later.
Wasmtime GitHub notifications bot (May 15 2025 at 21:32):abrown submitted PR review.
Wasmtime GitHub notifications bot (May 15 2025 at 21:32):abrown created PR review comment:
Once again (as in #10762), we run into ISLE difficulties matching immediates. I did put up #10775, but I'm not really sure if it helps all that much. Another option to avoid these extra conversions would be to wrap up the
i32in aRegMemImm.Immand then immediately unwrap it, but that confuses the type declaration. Not sure what to do, but it seems this is ripe for some refactoring...
Wasmtime GitHub notifications bot (May 15 2025 at 22:46):github-actions[bot] commented on PR #10782:
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 (May 16 2025 at 09:19):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (May 16 2025 at 09:19):alexcrichton created PR review comment:
One thing that can be a bit unfortunate here is that this implementation hides information of pre-register-allocation instructions that have implicit operands and the operands won't get printed. That seems like it could create a pretty surprising situation where an instruction uses an vreg operand but it doesn't show up in the disassembly.
Would it be possible to skip printing the operands post-regalloc but print them, maybe in a comment, pre-regalloc?
Wasmtime GitHub notifications bot (May 16 2025 at 09:19):alexcrichton created PR review comment:
I'm a bit confused, how is this related to #10762? Here the argument is
i32, not a*MemImm, so this to me at least looks like a normal what I'd expect "match the size/value of the immediate and emit an appropriate instruction"
Wasmtime GitHub notifications bot (May 16 2025 at 09:19):alexcrichton created PR review comment:
We have a number of conversions along these lines in
prelude.isleof varying names (e.g.u8_try_from_u64)Could the conversions added here purely between integer types get moved over to
prelude.isleand named similarly?
Wasmtime GitHub notifications bot (May 16 2025 at 23:31):abrown submitted PR review.
Wasmtime GitHub notifications bot (May 16 2025 at 23:31):abrown created PR review comment:
What I mean is that it's not easy to line up all the types so that matching can be clear and simple; this is not a problem with ISLE per se but with the network of rules that currently exist in the
*.islefiles.
Wasmtime GitHub notifications bot (May 18 2025 at 09:32):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (May 18 2025 at 09:32):alexcrichton created PR review comment:
Hm I'm not sure I understand still, but nevertheless this whole PR looks quite reasonable to me so I wouldn't have any qualms about landing it myself.
Wasmtime GitHub notifications bot (May 19 2025 at 19:47):abrown updated PR #10782.
Wasmtime GitHub notifications bot (May 19 2025 at 19:48):abrown submitted PR review.
Wasmtime GitHub notifications bot (May 19 2025 at 19:48):abrown created PR review comment:
Yup, made up some new syntax:
... ;; implicit: %rax.
Wasmtime GitHub notifications bot (May 19 2025 at 19:48):abrown submitted PR review.
Wasmtime GitHub notifications bot (May 19 2025 at 19:48):abrown created PR review comment:
Moved.
Wasmtime GitHub notifications bot (May 19 2025 at 19:49):abrown submitted PR review.
Wasmtime GitHub notifications bot (May 19 2025 at 19:49):abrown created PR review comment:
Moving the extractors to
prelude.islemakes this better.
Wasmtime GitHub notifications bot (May 19 2025 at 19:50):abrown has marked PR #10782 as ready for review.
Wasmtime GitHub notifications bot (May 19 2025 at 19:50):abrown requested cfallin for a review on PR #10782.
Wasmtime GitHub notifications bot (May 19 2025 at 19:50):abrown requested wasmtime-compiler-reviewers for a review on PR #10782.
Wasmtime GitHub notifications bot (May 19 2025 at 19:50):abrown requested alexcrichton for a review on PR #10782.
Wasmtime GitHub notifications bot (May 19 2025 at 19:50):abrown requested wasmtime-core-reviewers for a review on PR #10782.
Wasmtime GitHub notifications bot (May 19 2025 at 19:56):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (May 19 2025 at 19:56):alexcrichton has enabled auto merge for PR #10782.
Wasmtime GitHub notifications bot (May 19 2025 at 20:22):alexcrichton merged PR #10782.
Last updated: Dec 06 2025 at 06:05 UTC