Stream: git-wasmtime

Topic: wasmtime / PR #10762 x64: convert some SSE shift instruct...

Wasmtime GitHub notifications bot (May 09 2025 at 22:26):

abrown opened PR #10762 from abrown:asm-sse-shift to bytecodealliance:main:

<!--
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 09 2025 at 22:26):

abrown requested wasmtime-compiler-reviewers for a review on PR #10762.

Wasmtime GitHub notifications bot (May 09 2025 at 22:26):

abrown requested cfallin for a review on PR #10762.

Wasmtime GitHub notifications bot (May 09 2025 at 22:28):

abrown submitted PR review.

Wasmtime GitHub notifications bot (May 09 2025 at 22:28):

abrown created PR review comment:

I don't really want to add this but, because of the type signatures of the inst.isle constructors I have to find a way to match on a XmmMemImm instead of GprMemImm. One alternative to adding this would be to have is_imm8 examine the more general RegMemImm instead, but this would mean the constructor types would have to change. Thoughts?

Wasmtime GitHub notifications bot (May 09 2025 at 22:28):

abrown edited PR review comment.

Wasmtime GitHub notifications bot (May 09 2025 at 22:50):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (May 09 2025 at 22:50):

alexcrichton created PR review comment:

Poking around it looks like these shifting instructions are the only ones using XmmMemImm, so perhaps we could drop that type entirely? All the various helpers seem to be using mask_xmm_shift and the current lowerings could dispatch to a helper taking the RegMemImm result of mask_xmm_shift which handles the *Imm case to delegate to different constructors than when in the Reg or Mem case?

Wasmtime GitHub notifications bot (May 10 2025 at 01:05):

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

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "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 13 2025 at 18:44):

cfallin submitted PR review:

LGTM with Alex's suggestion -- thanks!

Wasmtime GitHub notifications bot (May 14 2025 at 22:30):

alexcrichton closed without merge PR #10762.

Wasmtime GitHub notifications bot (May 14 2025 at 22:30):

alexcrichton commented on PR #10762:

I can also clarify I'm happy to see this landed as-is. @abrown mentioned he wanted to chat with me about my suggestion but I'm busy all this week and won't be back until next week so I think it's ok to land this as-is and follow-up as necessary if you'd like to @abrown

Wasmtime GitHub notifications bot (May 14 2025 at 22:31):

alexcrichton reopened PR #10762.

Wasmtime GitHub notifications bot (May 14 2025 at 22:31):

alexcrichton commented on PR #10762:

that was the wrong button...

Wasmtime GitHub notifications bot (May 15 2025 at 00:59):

abrown merged PR #10762.

Last updated: Dec 06 2025 at 06:05 UTC