Stream: git-wasmtime

Topic: wasmtime / PR #8117 x64: Remove load sinking from `XmmCmove`

Wasmtime GitHub notifications bot (Mar 13 2024 at 17:03):

alexcrichton opened PR #8117 from alexcrichton:fix-more-cmove-things to bytecodealliance:main:

This commit completely replaces the XmmMemAligned operand in XmmCmove with Xmm instead. Looking more into the fix #8113 I was looking to add some *.wast runtime tests to assert that the fix works at the wasm layer in addition to the instruction selection layer. I was poking around and there's a second user of XmmCmove which wasn't addressed in #8113.

In #8113 the only caller of cmove_xmm was updated to ensure that everything was always in memory. This was done to both prevent the upgrade-to-an-aligned-load from loading too much but additionally to ensure that the load always happened, regardless of the condition. There was a second constructor of XmmCmove, however, from cmove_or_xmm. This is triggered when the condition is a f64.ne instruction, for example, and ran into the same bug that #8112 was exposing.

To fix both of these and prevent any future issues about skipping a load by accident due to control flow this commit removes the XmmMemAligned argument entirely from XmmCmove and replaces it with Xmm. This prevents sinking loads entirely and sidesteps all these issues at the type level.

<!--
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 (Mar 13 2024 at 17:03):

alexcrichton requested cfallin for a review on PR #8117.

Wasmtime GitHub notifications bot (Mar 13 2024 at 17:03):

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

Wasmtime GitHub notifications bot (Mar 13 2024 at 17:03):

alexcrichton requested fitzgen for a review on PR #8117.

Wasmtime GitHub notifications bot (Mar 13 2024 at 17:03):

alexcrichton requested wasmtime-core-reviewers for a review on PR #8117.

Wasmtime GitHub notifications bot (Mar 13 2024 at 17:05):

cfallin submitted PR review:

Thanks!

Wasmtime GitHub notifications bot (Mar 13 2024 at 17:06):

cfallin has enabled auto merge for PR #8117.

Wasmtime GitHub notifications bot (Mar 13 2024 at 17:44):

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

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 (Mar 13 2024 at 17:46):

cfallin merged PR #8117.

Last updated: Dec 23 2024 at 13:07 UTC