Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support wasm select instruction with V128-typed operands. #2391

Merged

Conversation

julian-seward1
Copy link
Contributor

  • this requires upgrading to wasmparser 0.67.0.

  • There are no CLIF side changes because the CLIF select instruction is
    polymorphic enough.

  • on aarch64, there is unfortunately no conditional-move (csel) instruction on
    vectors. This patch adds a synthetic instruction VecCSel which does
    behave like that. At emit time, this is emitted as an if-then-else diamond
    (4 insns).

  • aarch64 implementation is otherwise straightforwards.

@julian-seward1
Copy link
Contributor Author

cc @yurydelendik

@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:wasm fuzzing Issues related to our fuzzing infrastructure lightbeam Issues related to the Lightbeam compiler wasmtime:api Related to the API of the `wasmtime` crate itself labels Nov 11, 2020
@github-actions
Copy link

Subscribe to Label Action

cc @fitzgen, @peterhuene

This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:wasm", "fuzzing", "lightbeam", "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: fuzzing
  • peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@@ -1412,6 +1412,8 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
ctx.emit(Inst::FpuCSel32 { cond, rd, rn, rm });
} else if is_float && bits == 64 {
ctx.emit(Inst::FpuCSel64 { cond, rd, rn, rm });
} else if is_float && bits == 128 {
ctx.emit(Inst::VecCSel { cond, rd, rn, rm });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note, it is weird to see that ctx.emit(Inst::CSel ... below did not have assert to prevent float/128 to become csel.

* this requires upgrading to wasmparser 0.67.0.

* There are no CLIF side changes because the CLIF `select` instruction is
  polymorphic enough.

* on aarch64, there is unfortunately no conditional-move (csel) instruction on
  vectors.  This patch adds a synthetic instruction `VecCSel` which *does*
  behave like that.  At emit time, this is emitted as an if-then-else diamond
  (4 insns).

* aarch64 implementation is otherwise straightforwards.
@julian-seward1 julian-seward1 merged commit 41e87a2 into bytecodealliance:main Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:wasm cranelift Issues related to the Cranelift code generator fuzzing Issues related to our fuzzing infrastructure lightbeam Issues related to the Lightbeam compiler wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants