Skip to content

Commit 2dc8c3a

Browse files
CelluleCellule
committed
[MERGE #5245 @Cellule] WASM: NYI M128 globals + case type strenghtening
Merge pull request #5245 from Cellule:wasm/cases Make it explicit that we don't support Wasm m128 globals at this time. Strengthen switch-cases with WasmTypes to make sure we always handled the main types. This will cause compile errors when we add new types to make sure we correctly handled them everywhere
2 parents 0e56d2d + 3e64774 commit 2dc8c3a

File tree

10 files changed

+131
-25
lines changed

10 files changed

+131
-25
lines changed

‎lib/Common/Core/Assertions.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ extern int IsInAssert;
9595
#define CompileAssert(e) static_assert(e, #e)
9696
#endif
9797

98+
#ifndef CompileAssertMsg
99+
#define CompileAssertMsg(e, msg) static_assert(e, msg)
100+
#endif
101+
98102
// We set IsPointer<T>::IsTrue to true if T is a pointer type
99103
// Otherwise, it's set to false
100104
template <class T>

‎lib/Runtime/Library/WebAssemblyEnvironment.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,7 @@ void WebAssemblyEnvironment::SetGlobalInternal(uint32 offset, T val)
150150

151151
Wasm::WasmConstLitNode WebAssemblyEnvironment::GetGlobalValue(Wasm::WasmGlobal* global) const
152152
{
153-
if (!global)
154-
{
155-
Js::Throw::InternalError();
156-
}
153+
AssertOrFailFast(global);
157154
Wasm::WasmConstLitNode cnst;
158155
uint32 offset = module->GetOffsetForGlobal(global);
159156

@@ -163,18 +160,18 @@ Wasm::WasmConstLitNode WebAssemblyEnvironment::GetGlobalValue(Wasm::WasmGlobal*
163160
case Wasm::WasmTypes::I64: cnst.i64 = GetGlobalInternal<int64>(offset); break;
164161
case Wasm::WasmTypes::F32: cnst.f32 = GetGlobalInternal<float>(offset); break;
165162
case Wasm::WasmTypes::F64: cnst.f64 = GetGlobalInternal<double>(offset); break;
163+
#ifdef ENABLE_WASM_SIMD
164+
case Wasm::WasmTypes::M128: AssertOrFailFastMsg(UNREACHED, "Wasm.Simd globals not supported");
165+
#endif
166166
default:
167-
Js::Throw::InternalError();
167+
Wasm::WasmTypes::CompileAssertCases<Wasm::WasmTypes::I32, Wasm::WasmTypes::I64, Wasm::WasmTypes::F32, Wasm::WasmTypes::F64, WASM_M128_CHECK_TYPE>();
168168
}
169169
return cnst;
170170
}
171171

172172
void WebAssemblyEnvironment::SetGlobalValue(Wasm::WasmGlobal* global, Wasm::WasmConstLitNode cnst)
173173
{
174-
if (!global)
175-
{
176-
Js::Throw::InternalError();
177-
}
174+
AssertOrFailFast(global);
178175
uint32 offset = module->GetOffsetForGlobal(global);
179176

180177
switch (global->GetType())
@@ -183,8 +180,11 @@ void WebAssemblyEnvironment::SetGlobalValue(Wasm::WasmGlobal* global, Wasm::Wasm
183180
case Wasm::WasmTypes::I64: SetGlobalInternal<int64>(offset, cnst.i64); break;
184181
case Wasm::WasmTypes::F32: SetGlobalInternal<float>(offset, cnst.f32); break;
185182
case Wasm::WasmTypes::F64: SetGlobalInternal<double>(offset, cnst.f64); break;
183+
#ifdef ENABLE_WASM_SIMD
184+
case Wasm::WasmTypes::M128: AssertOrFailFastMsg(UNREACHED, "Wasm.Simd globals not supported");
185+
#endif
186186
default:
187-
Js::Throw::InternalError();
187+
Wasm::WasmTypes::CompileAssertCases<Wasm::WasmTypes::I32, Wasm::WasmTypes::I64, Wasm::WasmTypes::F32, Wasm::WasmTypes::F64, WASM_M128_CHECK_TYPE>();
188188
}
189189
}
190190

‎lib/Runtime/Library/WebAssemblyInstance.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -276,17 +276,20 @@ Var WebAssemblyInstance::CreateExportObject(WebAssemblyModule * wasmModule, Scri
276276
case Wasm::WasmTypes::I32:
277277
obj = JavascriptNumber::ToVar(cnst.i32, scriptContext);
278278
break;
279+
case Wasm::WasmTypes::I64:
280+
JavascriptError::ThrowTypeErrorVar(wasmModule->GetScriptContext(), WASMERR_InvalidTypeConversion, _u("i64"), _u("Var"));
279281
case Wasm::WasmTypes::F32:
280282
obj = JavascriptNumber::New(cnst.f32, scriptContext);
281283
break;
282284
case Wasm::WasmTypes::F64:
283285
obj = JavascriptNumber::New(cnst.f64, scriptContext);
284286
break;
285-
case Wasm::WasmTypes::I64:
286-
JavascriptError::ThrowTypeError(wasmModule->GetScriptContext(), WASMERR_InvalidTypeConversion);
287+
#ifdef ENABLE_WASM_SIMD
288+
case Wasm::WasmTypes::M128:
289+
JavascriptError::ThrowTypeErrorVar(wasmModule->GetScriptContext(), WASMERR_InvalidTypeConversion, _u("m128"), _u("Var"));
290+
#endif
287291
default:
288-
Assert(UNREACHED);
289-
break;
292+
Wasm::WasmTypes::CompileAssertCases<Wasm::WasmTypes::I32, Wasm::WasmTypes::I64, Wasm::WasmTypes::F32, Wasm::WasmTypes::F64, WASM_M128_CHECK_TYPE>();
290293
}
291294
}
292295
JavascriptOperators::OP_SetProperty(exportsNamespace, propertyRecord->GetPropertyId(), obj, scriptContext);
@@ -416,9 +419,12 @@ void WebAssemblyInstance::LoadImports(
416419
case Wasm::WasmTypes::I32: cnst.i32 = JavascriptConversion::ToInt32(prop, ctx); break;
417420
case Wasm::WasmTypes::F32: cnst.f32 = (float)JavascriptConversion::ToNumber(prop, ctx); break;
418421
case Wasm::WasmTypes::F64: cnst.f64 = JavascriptConversion::ToNumber(prop, ctx); break;
419-
case Wasm::WasmTypes::I64: Js::JavascriptError::ThrowTypeError(ctx, WASMERR_InvalidTypeConversion);
422+
case Wasm::WasmTypes::I64: Js::JavascriptError::ThrowTypeErrorVar(ctx, WASMERR_InvalidTypeConversion, _u("Var"), _u("i64"));
423+
#ifdef ENABLE_WASM_SIMD
424+
case Wasm::WasmTypes::M128: Js::JavascriptError::ThrowTypeErrorVar(ctx, WASMERR_InvalidTypeConversion, _u("Var"), _u("m128"));
425+
#endif
420426
default:
421-
Js::Throw::InternalError();
427+
Wasm::WasmTypes::CompileAssertCases<Wasm::WasmTypes::I32, Wasm::WasmTypes::I64, Wasm::WasmTypes::F32, Wasm::WasmTypes::F64, WASM_M128_CHECK_TYPE>();
422428
}
423429
env->SetGlobalValue(global, cnst);
424430
break;

‎lib/Runtime/Library/WebAssemblyModule.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,12 @@ WebAssemblyModule::AttachCustomInOutTracingReader(Wasm::WasmFunctionInfo* func,
519519
case Wasm::Local::I64: node.op = Wasm::wbPrintI64; break;
520520
case Wasm::Local::F32: node.op = Wasm::wbPrintF32; break;
521521
case Wasm::Local::F64: node.op = Wasm::wbPrintF64; break;
522+
#ifdef ENABLE_WASM_SIMD
523+
// todo:: Add support to print m128 argument values
524+
case Wasm::WasmTypes::M128: continue;
525+
#endif
522526
default:
527+
Wasm::WasmTypes::CompileAssertCasesNoFailFast<Wasm::WasmTypes::I32, Wasm::WasmTypes::I64, Wasm::WasmTypes::F32, Wasm::WasmTypes::F64, WASM_M128_CHECK_TYPE>();
523528
throw Wasm::WasmCompilationException(_u("Unknown param type"));
524529
}
525530
customReader->AddNode(node);
@@ -559,11 +564,19 @@ WebAssemblyModule::AttachCustomInOutTracingReader(Wasm::WasmFunctionInfo* func,
559564
case Wasm::WasmTypes::I64: node.op = Wasm::wbPrintI64; break;
560565
case Wasm::WasmTypes::F32: node.op = Wasm::wbPrintF32; break;
561566
case Wasm::WasmTypes::F64: node.op = Wasm::wbPrintF64; break;
567+
#ifdef ENABLE_WASM_SIMD
568+
// todo:: Add support to print m128 return values
569+
case Wasm::WasmTypes::M128: goto SkipReturnPrint;
570+
#endif
562571
default:
572+
Wasm::WasmTypes::CompileAssertCasesNoFailFast<Wasm::WasmTypes::I32, Wasm::WasmTypes::I64, Wasm::WasmTypes::F32, Wasm::WasmTypes::F64, WASM_M128_CHECK_TYPE>();
563573
throw Wasm::WasmCompilationException(_u("Unknown return type"));
564574
}
565575
customReader->AddNode(node);
566576
}
577+
#ifdef ENABLE_WASM_SIMD
578+
SkipReturnPrint:
579+
#endif
567580
endNode.op = Wasm::wbPrintNewLine;
568581
customReader->AddNode(endNode);
569582

‎lib/WasmReader/WasmBinaryReader.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,8 @@ void WasmBinaryReader::ConstNode()
692692
m_funcState.count += Simd::VEC_WIDTH;
693693
break;
694694
#endif
695+
default:
696+
WasmTypes::CompileAssertCases<Wasm::WasmTypes::I32, Wasm::WasmTypes::I64, Wasm::WasmTypes::F32, Wasm::WasmTypes::F64, WASM_M128_CHECK_TYPE>();
695697
}
696698
}
697699

@@ -1065,6 +1067,24 @@ void WasmBinaryReader::ReadGlobalSection()
10651067
for (uint32 i = 0; i < numGlobals; ++i)
10661068
{
10671069
WasmTypes::WasmType type = ReadWasmType(len);
1070+
1071+
if (!WasmTypes::IsLocalType(type))
1072+
{
1073+
ThrowDecodingError(_u("Invalid global type %u"), type);
1074+
}
1075+
switch (type)
1076+
{
1077+
case WasmTypes::I32: break; // Handled
1078+
case WasmTypes::I64: break; // Handled
1079+
case WasmTypes::F32: break; // Handled
1080+
case WasmTypes::F64: break; // Handled
1081+
#ifdef ENABLE_WASM_SIMD
1082+
case WasmTypes::M128: ThrowDecodingError(_u("m128 globals not supported"));
1083+
#endif
1084+
default:
1085+
WasmTypes::CompileAssertCases<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
1086+
}
1087+
10681088
bool isMutable = ReadMutableValue();
10691089
WasmNode globalNode = ReadInitExpr();
10701090
GlobalReferenceTypes::Type refType = GlobalReferenceTypes::Const;

‎lib/WasmReader/WasmByteCodeGenerator.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ Js::AsmJsRetType WasmToAsmJs::GetAsmJsReturnType(WasmTypes::WasmType wasmType)
208208
return Js::AsmJsRetType::Float32x4;
209209
#endif
210210
default:
211+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
211212
throw WasmCompilationException(_u("Unknown return type %u"), wasmType);
212213
}
213214
}
@@ -228,6 +229,7 @@ Js::AsmJsVarType WasmToAsmJs::GetAsmJsVarType(WasmTypes::WasmType wasmType)
228229
return Js::AsmJsVarType::Float32x4;
229230
#endif
230231
default:
232+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
231233
throw WasmCompilationException(_u("Unknown var type %u"), wasmType);
232234
}
233235
}
@@ -607,10 +609,6 @@ void WasmBytecodeGenerator::EnregisterLocals()
607609
{
608610
WasmTypes::WasmType type = m_funcInfo->GetLocal(i);
609611
WasmRegisterSpace* regSpace = GetRegisterSpace(type);
610-
if (regSpace == nullptr)
611-
{
612-
throw WasmCompilationException(_u("Unable to find local register space"));
613-
}
614612
m_locals[i] = WasmLocal(regSpace->AcquireRegister(), type);
615613

616614
// Zero only the locals not corresponding to formal parameters.
@@ -1006,6 +1004,7 @@ void WasmBytecodeGenerator::EmitLoadConst(EmitInfo dst, WasmConstLitNode cnst)
10061004
}
10071005
#endif
10081006
default:
1007+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
10091008
throw WasmCompilationException(_u("Unknown type %u"), dst.type);
10101009
}
10111010
}
@@ -1229,6 +1228,7 @@ PolymorphicEmitInfo WasmBytecodeGenerator::EmitCall()
12291228
}
12301229
// Fall through
12311230
default:
1231+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
12321232
throw WasmCompilationException(_u("Unknown argument type %u"), info.type);
12331233
}
12341234

@@ -1318,7 +1318,12 @@ PolymorphicEmitInfo WasmBytecodeGenerator::EmitCall()
13181318
case WasmTypes::I64:
13191319
convertOp = Js::OpCodeAsmJs::Conv_VTL;
13201320
break;
1321+
#ifdef ENABLE_WASM_SIMD
1322+
case WasmTypes::M128:
1323+
throw WasmCompilationException(_u("Return type: m128 not supported in import calls"));
1324+
#endif
13211325
default:
1326+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
13221327
throw WasmCompilationException(_u("Unknown call return type %u"), singleResType);
13231328
}
13241329
Js::RegSlot location = GetRegisterSpace(singleResType)->AcquireTmpRegister();
@@ -1832,6 +1837,7 @@ Js::OpCodeAsmJs WasmBytecodeGenerator::GetLoadOp(WasmTypes::WasmType wasmType)
18321837
return Js::OpCodeAsmJs::Ld_Int;
18331838
}
18341839
default:
1840+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
18351841
throw WasmCompilationException(_u("Unknown load operator %u"), wasmType);
18361842
}
18371843
}
@@ -1867,6 +1873,7 @@ Js::OpCodeAsmJs WasmBytecodeGenerator::GetReturnOp(WasmTypes::WasmType type)
18671873
return Js::OpCodeAsmJs::Return_Int;
18681874
}
18691875
default:
1876+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
18701877
throw WasmCompilationException(_u("Unknown return type %u"), type);
18711878
}
18721879
return retOp;
@@ -2052,7 +2059,8 @@ WasmRegisterSpace* WasmBytecodeGenerator::GetRegisterSpace(WasmTypes::WasmType t
20522059
return mTypedRegisterAllocator.GetRegisterSpace(WAsmJs::SIMD);
20532060
#endif
20542061
default:
2055-
return nullptr;
2062+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
2063+
throw WasmCompilationException(_u("Unknown type %u"), type);
20562064
}
20572065
}
20582066

‎lib/WasmReader/WasmParseTree.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ bool IsLocalType(WasmTypes::WasmType type)
8686
return false;
8787
}
8888
#endif
89-
return type > WasmTypes::Void && type < WasmTypes::Limit;
89+
return type >= WasmTypes::FirstLocalType && type < WasmTypes::Limit;
9090
}
9191

9292
uint32 GetTypeByteSize(WasmType type)

‎lib/WasmReader/WasmParseTree.h

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,63 @@ namespace Wasm
4646
#endif
4747
Limit,
4848
Ptr,
49-
Any
49+
Any,
50+
51+
FirstLocalType = I32,
52+
AllLocalTypes =
53+
1 << I32
54+
| 1 << I64
55+
| 1 << F32
56+
| 1 << F64
57+
#ifdef ENABLE_WASM_SIMD
58+
| 1 << M128
59+
#endif
5060
};
5161

62+
namespace SwitchCaseChecks
63+
{
64+
template<WasmType... T>
65+
struct bv;
66+
67+
template<>
68+
struct bv<>
69+
{
70+
static constexpr uint value = 0;
71+
};
72+
73+
template<WasmType... K>
74+
struct bv<Limit, K...>
75+
{
76+
static constexpr uint value = bv<K...>::value;
77+
};
78+
79+
template<WasmType K1, WasmType... K>
80+
struct bv<K1, K...>
81+
{
82+
static constexpr uint value = (1 << K1) | bv<K...>::value;
83+
};
84+
}
85+
86+
#ifdef ENABLE_WASM_SIMD
87+
#define WASM_M128_CHECK_TYPE Wasm::WasmTypes::M128
88+
#else
89+
#define WASM_M128_CHECK_TYPE Wasm::WasmTypes::Limit
90+
#endif
91+
92+
template<WasmType... T>
93+
__declspec(noreturn) void CompileAssertCases()
94+
{
95+
CompileAssertMsg(SwitchCaseChecks::bv<T...>::value == AllLocalTypes, "WasmTypes missing in switch-case");
96+
AssertOrFailFastMsg(UNREACHED, "The WasmType case should have been handled");
97+
}
98+
99+
template<WasmType... T>
100+
void CompileAssertCasesNoFailFast()
101+
{
102+
CompileAssertMsg(SwitchCaseChecks::bv<T...>::value == AllLocalTypes, "WasmTypes missing in switch-case");
103+
AssertMsg(UNREACHED, "The WasmType case should have been handled");
104+
}
105+
52106
extern const char16* const strIds[Limit];
53107

54108
bool IsLocalType(WasmTypes::WasmType type);

‎lib/WasmReader/WasmSignature.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ Js::ArgSlot WasmSignature::GetParamSize(Js::ArgSlot index) const
155155
break;
156156
#endif
157157
default:
158+
WasmTypes::CompileAssertCasesNoFailFast<WasmTypes::I32, WasmTypes::I64, WasmTypes::F32, WasmTypes::F64, WASM_M128_CHECK_TYPE>();
158159
throw WasmCompilationException(_u("Invalid param type"));
159160
}
160161
}

‎test/wasm/baselines/global.baseline

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,6 @@ import: 78.145
5858
impInit: 78.145
5959

6060
Invalid cases
61-
Should be invalid type conversion: Invalid WebAssembly type conversion
62-
Should be invalid type conversion: Invalid WebAssembly type conversion
61+
Should be invalid type conversion: Invalid WebAssembly type conversion Var to i64
62+
Should be invalid type conversion: Invalid WebAssembly type conversion i64 to Var
6363
Should be invalid init expr: initializer expression can only use imported globals

0 commit comments

Comments
 (0)