[WASM] Simplify SIMD shuffle opcodes.

src/wasm/function-body-decoder.cc

Index: src/wasm/function-body-decoder.cc
diff --git a/src/wasm/function-body-decoder.cc b/src/wasm/function-body-decoder.cc
index 9241c2d448f59aa5cdc9bfde189f2b7b24d558ec..e29f4139675e49f31105da1a2d3f4a9f4a413711 100644
--- a/src/wasm/function-body-decoder.cc
+++ b/src/wasm/function-body-decoder.cc
@@ -146,21 +146,6 @@ struct Control {
   }
 };
 
-namespace {
-inline unsigned GetShuffleMaskSize(WasmOpcode opcode) {
-  switch (opcode) {
-    case kExprS32x4Shuffle:
-      return 4;
-    case kExprS16x8Shuffle:
-      return 8;
-    case kExprS8x16Shuffle:
-      return 16;
-    default:
-      UNREACHABLE();
-  }
-}
-}  // namespace
-
 // Macros that build nodes only if there is a graph and the current SSA
 // environment is reachable from start. This avoids problems with malformed
 // TF graphs when decoding inputs that have unreachable code.
@@ -421,13 +406,12 @@ class WasmDecoder : public Decoder {
     }
   }
 
-  inline bool Validate(const byte* pc, WasmOpcode opcode,
-                       SimdShuffleOperand<true>& operand) {
-    unsigned lanes = GetShuffleMaskSize(opcode);
+  inline bool Validate(const byte* pc, Simd8x16ShuffleOperand<true>& operand) {
     uint8_t max_lane = 0;
-    for (unsigned i = 0; i < lanes; i++)
+    for (unsigned i = 0; i < 16; i++)

[Mircea Trofin, 2017/06/13 22:21:46]

since we're here - could we use "uint32_t" or one of the other uint types, and
not "unsigned" (as per style guide)?

[bbudge, 2017/06/13 23:07:54]

Done.

       max_lane = std::max(max_lane, operand.shuffle[i]);
-    if (operand.lanes != lanes || max_lane > 2 * lanes) {
+    // Shuffle indices must be in [0..31] for a 16 lane shuffle.
+    if (max_lane > 32) {
       error(pc_ + 2, "invalid shuffle mask");
       return false;
     } else {
@@ -520,11 +504,9 @@ class WasmDecoder : public Decoder {
           {
             return 3;
           }
-          // Shuffles contain a byte array to determine the shuffle.
-          case kExprS32x4Shuffle:
-          case kExprS16x8Shuffle:
+          // Shuffles are a 2 byte opcode followed by a 16 byte index array.
           case kExprS8x16Shuffle:
-            return 2 + GetShuffleMaskSize(opcode);
+            return 18;

[Mircea Trofin, 2017/06/13 22:21:46]

nit: The old code was a bit more readable, I think. 

[bbudge, 2017/06/13 23:07:54]

Changed to 2 + kSimd128Size.

           default:
             decoder->error(pc, "invalid SIMD opcode");
             return 2;
@@ -1558,17 +1540,16 @@ class WasmFullDecoder : public WasmDecoder {
     return operand.length;
   }
 
-  unsigned SimdShuffleOp(WasmOpcode opcode) {
-    SimdShuffleOperand<true> operand(this, pc_, GetShuffleMaskSize(opcode));
-    if (Validate(pc_, opcode, operand)) {
+  unsigned Simd8x16ShuffleOp() {
+    Simd8x16ShuffleOperand<true> operand(this, pc_);
+    if (Validate(pc_, operand)) {
       compiler::NodeVector inputs(2, zone_);
       inputs[1] = Pop(1, ValueType::kSimd128).node;
       inputs[0] = Pop(0, ValueType::kSimd128).node;
-      TFNode* node =
-          BUILD(SimdShuffleOp, operand.shuffle, operand.lanes, inputs);
+      TFNode* node = BUILD(Simd8x16ShuffleOp, operand.shuffle, inputs);
       Push(ValueType::kSimd128, node);
     }
-    return operand.lanes;
+    return 16;
   }
 
   unsigned DecodeSimdOpcode(WasmOpcode opcode) {
@@ -1606,10 +1587,8 @@ class WasmFullDecoder : public WasmDecoder {
         len = SimdShiftOp(opcode);
         break;
       }
-      case kExprS32x4Shuffle:
-      case kExprS16x8Shuffle:
       case kExprS8x16Shuffle: {
-        len = SimdShuffleOp(opcode);
+        len = Simd8x16ShuffleOp();
         break;
       }
       default: {