Wasmtime GitHub notifications bot (Jun 02 2025 at 18:38): Wasmtime GitHub notifications bot (Jun 02 2025 at 18:38):abrown opened PR #10898 from abrown:asm-movlhps to bytecodealliance:main:
This change continues the push to convert all
mov-related instruction and convertsmovlhpsto use the new assembler (at least the SSE variant). In doing so, it discovers that we were incorrectly allowing anXmmMemas one of the operands; this is fixed to beXmmonly.<!--
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.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:38):abrown updated PR #10898.
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:41):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:41):abrown created PR review comment:
Note how we're gaining an extra
vmovsdhere because we were probably doing some internalemit-time matching to emitvmovhps.
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:42):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:42):abrown created PR review comment:
The old version seems wrong... this _should_ be a
movsdI believe.
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:49):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:49):alexcrichton created PR review comment:
It looks like
movhpshas the exact same encoding asmovlhps, which might be how this worked beforehand? Nevertheless from an assembler point of view I'd much rather match the Intel manual for sure.Perhaps though
XmmMemcould stay as an argument on some handwritten ISLE-helper and that delegates tomovlhpsandmovhpsas appropriate?
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:49):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 02 2025 at 18:49):alexcrichton created PR review comment:
Yeah definitely wrong,
XmmMemauto-converted toXmmalways loads 128-bits, and this is just an incorrect translation and means that on-the-boundary-of-linear-memory loads of f32/f64 will be incorrectly identified as out-of-bounds.
Wasmtime GitHub notifications bot (Jun 02 2025 at 20:06):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 02 2025 at 20:06):alexcrichton created PR review comment:
For this, mind adding this test to this PR?
Wasmtime GitHub notifications bot (Jun 04 2025 at 23:37):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jun 04 2025 at 23:37):abrown created PR review comment:
Yeah, the more I stare at the manual, it could be that they are truly the same instruction and we _should_ make that second operand an
xmm_m64. I suspect that the names were different for some historical reason and we'll just have to paper over the mnemonic difference with #10918 once that is done.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:15):abrown updated PR #10898.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:23):abrown updated PR #10898.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:26):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:26):abrown created PR review comment:
Ok, I ended up creating a separate instruction with the same encoding (avoids any customizations) and everything is fine. The only real user of
vmovhpsis Winch, though. I looked through the five uses ofx64_movlhpsinlower.isleand, unless I'm missing some auto-conversion somewhere, it appears to me that we are fine using themovlhpsreg-to-reg form and don't really needmovhpsin Cranelift.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:27):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:27):abrown created PR review comment:
Added. It doesn't fail, though?
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:27):abrown has marked PR #10898 as ready for review.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:27):abrown requested alexcrichton for a review on PR #10898.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:27):abrown requested wasmtime-compiler-reviewers for a review on PR #10898.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:27):abrown requested wasmtime-core-reviewers for a review on PR #10898.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:28):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:28):abrown created PR review comment:
(Maybe it would have failed with the old version?)
Wasmtime GitHub notifications bot (Jun 11 2025 at 23:36):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 12 2025 at 00:48):alexcrichton commented on PR #10898:
Wasmtime GitHub notifications bot (Jun 12 2025 at 02:21):github-actions[bot] commented on PR #10898:
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 (Jun 12 2025 at 16:18):abrown updated PR #10898.
Wasmtime GitHub notifications bot (Jun 12 2025 at 16:25):abrown has enabled auto merge for PR #10898.
Wasmtime GitHub notifications bot (Jun 12 2025 at 17:26):abrown updated PR #10898.
Wasmtime GitHub notifications bot (Jun 12 2025 at 18:09):abrown merged PR #10898.
Last updated: Dec 06 2025 at 06:05 UTC