Inefficient x64 codegen for swizzle

[abrown]

Looking at https://github.com/WebAssembly/simd/blob/master/proposals/simd/SIMD.md#swizzling-using-variable-indices I discovered that it would take me more than one instruction to implement v128.swizzle on x86. I had assumed, like @stoklund in #11, that I would be able to use PSHUFB as-is. However, I am now convinced that the assumptions of #11 may be incorrect:

Lanes with an out-of-range selector become 0 in the output vector.

According to the Intel manual (and some experiments I ran), PSHUFB uses the four least significant bits to decide which lane to grab from a vector. If the most significant bit is one (e.g. 0b10000000), then the result is zeroed. But index values in between 0x0f and 0x80 will use the four least significant bits as an index and will not zero the value. To correctly implement the spec as it currently reads we would need to copy the swizzle mask to another register, do a greater-than comparison to get a bit in the most significant position, and OR this with the original swizzle mask before using the PSHUFB instruction--four instructions instead of one.

Should v128.swizzle change to allow more optimal implementations? Are there considerations for other architectures that I am not aware of?

[AndrewScheidecker]

See this comment for a relatively efficient way to compile it for SSE. You just need an unsigned saturated add of 112 to set the MSB on out-of-range lanes.

[abrown]

Cool, I guess that answers my question; thanks!

[jlb6740]

See this comment for a relatively efficient way to compile it for SSE. You just need an unsigned saturated add of 112 to set the MSB on out-of-range lanes.

@abrown @AndrewScheidecker Hi guys ... I am following some SIMD implementation work and now looking at the SIMD spec more closely. Looking at this thread and then seeing the conversation here: #91, at the surface it seems the definition of instructions like this may be diluting the capabilities of x86? @abrown how many more instructions will this take ... the thread pointed to says two?

[abrown]

I used three:

• MOVUPS to get the magic constant in place (112 in decimal, 0x70707070... in memory)
• PADDUSB to do the saturating add
• PSHUFB to swizzle the bytes

[penzn]

movups assumes that the value is in static section. That may not work for a Wasm runtime.

[abrown]

@penzn, after we discussed a version to do some shifts to get the 0x707070... value I looked through the Intel manual and was unable to find a shift for byte-width lanes that would work for what we described (but maybe I just didn't see it). Here's a five-instruction version that still uses moves but from immediates and registers:

• MOV 0x70707070_70707070, %eax sets up half of the magic value
• MOVQ %eax, %xmm0 moves into the lower eight bytes of the XMM register
• PSHUFD to copy the lower eight bytes to the upper eight bytes
• PADDUSB to do the saturating add
• PSHUFB

In any case, the "zero out-of-range indices" requirement seems to cause a rather disappointing lowering for x86.

[penzn]

Yeah, you are right, what I had in mind would not work, since there is no shift packed byte instruction...

There are "VPBROADCAST[BWDQ]" instructions in AVX2, which can broadcast a value from general purpose register to all lanes, you can avoid doing en extra shuffle if that is available.

Also, see #70 and #63

[arunetm]

@abrown @jlb6740 swizzle will need multiple instructions using SSE4 (can be implemented as a special case of shuffle).

This is a very useful operation for application developers in expressing matrix operations like multiply, inverse etc. The extra cost of additional instructions for swizzle on IA is anticipated to be amortized away in applications and kernels. It will be very useful if we could verify this assumption using your implementation.

The current instruction set for SIMD is being iterated over and the above data point will help to make the right choices to ensure expressiveness and portable perf for wasm simd applications.

[zeux]

Note that as described in #68 (comment), the out-of-range behavior is substantially different between different architectures; the current spec at the time of the variable-index shuffle proposal was believed to be as optimal as the next best alternative which is to specify that only lower 4 bits affect the results, because that would require masking the extra bits off on x86.

The limitations on loading constants seems unfortunate. With RIP-relative addressing, the codegen for a shuffle with the specification as declared by the spec is, trivially (https://gcc.godbolt.org/z/gY5qE8),

shuffle(long long __vector(2), long long __vector(2)):                      # @shuffle(long long __vector(2), long long __vector(2))
        vpaddusb        xmm1, xmm1, xmmword ptr [rip + .LCPI0_0]
        vpshufb xmm0, xmm0, xmm1
        ret

It seems like any specification of shuffle that doesn't try to be 100% compatible with x64 behavior (which would penalize other platforms) would require some form of vector constant load or materialization which invariably will be expensive.

[penzn]

@zeux that matches @abrown's initial solution above with a load. It would be good to test swizzle's performance. Do you know of any codes to hit this instruction?

[zeux]

@penzn Not sure - I actually wanted to try porting my vertex codec that relies on pshufb to wasm, but the issue is that variable shuffles are currently not exposed in Emscripten's intrinsic header at all, so it's hard to experiment with this.

[tlively]

I expect to get the toolchain up to date with the latest spec proposal this week, so you should be able to experiment soon.

[abrown]

@zeux I've looked at meshoptimizer a bit and I can't see how having swizzle's "zero out-of-range index" behavior would change your code. It seems to me that to the shuffle (https://github.com/abrown/meshoptimizer/blob/284728f0f717ae9eda7f3e1d41dfaaa3423b189b/src/vertexcodec.cpp#L483) you will still need a kDecodeBytesGroupShuffle lookup table for generating the shuffle mask (https://github.com/abrown/meshoptimizer/blob/284728f0f717ae9eda7f3e1d41dfaaa3423b189b/src/vertexcodec.cpp#L418)--regardless of if the top bit is 1 or just greater than 15. Am I correctly reading that code? Or perhaps you would create a different implementation entirely without the lookup table for compiling to Wasm SIMD? (In essence, I'm trying to understand how and why the shuffle/swizzle is used in your decoding so any help is appreciated 😄).