Enable v128 in the interpreter

    Fix (?) the create scalar intrinsics in the jiterpreter
    Fix trimming removing SIMD even if SIMD was enabled
    Add wasm opcode mapping for extract msb
    Fix some SN_ lists in transform-simd being ordered incorrectly
    Due to mixed mode it is not possible to control interp SIMD with a runtime option, so remove the options
    Disable linker substitutions for SIMD unless AOT is active, so the interp is responsible for the properties
    Generate a MINT_NIY for unimplemented PackedSimd methods

Author: kg

eng/testing/tests.wasm.targets

@@ -61,7 +61,7 @@
       So, set those parameters explicitly here.
       -->
     <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MonoProjectRoot)\wasm\build\ILLink.Substitutions.WasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
-    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true' or '$(RunAOTCompilation)' != 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MonoProjectRoot)\wasm\build\ILLink.Substitutions.NoWasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MonoProjectRoot)\wasm\build\ILLink.Substitutions.NoWasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
   </PropertyGroup>
 
   <ItemGroup>

src/mono/mono/mini/interp/interp.c

@@ -3802,6 +3802,10 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
 			memset (locals + ip [1], 0, ip [2]);
 			ip += 3;
 			MINT_IN_BREAK;
+		MINT_IN_CASE(MINT_NIY)
+			g_printf ("MONO interpreter: NIY encountered in method %s\n", frame->imethod->method->name);
+			g_assert_not_reached ();
+			MINT_IN_BREAK;
 		MINT_IN_CASE(MINT_BREAK)
 			++ip;
 			SAVE_INTERP_STATE (frame);

src/mono/mono/mini/interp/mintops.def

@@ -26,6 +26,7 @@
 #define IROPDEF(opsymbol, opstring, oplength, num_dregs, num_sregs, optype) OPDEF(opsymbol, opstring, oplength, num_dregs, num_sregs, optype)
 #endif // IROPDEF
 
+OPDEF(MINT_NIY, "niy", 1, 0, 0, MintOpNoArgs)
 OPDEF(MINT_BREAK, "break", 1, 0, 0, MintOpNoArgs)
 OPDEF(MINT_BREAKPOINT, "breakpoint", 1, 0, 0, MintOpNoArgs)
 
@@ -843,7 +844,6 @@ OPDEF(MINT_TIER_MONITOR_JITERPRETER, "tier_monitor_jiterpreter", 3, 0, 0, MintOp
 #endif // HOST_BROWSER
 
 IROPDEF(MINT_NOP, "nop", 1, 0, 0, MintOpNoArgs)
-IROPDEF(MINT_NIY, "niy", 1, 0, 0, MintOpNoArgs)
 IROPDEF(MINT_DEF, "def", 2, 1, 0, MintOpNoArgs)
 IROPDEF(MINT_IL_SEQ_POINT, "il_seq_point", 1, 0, 0, MintOpNoArgs)
 IROPDEF(MINT_DUMMY_USE, "dummy_use", 2, 0, 1, MintOpNoArgs)

src/mono/mono/mini/interp/transform-simd.c

@@ -67,10 +67,6 @@ static guint16 sri_vector128_methods [] = {
 };
 
 static guint16 sri_vector128_t_methods [] = {
-	SN_get_AllBitsSet,
-	SN_get_Count,
-	SN_get_One,
-	SN_get_Zero,
 	SN_op_Addition,
 	SN_op_BitwiseAnd,
 	SN_op_BitwiseOr,
@@ -84,12 +80,29 @@ static guint16 sri_vector128_t_methods [] = {
 	SN_op_RightShift,
 	SN_op_Subtraction,
 	SN_op_UnaryNegation,
-	SN_op_UnsignedRightShift
+	SN_op_UnsignedRightShift,
+	SN_get_AllBitsSet,
+	SN_get_Count,
+	SN_get_One,
+	SN_get_Zero,
 };
 
 static guint16 sri_packedsimd_methods [] = {
+	SN_Add,
+	SN_And,
+	SN_Bitmask,
+	SN_CompareEqual,
+	SN_CompareNotEqual,
 	SN_ConvertNarrowingSignedSaturate,
 	SN_ConvertNarrowingUnsignedSaturate,
+	SN_Dot,
+	SN_Multiply,
+	SN_Negate,
+	SN_ShiftLeft,
+	SN_ShiftRightArithmetic,
+	SN_ShiftRightLogical,
+	SN_Splat,
+	SN_Subtract,
 	SN_Swizzle,
 	SN_get_IsHardwareAccelerated,
 	SN_get_IsSupported,
@@ -534,12 +547,12 @@ static gboolean
 emit_sri_packedsimd (TransformData *td, MonoMethod *cmethod, MonoMethodSignature *csignature)
 {
 	int id = lookup_intrins (sri_packedsimd_methods, sizeof (sri_packedsimd_methods), cmethod);
-	if (id == -1)
-		return FALSE;
+	// We don't early-out for an unrecognized method, we will generate an NIY later
 
 	MonoClass *vector_klass = mono_class_from_mono_type_internal (csignature->ret);
 	int vector_size = -1;
 
+	// NOTE: Linker substitutions (used in AOT) will prevent this from running.
 	if ((id == SN_get_IsSupported) || (id == SN_get_IsHardwareAccelerated)) {
 #if HOST_BROWSER
 		interp_add_ins (td, MINT_LDC_I4_1);
@@ -550,6 +563,23 @@ emit_sri_packedsimd (TransformData *td, MonoMethod *cmethod, MonoMethodSignature
 	}
 
 #if HOST_BROWSER
+	if (id < 0) {
+		g_print ("MONO interpreter: Unimplemented method: System.Runtime.Intrinsics.Wasm.PackedSimd.%s\n", cmethod->name);
+
+		// If we're missing a packedsimd method but the packedsimd method was AOT'd, we can
+		//  just let the interpreter generate a native call to the AOT method instead of
+		//  generating an NIY that will halt execution
+		ERROR_DECL (error);
+		gpointer addr = mono_aot_get_method (cmethod, error);
+		if (addr)
+			return FALSE;
+
+		// The packedsimd method implementations recurse infinitely and cause a stack overflow,
+		//  so replace them with a NIY opcode instead that will assert
+		interp_add_ins (td, MINT_NIY);
+		goto opcode_added;
+	}
+
 	gint16 simd_opcode = -1;
 	gint16 simd_intrins = -1;
 	if (!m_class_is_simd_type (vector_klass))
@@ -705,21 +735,14 @@ interp_emit_simd_intrinsics (TransformData *td, MonoMethod *cmethod, MonoMethodS
 	class_ns = m_class_get_name_space (cmethod->klass);
 	class_name = m_class_get_name (cmethod->klass);
 
-	if (mono_opt_interp_simd_v128 && !strcmp (class_ns, "System.Runtime.Intrinsics")) {
+	if (!strcmp (class_ns, "System.Runtime.Intrinsics")) {
 		if (!strcmp (class_name, "Vector128"))
 			return emit_sri_vector128 (td, cmethod, csignature);
 		else if (!strcmp (class_name, "Vector128`1"))
 			return emit_sri_vector128_t (td, cmethod, csignature);
-	} else if (mono_opt_interp_simd_packedsimd && !strcmp (class_ns, "System.Runtime.Intrinsics.Wasm")) {
-		if (!strcmp (class_name, "PackedSimd")) {
-			gboolean res = emit_sri_packedsimd (td, cmethod, csignature);
-#if HOST_BROWSER
-			if (!res)
-				g_print ("MONO interpreter: Unsupported method: System.Runtime.Intrinsics.Wasm.PackedSimd.%s\n", cmethod->name);
-			g_assert (res);
-#endif
-			return res;
-		}
+	} else if (!strcmp (class_ns, "System.Runtime.Intrinsics.Wasm")) {
+		if (!strcmp (class_name, "PackedSimd"))
+			return emit_sri_packedsimd (td, cmethod, csignature);
 	}
 	return FALSE;
 }

src/mono/mono/utils/options-def.h

@@ -60,12 +60,6 @@ DEFINE_BOOL_READONLY(readonly_flag, "readonly-flag", FALSE, "Example")
 DEFINE_BOOL(wasm_exceptions, "wasm-exceptions", FALSE, "Enable codegen for WASM exceptions")
 DEFINE_BOOL(wasm_gc_safepoints, "wasm-gc-safepoints", FALSE, "Use GC safepoints on WASM")
 DEFINE_BOOL(aot_lazy_assembly_load, "aot-lazy-assembly-load", FALSE, "Load assemblies referenced by AOT images lazily")
-#if HOST_BROWSER
-DEFINE_BOOL(interp_simd_v128, "interp-simd-v128", FALSE, "Enable interpreter Vector128 support")
-#else
-DEFINE_BOOL(interp_simd_v128, "interp-simd-v128", TRUE, "Enable interpreter Vector128 support")
-#endif
-DEFINE_BOOL(interp_simd_packedsimd, "interp-simd-packedsimd", FALSE, "Enable interpreter WASM PackedSimd support")
 
 #if HOST_BROWSER
 

src/mono/wasm/build/WasmApp.targets

@@ -124,7 +124,7 @@
     <UseAppHost>false</UseAppHost>
     <TrimMode Condition="'$(TrimMode)' == ''">full</TrimMode>
     <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.WasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
-    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true' or '$(RunAOTCompilation)' != 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.NoWasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.NoWasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
     <_ExtraTrimmerArgs Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.LegacyJsInterop.xml&quot;</_ExtraTrimmerArgs>
 
     <!-- Temporarily `false`, till sdk gets a fix for supporting the new file -->

src/mono/wasm/runtime/jiterpreter-tables.ts

@@ -1,8 +1,8 @@
 import {
-    WasmOpcode, JiterpSpecialOpcode
+    WasmOpcode, WasmSimdOpcode, JiterpSpecialOpcode
 } from "./jiterpreter-opcodes";
 import {
-    MintOpcode, SimdIntrinsic3
+    MintOpcode, SimdIntrinsic2, SimdIntrinsic3
 } from "./mintops";
 
 export const ldcTable: { [opcode: number]: [WasmOpcode, number] } = {
@@ -356,3 +356,17 @@ export const simdShiftTable = new Set<SimdIntrinsic3>([
     SimdIntrinsic3.V128_I4_URIGHT_SHIFT,
     SimdIntrinsic3.V128_I8_URIGHT_SHIFT,
 ]);
+
+export const bitmaskTable : { [intrinsic: number]: WasmSimdOpcode } = {
+    [SimdIntrinsic2.V128_I1_EXTRACT_MSB]: WasmSimdOpcode.i8x16_bitmask,
+    [SimdIntrinsic2.V128_I2_EXTRACT_MSB]: WasmSimdOpcode.i16x8_bitmask,
+    [SimdIntrinsic2.V128_I4_EXTRACT_MSB]: WasmSimdOpcode.i32x4_bitmask,
+    [SimdIntrinsic2.V128_I8_EXTRACT_MSB]: WasmSimdOpcode.i64x2_bitmask,
+};
+
+export const createScalarTable : { [intrinsic: number]: [WasmOpcode, WasmSimdOpcode] } = {
+    [SimdIntrinsic2.V128_I1_CREATE_SCALAR]: [WasmOpcode.i32_load8_s, WasmSimdOpcode.i8x16_replace_lane],
+    [SimdIntrinsic2.V128_I2_CREATE_SCALAR]: [WasmOpcode.i32_load16_s, WasmSimdOpcode.i16x8_replace_lane],
+    [SimdIntrinsic2.V128_I4_CREATE_SCALAR]: [WasmOpcode.i32_load, WasmSimdOpcode.i32x4_replace_lane],
+    [SimdIntrinsic2.V128_I8_CREATE_SCALAR]: [WasmOpcode.i64_load, WasmSimdOpcode.i64x2_replace_lane],
+};

src/mono/wasm/runtime/jiterpreter-trace-generator.ts

@@ -48,6 +48,7 @@ import {
     relopbranchTable, mathIntrinsicTable,
     simdCreateLoadOps, simdCreateSizes,
     simdCreateStoreOps, simdShiftTable,
+    bitmaskTable, createScalarTable,
 } from "./jiterpreter-tables";
 import { mono_log_error, mono_log_info } from "./logging";
 
@@ -3156,13 +3157,6 @@ function append_simd_4_load(builder: WasmBuilder, ip: MintOpcodePtr) {
     append_ldloc(builder, getArgU16(ip, 4), WasmOpcode.PREFIX_simd, WasmSimdOpcode.v128_load);
 }
 
-function append_stloc_simd_zero(builder: WasmBuilder, offset: number) {
-    builder.local("pLocals");
-    builder.appendSimd(WasmSimdOpcode.v128_const);
-    builder.appendBytes(new Uint8Array(sizeOfV128));
-    append_stloc_tail(builder, offset, WasmOpcode.PREFIX_simd, WasmSimdOpcode.v128_store);
-}
-
 function emit_simd_2(builder: WasmBuilder, ip: MintOpcodePtr, index: SimdIntrinsic2): boolean {
     const simple = <WasmSimdOpcode>cwraps.mono_jiterp_get_simd_opcode(1, index);
     if (simple) {
@@ -3172,35 +3166,33 @@ function emit_simd_2(builder: WasmBuilder, ip: MintOpcodePtr, index: SimdIntrins
         return true;
     }
 
+    const bitmask = bitmaskTable[index];
+    if (bitmask) {
+        append_simd_2_load(builder, ip);
+        builder.appendSimd(bitmask);
+        append_stloc_tail(builder, getArgU16(ip, 1), WasmOpcode.i32_store);
+        return true;
+    }
+
     switch (index) {
         case SimdIntrinsic2.V128_I1_CREATE_SCALAR:
-            // Zero then write scalar component
-            builder.local("pLocals");
-            append_stloc_simd_zero(builder, getArgU16(ip, 1));
-            append_ldloc(builder, getArgU16(ip, 2), WasmOpcode.i32_load8_s);
-            append_stloc_tail(builder, getArgU16(ip, 1), WasmOpcode.i32_store8);
-            return true;
         case SimdIntrinsic2.V128_I2_CREATE_SCALAR:
-            // Zero then write scalar component
-            builder.local("pLocals");
-            append_stloc_simd_zero(builder, getArgU16(ip, 1));
-            append_ldloc(builder, getArgU16(ip, 2), WasmOpcode.i32_load16_s);
-            append_stloc_tail(builder, getArgU16(ip, 1), WasmOpcode.i32_store16);
-            return true;
         case SimdIntrinsic2.V128_I4_CREATE_SCALAR:
-            // Zero then write scalar component
+        case SimdIntrinsic2.V128_I8_CREATE_SCALAR: {
+            const tableEntry = createScalarTable[index];
             builder.local("pLocals");
-            append_stloc_simd_zero(builder, getArgU16(ip, 1));
-            append_ldloc(builder, getArgU16(ip, 2), WasmOpcode.i32_load);
-            append_stloc_tail(builder, getArgU16(ip, 1), WasmOpcode.i32_store);
-            return true;
-        case SimdIntrinsic2.V128_I8_CREATE_SCALAR:
-            // Zero then write scalar component
-            builder.local("pLocals");
-            append_stloc_simd_zero(builder, getArgU16(ip, 1));
-            append_ldloc(builder, getArgU16(ip, 2), WasmOpcode.i64_load);
-            append_stloc_tail(builder, getArgU16(ip, 1), WasmOpcode.i64_store);
+            // Make a zero vector
+            builder.i52_const(0);
+            builder.appendSimd(WasmSimdOpcode.i64x2_splat);
+            // Load the scalar value
+            append_ldloc(builder, getArgU16(ip, 2), tableEntry[0]);
+            // Replace the first lane
+            builder.appendSimd(tableEntry[1]);
+            builder.appendU8(0);
+            // Store result
+            append_stloc_tail(builder, getArgU16(ip, 1), WasmOpcode.PREFIX_simd, WasmSimdOpcode.v128_store);
             return true;
+        }
 
         case SimdIntrinsic2.V128_I1_CREATE:
             append_simd_2_load(builder, ip, WasmSimdOpcode.v128_load8_splat);