[wasm] Implement remaining SIMD x64 compare ops, unops.

src/compiler/x64/code-generator-x64.cc

Index: src/compiler/x64/code-generator-x64.cc
diff --git a/src/compiler/x64/code-generator-x64.cc b/src/compiler/x64/code-generator-x64.cc
index 2872aeac194dfb5fb3269751cc526d58fdf2d1bd..4b961f53dc7987a0560da616ad610d2c9e580119 100644
--- a/src/compiler/x64/code-generator-x64.cc
+++ b/src/compiler/x64/code-generator-x64.cc
@@ -2186,6 +2186,19 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       }
       break;
     }
+    case kX64I32x4Neg: {
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(0);
+      if (dst.is(src)) {
+        __ movaps(kScratchDoubleReg, dst);
+        __ pxor(dst, dst);
+        __ psubd(dst, kScratchDoubleReg);
+      } else {
+        __ pxor(dst, dst);
+        __ psubd(dst, src);
+      }
+      break;
+    }
     case kX64I32x4Shl: {
       __ pslld(i.OutputSimd128Register(), i.InputInt8(1));
       break;
@@ -2232,6 +2245,18 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       __ pxor(i.OutputSimd128Register(), kScratchDoubleReg);
       break;
     }
+    case kX64I32x4GtS: {
+      __ pcmpgtd(i.OutputSimd128Register(), i.InputSimd128Register(1));
+      break;
+    }
+    case kX64I32x4GeS: {
+      XMMRegister dst = i.OutputSimd128Register();
+      __ movaps(kScratchDoubleReg, i.InputSimd128Register(1));

[bbudge, 2017/06/22 17:30:10]

Can you use pminsd here to save 1 instruction?

[gdeepti, 2017/06/27 20:47:42]

Done.

+      __ pcmpgtd(kScratchDoubleReg, dst);
+      __ pcmpeqd(dst, dst);
+      __ pxor(dst, kScratchDoubleReg);
+      break;
+    }
     case kX64I32x4ShrU: {
       __ psrld(i.OutputSimd128Register(), i.InputInt8(1));
       break;
@@ -2246,6 +2271,27 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       __ pmaxud(i.OutputSimd128Register(), i.InputSimd128Register(1));
       break;
     }
+    case kX64I32x4GtU: {
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(1);
+      __ Set(kScratchRegister, 0x80000000);
+      __ movd(kScratchDoubleReg, kScratchRegister);
+      __ pshufd(kScratchDoubleReg, kScratchDoubleReg, 0x0);
+      __ pxor(src, kScratchDoubleReg);
+      __ pxor(dst, kScratchDoubleReg);
+      __ pcmpgtd(dst, src);
+      // Reset input to not clobber.
+      __ pxor(src, kScratchDoubleReg);

[gdeepti, 2017/06/21 03:44:01]

This is similar to the code Clang generates for unsigned compares, but is worse
than it should be, because we do not have S128Consts. Given that, would it be
better to implement this using pminu(d/w/b) instructions as for I32x4GeU?

[bbudge, 2017/06/22 16:29:49]

I think we'll have to support S128 constants at some point. They're in the spec
proposal, and may be helpful in other contexts too, e.g. irregular shuffle masks
on ARM and MIPS.

These constants could be created by setting all 1's, then left shift by 31. It
might be slightly faster.

However, it does seem simpler to use pmaxud (SSE 4.1?) here: a > b => max(a, b)
== a
movd(scratch, dst)
pmaxud(scratch, src)
pcmpeqd(dst, scratch)

Similarly, pmaxub, pmaxus for other formats.

[gdeepti, 2017/06/22 17:26:48]

I like the simplicity of the pminud/pmaxud approach to implement the GeU/GtU
operations, I added both approaches to poll feedback, as gcc/clang both generate
code in terms of pxor/psubd, but for our use case pmaxu/pminu(d/w/b) seems to be
better, or at least that till we have S128Constants. I'll use pmaxud/pminud and 
add a TODO to reconsider when we have S128Constants. 

[zvi, 2017/06/24 23:04:09]

I agree that the pmax/pmin option is preferable for 128-bit or larger operands
(pmin/pmax MMX throughput was reduced for mmx-types in recent processor
generations so it is better to avoid these).

Here is a comparison of IACA reports for two alternatives in the 'tie-braker'
case. The throughput is the same, but you save on code size and one micro-op
less:

Block Throughput: 2.00 Cycles
 | Num Of |                    Ports pressure in cycles                     |   
|
 |  Uops  |  0  - DV  |  1  |  2  -  D  |  3  -  D  |  4  |  5  |  6  |  7  |   
|

---------------------------------------------------------------------------------
 |   1    |           | 0.3 |           |           |     | 0.6 |     |     | CP
| vpcmpgtd xmm0, xmm1, xmm0
 |   1    |           | 0.7 |           |           |     | 0.3 |     |     |   
| vpcmpeqd xmm1, xmm1, xmm1
 |   1    | 0.8       | 0.1 |           |           |     | 0.2 |     |     | CP
| vpxor xmm0, xmm0, xmm1
 Total Num Of Uops: 3



Block Throughput: 2.00 Cycles
 | Num Of |                    Ports pressure in cycles                     |   
|
 |  Uops  |  0  - DV  |  1  |  2  -  D  |  3  -  D  |  4  |  5  |  6  |  7  |   
|

---------------------------------------------------------------------------------
 |   1    |           | 1.0 |           |           |     |     |     |     | CP
| vpmaxsd xmm2, xmm0, xmm1
 |   1    |           |     |           |           |     | 1.0 |     |     | CP
| vpcmpeqd xmm0, xmm0, xmm2
 Total Num Of Uops: 2

[gdeepti, 2017/06/27 20:47:42]

Bill: 
This returns true for the == case as well, using pmax + complement, does this
look reasonable?
Zvi: 

Thanks for the detailed comparison. Reasons for using MMX instructions here:
The v8 assembler does not fully support the AVX instruction set - my assumption
here is that there is a possibility of encountering AVX/SSE transition penalties
if AVX instructions are used incosistently in the code generator. Is this
assumption correct? 

In the future when we have full assembler support for AVX instructions, the
macroassembler should be able to chose at runtime based on CPU support. 

[zvi, 2017/06/27 22:30:32]

I didn't mean to say you are selecting MMX instructions here - it appears to be
SSE which is fine. MMX is the older first generation SIMD instruction set that
used 64-bit registers. IIUC, the code under discussion here is targeting SSE
which shares the same execution resources as the 128-bit AVX instructions, so in
that respect SSE is not inferior. I'm sorry if my side-note about MMX mislead
you.

You are right that SSE and AVX code should not be mixed and in some cases you
can hit high penalties if you do not clear the upper 128-bits of the YMM
registers upon transition from AVX to SSE.

One thing i was wondering about:
AVX introduces non-destructive sources which will make the 'if (dst.is(src))'
checks unnecessary because with AVX the binary operations will not share an
operand for source and dest. Of course the register allocator is free to assign
the same register to source and dest if source is killed. So will there need to
be a redesign of these interfaces for AVX?

[gdeepti, 2017/06/27 23:23:37]

Sorry for the confusion, I remembered stale state where we were using MMX
encodings, that is not the case anymore, we are using SSE encodings now. 
Thanks for clarifying! 
We will have to take a look at these again for AVX, but it should hopefully be
straightforward to fix. Currently we have an optimization for all SIMD binops in
the instruction-selector that handles destructive sources, that will need to be
disabled or handled per operation. 

+      break;
+    }
+    case kX64I32x4GeU: {
+      CpuFeatureScope sse_scope(masm(), SSE4_1);
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(1);
+      __ pminud(dst, src);
+      __ pcmpeqd(dst, src);
+      break;
+    }
     case kX64S128Zero: {
       XMMRegister dst = i.OutputSimd128Register();
       __ xorps(dst, dst);
@@ -2276,6 +2322,19 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       }
       break;
     }
+    case kX64I16x8Neg: {
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(0);
+      if (dst.is(src)) {
+        __ movaps(kScratchDoubleReg, dst);
+        __ pxor(dst, dst);
+        __ psubw(dst, kScratchDoubleReg);
+      } else {
+        __ pxor(dst, dst);
+        __ psubw(dst, src);
+      }
+      break;
+    }
     case kX64I16x8Shl: {
       __ psllw(i.OutputSimd128Register(), i.InputInt8(1));
       break;
@@ -2330,6 +2389,18 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       __ pxor(i.OutputSimd128Register(), kScratchDoubleReg);
       break;
     }
+    case kX64I16x8GtS: {
+      __ pcmpgtw(i.OutputSimd128Register(), i.InputSimd128Register(1));
+      break;
+    }
+    case kX64I16x8GeS: {
+      XMMRegister dst = i.OutputSimd128Register();
+      __ movaps(kScratchDoubleReg, i.InputSimd128Register(1));
+      __ pcmpgtw(kScratchDoubleReg, dst);
+      __ pcmpeqd(dst, dst);
+      __ pxor(dst, kScratchDoubleReg);
+      break;
+    }
     case kX64I16x8ShrU: {
       __ psrlw(i.OutputSimd128Register(), i.InputInt8(1));
       break;
@@ -2352,6 +2423,27 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       __ pmaxuw(i.OutputSimd128Register(), i.InputSimd128Register(1));
       break;
     }
+    case kX64I16x8GtU: {
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(1);
+      __ Set(kScratchRegister, 0x80008000);
+      __ movd(kScratchDoubleReg, kScratchRegister);
+      __ pshufd(kScratchDoubleReg, kScratchDoubleReg, 0x0);
+      __ pxor(src, kScratchDoubleReg);
+      __ pxor(dst, kScratchDoubleReg);
+      __ pcmpgtw(dst, src);
+      // Reset input to not clobber.
+      __ pxor(src, kScratchDoubleReg);
+      break;
+    }
+    case kX64I16x8GeU: {
+      CpuFeatureScope sse_scope(masm(), SSE4_1);
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(1);
+      __ pminuw(dst, src);
+      __ pcmpeqw(dst, src);
+      break;
+    }
     case kX64I8x16Splat: {
       CpuFeatureScope sse_scope(masm(), SSSE3);
       XMMRegister dst = i.OutputSimd128Register();
@@ -2377,6 +2469,19 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       }
       break;
     }
+    case kX64I8x16Neg: {
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(0);
+      if (dst.is(src)) {
+        __ movaps(kScratchDoubleReg, dst);
+        __ pxor(dst, dst);
+        __ psubb(dst, kScratchDoubleReg);
+      } else {
+        __ pxor(dst, dst);
+        __ psubb(dst, src);
+      }
+      break;
+    }
     case kX64I8x16Add: {
       __ paddb(i.OutputSimd128Register(), i.InputSimd128Register(1));
       break;
@@ -2413,6 +2518,18 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       __ pxor(i.OutputSimd128Register(), kScratchDoubleReg);
       break;
     }
+    case kX64I8x16GtS: {
+      __ pcmpgtb(i.OutputSimd128Register(), i.InputSimd128Register(1));
+      break;
+    }
+    case kX64I8x16GeS: {
+      XMMRegister dst = i.OutputSimd128Register();
+      __ movaps(kScratchDoubleReg, i.InputSimd128Register(1));
+      __ pcmpgtb(kScratchDoubleReg, dst);
+      __ pcmpeqd(dst, dst);
+      __ pxor(dst, kScratchDoubleReg);
+      break;
+    }
     case kX64I8x16AddSaturateU: {
       __ paddusb(i.OutputSimd128Register(), i.InputSimd128Register(1));
       break;
@@ -2431,6 +2548,27 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       __ pmaxub(i.OutputSimd128Register(), i.InputSimd128Register(1));
       break;
     }
+    case kX64I8x16GtU: {
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(1);
+      __ Set(kScratchRegister, 0x80808080);
+      __ movd(kScratchDoubleReg, kScratchRegister);
+      __ pshufd(kScratchDoubleReg, kScratchDoubleReg, 0x0);
+      __ pxor(src, kScratchDoubleReg);
+      __ pxor(dst, kScratchDoubleReg);
+      __ pcmpgtb(dst, src);
+      // Reset input to not clobber.
+      __ pxor(src, kScratchDoubleReg);
+      break;
+    }
+    case kX64I8x16GeU: {
+      CpuFeatureScope sse_scope(masm(), SSE4_1);
+      XMMRegister dst = i.OutputSimd128Register();
+      XMMRegister src = i.InputSimd128Register(1);
+      __ pminub(dst, src);
+      __ pcmpeqb(dst, src);
+      break;
+    }
     case kX64S128And: {
       __ pand(i.OutputSimd128Register(), i.InputSimd128Register(1));
       break;
@@ -2445,8 +2583,16 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
     }
     case kX64S128Not: {
       XMMRegister dst = i.OutputSimd128Register();
-      __ pcmpeqd(dst, dst);
-      __ pxor(dst, i.InputSimd128Register(1));
+      XMMRegister src = i.InputSimd128Register(0);
+      if (dst.is(src)) {
+        __ movaps(kScratchDoubleReg, dst);
+        __ pcmpeqd(dst, dst);
+        __ pxor(dst, kScratchDoubleReg);
+      } else {
+        __ pcmpeqd(dst, dst);
+        __ pxor(dst, src);
+      }
+
       break;
     }
     case kX64S128Select: {