Stream: git-wasmtime

Topic: wasmtime / PR #4645 x64: Remove unnecessary register use ...

Wasmtime GitHub notifications bot (Aug 08 2022 at 20:24):

elliottt opened PR #4645 from trevor/x64-opt-cmp-with-constant to main:

• Add a test case for icmps we hope to optimize
• Add a rule to better lower comparisons against constants

<!--

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 08 2022 at 20:27):

elliottt updated PR #4645 from trevor/x64-opt-cmp-with-constant to main.

Wasmtime GitHub notifications bot (Aug 08 2022 at 20:33):

elliottt edited PR #4645 from trevor/x64-opt-cmp-with-constant to main:

When lowering instructions like icmp eq (iconst 0), v2, we currently store the constant 0 in a register before performing the comparison. The result is that for inputs like the following:

  v1 = iconst.i32 1
  v2 = load.i32 v0
  v3 = icmp eq v1, v2
  brnz v3, block1

we generate this assembly:

  movl    0(%rdi), %r9d
  movl    $1, %r10d
  cmpl    %r9d, %r10d
  jz      label1; j label2

This PR introduces a case into emit_cmp that handles constants on the left-hand side of a comparison by swapping the order and reversing the operation. With this new rule, we emit the following assembly for the clif snippet above:

  movl    0(%rdi), %r8d
  cmpl    $1, %r8d
  jz      label1; j label2

<!--

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 08 2022 at 20:35):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Aug 08 2022 at 20:35):

elliottt created PR review comment:

I'm not super happy with the need for a rule priority here, and would have expected the simm32_from_value use to render it unnecessary. Any idea why it doesn't fire without higher priority?

Wasmtime GitHub notifications bot (Aug 08 2022 at 20:39):

elliottt has marked PR #4645 as ready for review.

Wasmtime GitHub notifications bot (Aug 09 2022 at 04:22):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 09 2022 at 18:42):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Aug 09 2022 at 18:42):

elliottt created PR review comment:

This can go away once #4661 is merged.

Wasmtime GitHub notifications bot (Aug 09 2022 at 20:11):

elliottt updated PR #4645 from trevor/x64-opt-cmp-with-constant to main.

Wasmtime GitHub notifications bot (Aug 09 2022 at 20:43):

elliottt has enabled auto merge for PR #4645.

Wasmtime GitHub notifications bot (Aug 09 2022 at 21:13):

elliottt updated PR #4645 from trevor/x64-opt-cmp-with-constant to main.

Wasmtime GitHub notifications bot (Aug 09 2022 at 22:34):

elliottt updated PR #4645 from trevor/x64-opt-cmp-with-constant to main.

Wasmtime GitHub notifications bot (Aug 09 2022 at 23:53):

elliottt merged PR #4645.

Last updated: Jan 10 2026 at 02:36 UTC