From de7a76b3ed9b2b6e6db4544064bf166b2cacb8fb Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Mon, 8 May 2023 09:07:58 -0700 Subject: [PATCH] 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 --- eng/testing/tests.wasm.targets | 2 +- src/mono/mono/mini/interp/interp.c | 4 ++ src/mono/mono/mini/interp/mintops.def | 2 +- src/mono/mono/mini/interp/transform-simd.c | 59 +++++++++++++------ src/mono/mono/utils/options-def.h | 6 -- src/mono/wasm/build/WasmApp.targets | 2 +- src/mono/wasm/runtime/jiterpreter-tables.ts | 18 +++++- .../runtime/jiterpreter-trace-generator.ts | 52 +++++++--------- 8 files changed, 86 insertions(+), 59 deletions(-) diff --git a/eng/testing/tests.wasm.targets b/eng/testing/tests.wasm.targets index 4d6abbf20effd8..c09fe52717a020 100644 --- a/eng/testing/tests.wasm.targets +++ b/eng/testing/tests.wasm.targets @@ -61,7 +61,7 @@ So, set those parameters explicitly here. --> <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions "$(MonoProjectRoot)\wasm\build\ILLink.Substitutions.WasmIntrinsics.xml" - <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true' or '$(RunAOTCompilation)' != 'true'">$(_ExtraTrimmerArgs) --substitutions "$(MonoProjectRoot)\wasm\build\ILLink.Substitutions.NoWasmIntrinsics.xml" + <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true'">$(_ExtraTrimmerArgs) --substitutions "$(MonoProjectRoot)\wasm\build\ILLink.Substitutions.NoWasmIntrinsics.xml" diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 6f45a2607bee5e..980b4887d69f97 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/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); diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index f3016efabf6155..c2b4a581d7fee2 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/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) diff --git a/src/mono/mono/mini/interp/transform-simd.c b/src/mono/mono/mini/interp/transform-simd.c index b259d3f994b1a1..175f38dab45397 100644 --- a/src/mono/mono/mini/interp/transform-simd.c +++ b/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; } diff --git a/src/mono/mono/utils/options-def.h b/src/mono/mono/utils/options-def.h index 6d8715c2465ffd..29cf845ca1b448 100644 --- a/src/mono/mono/utils/options-def.h +++ b/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 diff --git a/src/mono/wasm/build/WasmApp.targets b/src/mono/wasm/build/WasmApp.targets index d96c84131533f4..6a5d91ce57c626 100644 --- a/src/mono/wasm/build/WasmApp.targets +++ b/src/mono/wasm/build/WasmApp.targets @@ -124,7 +124,7 @@ false full <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions "$(MSBuildThisFileDirectory)ILLink.Substitutions.WasmIntrinsics.xml" - <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true' or '$(RunAOTCompilation)' != 'true'">$(_ExtraTrimmerArgs) --substitutions "$(MSBuildThisFileDirectory)ILLink.Substitutions.NoWasmIntrinsics.xml" + <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true'">$(_ExtraTrimmerArgs) --substitutions "$(MSBuildThisFileDirectory)ILLink.Substitutions.NoWasmIntrinsics.xml" <_ExtraTrimmerArgs Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(_ExtraTrimmerArgs) --substitutions "$(MSBuildThisFileDirectory)ILLink.Substitutions.LegacyJsInterop.xml" diff --git a/src/mono/wasm/runtime/jiterpreter-tables.ts b/src/mono/wasm/runtime/jiterpreter-tables.ts index ebaa91f309d1d7..0fef162c421098 100644 --- a/src/mono/wasm/runtime/jiterpreter-tables.ts +++ b/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.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], +}; diff --git a/src/mono/wasm/runtime/jiterpreter-trace-generator.ts b/src/mono/wasm/runtime/jiterpreter-trace-generator.ts index 8df1ff8a930599..4125a743f0a28e 100644 --- a/src/mono/wasm/runtime/jiterpreter-trace-generator.ts +++ b/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 = 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);