Stream: git-wasmtime

Topic: wasmtime / PR #5097 Remove redundant branch and select in...

Wasmtime GitHub notifications bot (Oct 23 2022 at 20:40):

elliottt edited PR #5097 from trevor/remove-branch-instructions to main:

As discussed in the 2022/10/19 meeting, this PR removes many of the branch and select instructions that used iflags, in favor if using brz/brnz and select in their place. Additionally, it reworks selectif_spectre_guard to take an i8 input instead of an iflags input.

For reference, the removed instructions are: br_icmp, brif, brff, trueif, trueff, and selectif.
<!--

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 (Oct 24 2022 at 17:00):

elliottt updated PR #5097 from trevor/remove-branch-instructions to main.

Wasmtime GitHub notifications bot (Oct 24 2022 at 17:23):

elliottt updated PR #5097 from trevor/remove-branch-instructions to main.

Wasmtime GitHub notifications bot (Oct 24 2022 at 17:24):

elliottt requested cfallin for a review on PR #5097.

Wasmtime GitHub notifications bot (Oct 24 2022 at 17:25):

elliottt has marked PR #5097 as ready for review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 18:46):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 18:46):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 18:46):

abrown created PR review comment:

This is an example where more work _must_ be done by the CPU due to this change, right?

Wasmtime GitHub notifications bot (Oct 24 2022 at 20:22):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 20:22):

elliottt created PR review comment:

It looks like we already generate multiple comparisons on main. I adapted the example you pointed out to work on the x64 backend (removed uses of trueff) and ran it through clif-util test as a precise-output test:

function %fflags(f32) {
block200(v0: f32):
    v1 = f32const 0x34.0p0
    v2 = ffcmp v0, v1
    brff eq v2, block201
    jump block400

block400:
    brff ord v2, block202
    jump block401

block401:
    return

block201:
    return

block202:
    trap heap_oob
}

We produce assembly that has one ucomiss for each brff, not one comparison whose flags are reused between those instructions, so the changes on this branch shouldn't affect performance.

  pushq   %rbp
  movq    %rsp, %rbp
block0:
  movl    $1112539136, %r8d
  movd    %r8d, %xmm5
  ucomiss %xmm5, %xmm0
  jp      label2
  jnz     label2; j label1
block1:
  movq    %rbp, %rsp
  popq    %rbp
  ret
block2:
  movl    $1112539136, %esi
  movd    %esi, %xmm9
  ucomiss %xmm9, %xmm0
  jnp     label3; j label4
block3:
  ud2 heap_oob
block4:
  movq    %rbp, %rsp
  popq    %rbp
  ret

For completeness, here's the assembly generated for that same example on this branch, adjusted for brff being removed:

  pushq   %rbp
  movq    %rsp, %rbp
block0:
  movl    $1112539136, %r8d
  movd    %r8d, %xmm5
  ucomiss %xmm5, %xmm0
  jp      label2
  jnz     label2; j label1
block1:
  jmp     label5
block2:
  movl    $1112539136, %esi
  movd    %esi, %xmm9
  ucomiss %xmm9, %xmm0
  jnp     label3; j label4
block3:
  ud2 heap_oob
block4:
  jmp     label5
block5:
  movq    %rbp, %rsp
  popq    %rbp
  ret

Wasmtime GitHub notifications bot (Oct 24 2022 at 20:55):

elliottt updated PR #5097 from trevor/remove-branch-instructions to main.

Wasmtime GitHub notifications bot (Oct 24 2022 at 21:02):

elliottt updated PR #5097 from trevor/remove-branch-instructions to main.

Wasmtime GitHub notifications bot (Oct 24 2022 at 21:30):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 21:30):

abrown created PR review comment:

Ah, good to know!

Wasmtime GitHub notifications bot (Oct 24 2022 at 22:59):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Oct 24 2022 at 23:14):

elliottt merged PR #5097.

Last updated: Dec 23 2024 at 12:05 UTC