Specify v8x16.shuffle text format

[tlively]

Separate each lane index because they are always interpreted
independently.

[tlively]

@binji does this seem reasonable to you? I don't think this is what WABT does currently.

[binji]

Seems reasonable to me, shouldn't be too hard to update wabt for this.

[gnzlbg]

I started working on a watb PR, I’ll try to submit it tomorrow

[gnzlbg]

LGTM!

[gnzlbg]

PR implementing this in WABT WebAssembly/wabt#1032

[binji]

So we're clear, the intention is to encode this as 16 immediate bytes, with each expected to be in the range [0,31], right? So it's a validation error if the values are out-of-range?

[gnzlbg]

So we're clear, the intention is to encode this as 16 immediate bytes, with each expected to be in the range [0,31], right?

I don't think that's the intent. The binary encoding (https://github.com/WebAssembly/simd/blob/master/proposals/simd/BinarySIMD.md#simd-instruction-encodings) says that the immediate mode argument is a LaneIdx32[16], that's 16 lane indices in range [0, 32), also 16 5-bit values. This matches the clarifications done in https://github.com/WebAssembly/simd/pull/30/files#diff-28263aca205221ef5f3c6b4a232654c3R293 / (e.g. see how it specifies that LaneIdx32[16] is 10 bytes wide (5 bit * 16 = 80 bits = 10 bytes):

This instruction is encoded with 10 bytes providing the indices of the elements to return. The indices i in range [0, 15] select the i-th element of a.

Otherwise the binary encoding would just use ImmBytes[16], to denote that the immediate mode operand contains 16 bytes.

So it's a validation error if the values are out-of-range?

This binary encoding does not need validation beyond the immediate mode argument being 10 bytes wide, because the 5-bit integers are always in range.

The textual encoding represents the shuffle arguments as natural numbers, which has to be validated (to avoid negative values, floating point, etc.), and these natural numbers have also to be validated to be in-range [0, 32).

[binji]

I see, it would be good to get everyone to sign off on that and land that PR first, since the current documentation is pretty explicit about it being one byte per lane index: https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#shuffle-lanes

[gnzlbg]

This parts of the docs is also very explicit about that: https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#immediate-operands

cc @lemaitre Using one byte per lane would make the WASM implementation significantly simpler, and would only introduce a small cost in binary size. What was the rationale for making the immediate mode operands as small as possible ?

[tlively]

This PR only uses i5 as part of the text format because that conveys the bounds information using conventions already set forth in the WebAssembly spec text. It is not meant to change the binary format from using one byte per index (16 index bytes total).

[gnzlbg]

@tlively yes, its the other PR open which does that change. For some reason (maybe because the actual instructions use bitpacked indices?) I thought that change was a bug fix as well, but I can't find any discussion about it anywhere.

[lemaitre]

cc @lemaitre Using one byte per lane would make the WASM implementation significantly simpler, and would only introduce a small cost in binary size. What was the rationale for making the immediate mode operands as small as possible ?

I just opened a discussion on my PR: #30

But to sum up, I just made an unverified assumption.
This has nothing to do with this PR anyway as it is about the textual representation only.