Stream: git-wasmtime

Topic: wasmtime / PR #3268 Implement `Swizzle` and `Splat` for i...

Wasmtime GitHub notifications bot (Aug 31 2021 at 11:19):

dheaton-arm opened PR #3268 from implement-swizzle-splat to main:

Implemented for the Cranelift interpreter:

• Swizzle to shuffle an i8x16 SIMD vector based
  on the indices specified in another vector of the same size.

• Splat to create a SIMD vector with all lanes having the same value.

Copyright (c) 2021, Arm Limited

<!--

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 31 2021 at 12:26):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2021 at 12:26):

bjorn3 created PR review comment:

Could you return an error in case of going out-of-bounds?

Wasmtime GitHub notifications bot (Aug 31 2021 at 12:30):

dheaton-arm submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2021 at 12:30):

dheaton-arm created PR review comment:

In the docs, it's specified that the resulting lane should just be 0 for indices out of bounds - unless that's incorrect?

Wasmtime GitHub notifications bot (Aug 31 2021 at 12:41):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2021 at 12:41):

bjorn3 created PR review comment:

Didn't know that. https://github.com/bytecodealliance/wasmtime/blob/16854e73c5bcd3323a51214b267f628ac41c9607/cranelift/codegen/src/isa/x64/lower.rs#L6339-L6343

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:19):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:19):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:19):

abrown created PR review comment:

I'm slightly surprised that I allowed swizzles for types other than i8x16 (here). In the spirit of constraining the instruction set, this might be another place to consider pruning: it is a bit strange that one can send in an i8x16, rearrange it with the swizzle mask, and then have some other vector type. Any thoughts on this? @cfallin, @akirilov-arm, @jlb6740?

(In any case, this PR should merge as-is and we can remove these later if that's what we decide--thanks for finding this!).

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:19):

abrown created PR review comment:

nit: when visually checking these values, it would be more helpful if they were in hex format so we could distinguish what lanes have what values.

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:27):

abrown edited PR review comment.

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:31):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:31):

afonso360 created PR review comment:

We can fix the old backend errors by replacing the target x86_64 with target x86_64 machinst.

This causes it to be ignored on the old style x86 backend.

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:34):

abrown submitted PR review.

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:34):

abrown created PR review comment:

Perfect, let's do that then.

Wasmtime GitHub notifications bot (Aug 31 2021 at 16:38):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Sep 01 2021 at 14:31):

dheaton-arm updated PR #3268 from implement-swizzle-splat to main.

Wasmtime GitHub notifications bot (Sep 03 2021 at 09:17):

dheaton-arm updated PR #3268 from implement-swizzle-splat to main.

Wasmtime GitHub notifications bot (Sep 03 2021 at 16:28):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Sep 03 2021 at 16:28):

cfallin created PR review comment:

Agreed!

Wasmtime GitHub notifications bot (Sep 03 2021 at 16:28):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Sep 03 2021 at 16:28):

cfallin created PR review comment:

Yes, in principle I agree; I think we should be sort of systematic about this in a pass over all instructions, thinking about what types make sense, so I don't think we need to make an issue just for this, or adjust the test right now, but noted!

Wasmtime GitHub notifications bot (Sep 07 2021 at 12:32):

dheaton-arm updated PR #3268 from implement-swizzle-splat to main.

Wasmtime GitHub notifications bot (Sep 07 2021 at 16:53):

abrown merged PR #3268.

Last updated: Oct 23 2024 at 20:03 UTC