Incorporating CR feedback1

lib/Runtime/Language/InterpreterStackFrame.cpp

@@ -2278,8 +2278,8 @@ namespace Js
     {
 #ifdef ENABLE_WASM
         CompileAssert(type < Wasm::WasmTypes::Limit);
-        const char16* fromType = toJs ? Wasm::WasmTypes::GetStrId(static_cast<Wasm::WasmTypes::WasmType>(type)) : _u("Javascript Variable");
-        const char16* toType = toJs ? _u("Javascript Variable") : Wasm::WasmTypes::GetStrId(static_cast<Wasm::WasmTypes::WasmType>(type));
+        const char16* fromType = toJs ? Wasm::WasmTypes::GetTypeName(static_cast<Wasm::WasmTypes::WasmType>(type)) : _u("Javascript Variable");
+        const char16* toType = toJs ? _u("Javascript Variable") : Wasm::WasmTypes::GetTypeName(static_cast<Wasm::WasmTypes::WasmType>(type));
         JavascriptError::ThrowTypeErrorVar(scriptContext, WASMERR_InvalidTypeConversion, fromType, toType);
 #else
         Assert(UNREACHED); //shouldn't get there

lib/WasmReader/WasmBinaryOpCodes.h

@@ -4,14 +4,14 @@
 //-------------------------------------------------------------------------------------------------------
 
 // define this to include all opcodes
-#ifndef WASM_SIGNATURE
-#define WASM_SIGNATURE(id,...)
-#endif
-
 #ifndef WASM_OPCODE
 #define WASM_OPCODE(opname, opcode, sig, nyi)
 #endif
 
+#ifndef WASM_SIGNATURE
+#define WASM_SIGNATURE(id,...)
+#endif
+
 #ifndef WASM_CTRL_OPCODE
 #define WASM_CTRL_OPCODE(opname, opcode, sig, nyi) WASM_OPCODE(opname, opcode, sig, nyi)
 #endif
@@ -85,8 +85,6 @@ WASM_SIGNATURE(D,       1,   WasmTypes::I64)
 WASM_SIGNATURE(V,       1,   WasmTypes::Void)
 WASM_SIGNATURE(Limit,   1,   WasmTypes::Void)
 
-#include "WasmBinaryOpcodesSimd.h"
-
 // Control flow operators
 WASM_CTRL_OPCODE(Unreachable, 0x00, Limit, false)
 WASM_CTRL_OPCODE(Nop,         0x01, Limit, false)
@@ -322,6 +320,11 @@ WASM_MISC_OPCODE(SimdStart, 0xfd, Limit, false)
 WASM_MISC_OPCODE(Extended, 0x06, Limit, false)
 WASM_MISC_OPCODE(Extended2, 0x07, Limit, false)
 
+//Simd
+#ifdef ENABLE_WASM_SIMD
+#include "WasmBinaryOpcodesSimd.h"
+#endif
+
 #undef WASM_OPCODE
 #undef WASM_SIGNATURE
 #undef WASM_CTRL_OPCODE

lib/WasmReader/WasmBinaryOpcodesSimd.h

@@ -3,19 +3,6 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 
-
-//temporary until 64x2 types are implemented
-#ifndef FOREACH_SIMD_TYPE
-#define FOREACH_SIMD_TYPE(V) \
-    V(M128, F)
-#endif
-
-#ifndef FOREACH_SIMD_TYPE_W_BASE
-#define FOREACH_SIMD_TYPE_W_BASE(V) \
-    V(M128, F)   \
-    V(M128, I)
-#endif
-
 #ifndef WASM_LANE_OPCODE
 #define WASM_LANE_OPCODE(opname, opcode, sig, nyi) WASM_OPCODE(opname, opcode, sig, nyi)
 #endif
@@ -41,24 +28,16 @@
 #endif
 
 //SIMD Signatures
-#define SIMD_EXTRACT(TYPE, BASE) WASM_SIGNATURE(BASE##_##TYPE, 2, WasmTypes::##BASE##32, WasmTypes::##TYPE)
-#define SIMD_BUILD(TYPE, BASE) WASM_SIGNATURE(TYPE##_##BASE, 2, WasmTypes::##TYPE, WasmTypes::##BASE##32)
-
-#define SIMD_ALL(TYPE, BASE) \
-    SIMD_EXTRACT(TYPE, BASE) \
-    SIMD_BUILD(TYPE, BASE)
-
-FOREACH_SIMD_TYPE_W_BASE(SIMD_ALL)
-#undef SIMD_ALL
-#undef SIMD_BUILD
-#undef SIMD_EXTRACT
-
+WASM_SIGNATURE(F_M128, 2, WasmTypes::F32, WasmTypes::M128)
+WASM_SIGNATURE(I_M128, 2, WasmTypes::I32, WasmTypes::M128)
+WASM_SIGNATURE(M128_I, 2, WasmTypes::M128, WasmTypes::I32)
+WASM_SIGNATURE(M128_F, 2, WasmTypes::M128, WasmTypes::F32)
+WASM_SIGNATURE(M128_L, 2, WasmTypes::M128, WasmTypes::I64)
+WASM_SIGNATURE(M128_D, 2, WasmTypes::M128, WasmTypes::F64)
 WASM_SIGNATURE(M128X3, 3, WasmTypes::M128, WasmTypes::M128, WasmTypes::M128)
 WASM_SIGNATURE(M128_M128_I, 3, WasmTypes::M128, WasmTypes::M128, WasmTypes::I32)
 WASM_SIGNATURE(M128_M128, 2, WasmTypes::M128, WasmTypes::M128)
-WASM_SIGNATURE(M128_L, 2, WasmTypes::M128, WasmTypes::I64)
 WASM_SIGNATURE(L_M128, 2, WasmTypes::I64, WasmTypes::M128)
-WASM_SIGNATURE(M128_D, 2, WasmTypes::M128, WasmTypes::F64)
 WASM_SIGNATURE(D_M128_I, 3, WasmTypes::F64, WasmTypes::M128, WasmTypes::I32)
 
 WASM_MISC_OPCODE(M128Const, 0x100, Limit, false)

lib/WasmReader/WasmBinaryReader.cpp

@@ -17,33 +17,12 @@ namespace Wasm
 namespace WasmTypes
 {
 
-const char16* const strIds[Limit] = {
-    nullptr,       //Void = 0,
-    _u("int32"),   //I32 = 1,
-    _u("int64"),   //I64 = 2,
-    _u("float"),   //F32 = 3,
-    _u("double"),  //F64 = 4,
-    _u("simd128")
-};
-
-const char16* GetStrId(WasmType type)
-{
-    Assert(type < WasmType::Limit);
-    return strIds[type];
-}
-
 bool IsLocalType(WasmTypes::WasmType type)
 {
     // Check if type in range ]Void,Limit[
     return (uint32)(type - 1) < (WasmTypes::Limit - 1);
 }
 
-bool IsSIMDType(WasmTypes::WasmType type)
-{
-    uint utype = (uint)(type);
-    return utype > WasmTypes::F64 && utype < WasmTypes::Limit;
-}
-
 uint32 GetTypeByteSize(WasmType type)
 {
     switch (type)
@@ -53,9 +32,7 @@ uint32 GetTypeByteSize(WasmType type)
     case I64: return sizeof(int64);
     case F32: return sizeof(float);
     case F64: return sizeof(double);
-#define SIMD_CASE(TYPE, BASE) case TYPE: 
-FOREACH_SIMD_TYPE(SIMD_CASE)
-#undef SIMD_CASE
+    case M128:
         CompileAssert(sizeof(Simd::simdvec) == 16);
         return sizeof(Simd::simdvec);
     case Ptr: return sizeof(void*);
@@ -713,7 +690,7 @@ void WasmBinaryReader::ConstNode()
         {
             m_currentNode.cnst.v128[i] = ReadConst<uint>();
         }
-        m_funcState.count += sizeof(m_currentNode.cnst.v128);
+        m_funcState.count += Simd::VEC_WIDTH;
         break;
     }
 }

lib/WasmReader/WasmByteCodeGenerator.cpp

@@ -210,8 +210,6 @@ Js::AsmJsVarType WasmToAsmJs::GetAsmJsVarType(WasmTypes::WasmType wasmType)
     case WasmTypes::F32: return Js::AsmJsVarType::Float;
     case WasmTypes::F64: return Js::AsmJsVarType::Double;
     case WasmTypes::M128:  return Js::AsmJsVarType::Float32x4;
-    //case WasmTypes::I2:  return Js::AsmJsVarType::Int64x2; @TODO
-    //case WasmTypes::F2:  return Js::AsmJsVarType::Float2x64;
     default:
         throw WasmCompilationException(_u("Unknown var type %u"), wasmType);
     }
@@ -471,7 +469,7 @@ WasmBytecodeGenerator::WasmBytecodeGenerator(Js::ScriptContext* scriptContext, W
     m_scriptContext(scriptContext),
     m_alloc(_u("WasmBytecodeGen"), scriptContext->GetThreadContext()->GetPageAllocator(), Js::Throw::OutOfMemory),
     m_evalStack(&m_alloc),
-    mTypedRegisterAllocator(&m_alloc, AllocateRegisterSpace, 0),
+    mTypedRegisterAllocator(&m_alloc, AllocateRegisterSpace, CONFIG_FLAG(WasmSimd) ? 0 : 1 << WAsmJs::SIMD),
     m_blockInfos(&m_alloc),
     currentProfileId(0),
     isUnreachable(false)
@@ -620,16 +618,12 @@ EmitInfo WasmBytecodeGenerator::EmitSimdBuildExpr(Js::OpCodeAsmJs op, const Wasm
     const WasmTypes::WasmType resultType = signature[0];
     const WasmTypes::WasmType type = signature[1];
 
-    Js::RegSlot resultReg = GetRegisterSpace(signature[0])->AcquireTmpRegister();
+    Js::RegSlot resultReg = GetRegisterSpace(resultType)->AcquireTmpRegister();
 
-    EmitInfo args[Simd::MAX_LANES];
+    EmitInfo args[lanes];
     for (uint i = 0; i < lanes; i++)
     {
-        args[i] = PopEvalStack();
-        if (type != args[i].type)
-        {
-            throw WasmCompilationException(_u("type mismatch"));
-        }
+        args[i] = PopEvalStack(type);
     }
 
     switch (lanes)
@@ -1168,7 +1162,7 @@ EmitInfo WasmBytecodeGenerator::EmitCall()
             argOp = isImportCall ? Js::OpCodeAsmJs::ArgOut_Long : Js::OpCodeAsmJs::I_ArgOut_Long;
             break;
         case WasmTypes::M128:
-            argOp = Js::OpCodeAsmJs::Simd128_I_ArgOut_F4;
+            argOp = isImportCall ? Js::OpCodeAsmJs::Simd128_ArgOut_F4 : Js::OpCodeAsmJs::Simd128_I_ArgOut_F4;
             break;
         case WasmTypes::Any:
             // In unreachable mode allow any type as argument since we won't actually emit the call
@@ -1308,7 +1302,6 @@ EmitInfo WasmBytecodeGenerator::EmitCall()
     return retInfo;
 }
 
-
 EmitInfo WasmBytecodeGenerator::EmitIfElseExpr()
 {
     Js::ByteCodeLabel falseLabel = m_writer->DefineLabel();
@@ -1445,7 +1438,7 @@ EmitInfo WasmBytecodeGenerator::EmitUnaryExpr(Js::OpCodeAsmJs op, const WasmType
     return EmitInfo(resultReg, resultType);
 }
 
-void WasmBytecodeGenerator::CheckLaneIndex(Js::OpCodeAsmJs op)
+void WasmBytecodeGenerator::CheckLaneIndex(Js::OpCodeAsmJs op, const uint index)
 {
     uint numLanes;
     switch (op) 
@@ -1476,12 +1469,10 @@ void WasmBytecodeGenerator::CheckLaneIndex(Js::OpCodeAsmJs op)
         break;
     default:
         Assert(UNREACHED);
-        numLanes = UINT_MAX;
+        numLanes = 0;
     }
 
-    const uint offset = GetReader()->m_currentNode.lane.index;
-
-    if (offset >= numLanes)
+    if (index >= numLanes)
     {
         throw WasmCompilationException(_u("index is out of range"));
     }
@@ -1489,9 +1480,10 @@ void WasmBytecodeGenerator::CheckLaneIndex(Js::OpCodeAsmJs op)
 
 EmitInfo WasmBytecodeGenerator::EmitLaneIndex(Js::OpCodeAsmJs op)
 {
-    CheckLaneIndex(op);
+    const uint index = GetReader()->m_currentNode.lane.index;
+    CheckLaneIndex(op, index);
     WasmConstLitNode dummy;
-    dummy.i32 = GetReader()->m_currentNode.lane.index;
+    dummy.i32 = index;
     return EmitConst(WasmTypes::I32, dummy);
 }
 
@@ -1500,10 +1492,10 @@ EmitInfo WasmBytecodeGenerator::EmitReplaceLaneExpr(Js::OpCodeAsmJs op, const Wa
     const WasmTypes::WasmType resultType = signature[0];
     const WasmTypes::WasmType valueType = signature[1];
     EmitInfo valueArg = PopEvalStack(valueType, _u("lane argument type mismatch"));
-    
+
     EmitInfo simdArg = PopEvalStack(WasmTypes::M128, _u("simd argument type mismatch"));
     Assert(resultType == WasmTypes::M128);
-   
+
     EmitInfo indexInfo = EmitLaneIndex(op);
     Js::RegSlot resultReg = GetRegisterSpace(resultType)->AcquireTmpRegister();
     EmitInfo result(resultReg, resultType);
@@ -1537,7 +1529,7 @@ EmitInfo WasmBytecodeGenerator::EmitV8X16Shuffle()
     {
         if (indices[i] >= Simd::MAX_LANES * 2)
         {
-            throw WasmCompilationException(_u("%-th shuffle lane index is larger than 31"), i);
+            throw WasmCompilationException(_u("%u-th shuffle lane index is larger than %u"), i, (Simd::MAX_LANES * 2 -1));
         }
     }
 
@@ -1549,7 +1541,7 @@ EmitInfo WasmBytecodeGenerator::EmitExtractLaneExpr(Js::OpCodeAsmJs op, const Wa
 {
     WasmTypes::WasmType resultType = signature[0];
     WasmTypes::WasmType simdArgType = signature[1];
-    
+
     EmitInfo simdArgInfo = PopEvalStack(simdArgType, _u("Argument should be of type M128"));
 
     Js::RegSlot resultReg = GetRegisterSpace(resultType)->AcquireTmpRegister();
@@ -1909,13 +1901,11 @@ WasmRegisterSpace* WasmBytecodeGenerator::GetRegisterSpace(WasmTypes::WasmType t
 #if TARGET_64
     case WasmTypes::Ptr:
 #endif
-    case WasmTypes::I64: return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::INT64);
-    case WasmTypes::F32: return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::FLOAT32);
-    case WasmTypes::F64: return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::FLOAT64);
-#define SIMD_CASE(TYPE, BASE) case WasmTypes::##TYPE:   return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::SIMD);
+    case WasmTypes::I64:    return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::INT64);
+    case WasmTypes::F32:    return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::FLOAT32);
+    case WasmTypes::F64:    return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::FLOAT64);
+    case WasmTypes::M128:   return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::SIMD);
 
-FOREACH_SIMD_TYPE(SIMD_CASE)
-#undef SIMD_CASE
     default:
         return nullptr;
     }

lib/WasmReader/WasmByteCodeGenerator.h

@@ -219,7 +219,7 @@ namespace Wasm
         EmitInfo EmitV8X16Shuffle();
         EmitInfo EmitExtractLaneExpr(Js::OpCodeAsmJs op, const WasmTypes::WasmType* signature);
         EmitInfo EmitReplaceLaneExpr(Js::OpCodeAsmJs op, const WasmTypes::WasmType* signature);
-        void CheckLaneIndex(Js::OpCodeAsmJs op);
+        void CheckLaneIndex(Js::OpCodeAsmJs op, const uint index);
         EmitInfo EmitLaneIndex(Js::OpCodeAsmJs op);
 
         EmitInfo EmitConst(WasmTypes::WasmType type, WasmConstLitNode cnst);

lib/WasmReader/WasmParseTree.h

@@ -32,11 +32,7 @@ namespace Wasm
 
         extern const char16* const strIds[Limit];
 
-        const char16* GetStrId(WasmType type);
         bool IsLocalType(WasmTypes::WasmType type);
-        bool IsSIMDType(WasmTypes::WasmType type);
-
-
         uint32 GetTypeByteSize(WasmType type);
         const char16* GetTypeName(WasmType type);
     }

lib/WasmReader/WasmSignature.cpp

@@ -115,10 +115,7 @@ Js::ArgSlot WasmSignature::GetParamSize(Js::ArgSlot index) const
         CompileAssert(sizeof(double) == sizeof(int64));
         return sizeof(int64);
         break;
-#define SIMD_CASE(TYPE, BASE) case WasmTypes::##TYPE:
-
-FOREACH_SIMD_TYPE(SIMD_CASE)
-#undef SIMD_CASE
+    case WasmTypes::M128:
         CompileAssert(sizeof(Simd::simdvec) == 16);
         return sizeof(Simd::simdvec);
         break;
@@ -148,14 +145,14 @@ void WasmSignature::FinalizeSignature()
 
     // 3 bits for result type, 2 for each arg
     // we don't need to reserve a sentinel bit because there is no result type with value of 7
-    uint32 sigSize = ((uint32)paramCount) * 2 + 3;
+    uint32 sigSize = ((uint32)paramCount) * 3 + 3;
     if (sigSize <= sizeof(m_shortSig) << 3)
     {
         m_shortSig = (m_shortSig << 3) | m_resultType;
         for (Js::ArgSlot i = 0; i < paramCount; ++i)
         {
             // we can use 2 bits per arg by dropping void
-            m_shortSig = (m_shortSig << 2) | (m_params[i] - 1);
+            m_shortSig = (m_shortSig << 3) | (m_params[i] - 1);
         }
     }
 #endif

test/rlexedirs.xml

@@ -329,8 +329,7 @@
 <dir>
   <default>
     <files>wasm.simd</files>
-    <tags>exclude_serialized,exclude_arm,exclude_arm64,require_backend,exclude_xplat</tags>
-    <tags>exclude_arm,exclude_arm64</tags>
+    <tags>require_wasm,exclude_serialized,exclude_arm,exclude_arm64,require_backend,exclude_xplat</tags>
   </default>
 </dir>
 </regress-exe>