Stream: git-wasmtime

Topic: wasmtime / PR #4728 x64: Refactor vector_all_ones, and re...

Wasmtime GitHub notifications bot (Aug 17 2022 at 20:56):

elliottt edited PR #4728 from trevor/sse-cmp-op-bug to main:

The rules for $F32X4 and $F64X2 overlap with the rules for (multi_lane 32 4) and (multi_lane 64 2) respectively. Additionally they would return SSE opcodes that must be used with xmm_rm_r_imm, but the result of sse_cmp_op is used directly with xmm_rm_r in the definition of vector_all_ones. If these cases were to suddenly become reachable through a heuristic change in ISLE, we would start seeing panics during code generation for lowerings that rely on vector_of_ones.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 17 2022 at 20:59):

elliottt edited PR #4728 from trevor/sse-cmp-op-bug to main:

The sse_cmp_op rule had cases that would produce SseOperand values that aren't legal to use with MInst.XmmRmR, and was only used in vector_all_ones when constructing an XmmRmR value. Additionally, vector_all_ones always called sse_cmp_op with the same type, so the other cases were redundant.

The solution in this PR is to remove sse_cmp_op entirely and inline a call to x64_pcmpeqd directly in vector_all_ones, and remove the unused argument from vector_all_ones.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Aug 17 2022 at 20:59):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Aug 17 2022 at 21:00):

elliottt has enabled auto merge for PR #4728.

Wasmtime GitHub notifications bot (Aug 17 2022 at 21:30):

elliottt merged PR #4728.

Last updated: Nov 22 2024 at 16:03 UTC