[mono][interp] Fix mis-sorted SIMD intrinsic tables making entries unreachable

[lewing]

Problem

lookup_intrins() in transform-simd.c resolves method names with mono_binary_search + strcmp, which requires the tables to be in strict ASCII order. Two tables had drifted out of order, so binary search never reached certain entries: lookup returns -1, the switch falls to default: return FALSE, and the intrinsic is silently never emitted.

The failure is invisible — no crash, no assert, no test failure. The call just falls back to the managed implementation and runs orders of magnitude slower. That is why this went unnoticed.

Entries unreachable on main:

Table	Unreachable entries
sri_vector128_methods	AsVector4
packedsimd_alias_methods	Store, StoreUnsafe, Subtract, SubtractSaturate

Changes

1. sri_vector128_methods — reorder SN_AsVector, SN_AsVector4, SN_AsVector128 → SN_AsVector, SN_AsVector128, SN_AsVector4.
2. packedsimd_alias_methods — move SN_Sqrt, SN_SquareRoot from after SN_SubtractSaturate to before SN_Subtract, restoring the sort invariant and making Subtract/SubtractSaturate reachable.
3. packedsimd_alias_methods — removeSN_Store / SN_StoreUnsafe (and the now-orphaned case block), replaced with a comment explaining why. See below.
4. New check_intrins_sorted() under ENABLE_CHECKED_BUILD, called from lookup_intrins(), so any future mis-sort fails loudly instead of silently degrading. ENABLE_CHECKED_BUILD is on for any Debug mono build, so this gets broad coverage. Cost is at most ~46 short strcmps per SIMD resolution at transform time.

Why Store/StoreUnsafe are removed rather than sorted into place

This is the part worth reviewing carefully. Naively re-sorting the table would have activated a latent memory-corruption bug, which is presumably why nobody noticed these were dead.

The operands are reversed between the two APIs:

// interp-simd.c:799 — expects (address, vector)interp_packedsimd_store (res, addr_of_addr, vec)
→ **(v128_t**)addr_of_addr=*(v128_t*)vec;

API	Signature	Matches helper?
PackedSimd.Store(T* address, Vector128<T> source)	(address, vector)	✅
Vector128.Store<T>(this Vector128<T> source, T* destination)	(vector, address)	❌ reversed
Vector128.StoreUnsafe<T>(this Vector128<T> source, ref T destination)	(vector, address)	❌ reversed

The alias path only renamescmethod_name; emit_common_simd_epilogue then assigns sregs[i] = sp[i].var in signature order with no reordering hook. The existing param_count != 2 guard is necessary but not sufficient — lookup_packedsimd_intrinsic("Store", ...) matches the Store, ANY entry regardless. The result would be vector data dereferenced as a pointer and 16 bytes written through it.

Removing the entries preserves main's behavior exactly (they are unreachable today) while making that explicit and keeping the table sorted.

Making these genuinely intrinsifiable is worth doing — measured ~6.4x on the interpreter — but it needs an operand-swap mechanism in the emit path, which is a feature rather than a bug fix and belongs in its own change.

Why the newly-reachable entries are safe

• Subtract — identical (Vector128<T> left, Vector128<T> right) shape in both APIs. Covers D1/D2/D4/D8/R4/R8 (including nint/nuint → D4 on wasm32 via resolve_native_size).
• SubtractSaturate — exists only for I1/U1/I2/U2. For other types packedsimd_type_matches hits default: return FALSE and falls back to managed. Structurally identical to the already-reachable AddSaturate.
• AsVector4 — only Vector128.AsVector4(this Vector128<float>) reaches the switch; return and argument are both 16 bytes, so it emits MINT_MOV_VT 16, exactly the Unsafe.BitCast it replaces. The Plane/Quaternion/Vector2/Vector3 overloads return FALSE earlier (non-GENERICINST).
• Neither Add nor Subtract nor SubtractSaturate has scalar overloads (only Multiply does, and SN_Multiply already guards with scalar_arg != -1), so the absent scalar_arg check is not exploitable here.

Sqrt/SquareRoot happened to be reachable despite their misplacement — the search path landed on them. Moving them is invariant hygiene, not a behavior change.

Measurements

browser-wasm / V8, interpreter, dependency-chained loops, best of 5 after 2 warmups:

Operation	Before	After	Speedup
Vector128.SubtractSaturate<short>	28.13 ns	0.84 ns	33.5x
Vector128.SubtractSaturate<byte>	30.73 ns	0.84 ns	36.6x
Vector128.Subtract<int> (static)	0.69 ns	0.68 ns	unchanged

SubtractSaturate was running a fully-managed scalar loop and now lowers to i8x16.sub_sat / i16x8.sub_sat. Static Subtract is unchanged because its body is a - b, already intrinsified through op_Subtraction and inlined — the table entry was redundant for it.

Validation

• Audit script replaying mono_binary_search over all 5 tables: all sorted, all entries reachable.
• Checked build with the new assert active: builds clean, 13021/13021System.Runtime.Intrinsics tests pass on browser-wasm, assert never fires.
• Shipping config: 0 warnings / 0 errors, 13021/13021.
• Native osx-arm64 mono: 0 warnings / 0 errors.
• IR dump confirms emission: simd_intrins_p_pp [80 <- 80 96], 166 → SubtractSaturateI2.

Note on merge order

This also touches sri_vector128_methods, which overlaps an incidental reorder in #132499. Trivial conflict; whichever merges second needs a rebase. It could not be scoped out of this change — the new sortedness assert fires on the pre-existing AsVector4 misplacement, so the fix has to land together with the assert.

Note

This pull request description was generated with the assistance of GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @vitek-karas, @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR fixes Mono interpreter SIMD intrinsic lookup reliability by restoring strict ASCII sort order for binary-searched method tables, and adds a checked-build invariant check so future table drift fails loudly instead of silently disabling intrinsics.

Changes:

• Reorders sri_vector128_methods so AsVector4 is reachable via mono_binary_search.
• Reorders packedsimd_alias_methods so Subtract/SubtractSaturate are reachable, and removes Store/StoreUnsafe alias entries (with an explanatory comment).
• Adds check_intrins_sorted() under ENABLE_CHECKED_BUILD and calls it from lookup_intrins().

Suppressed comments (1)

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

• Indentation in lookup_intrins uses spaces instead of the surrounding file's tab indentation, which makes the block stand out and can cause formatting churn in future diffs. Re-indent these lines to match the existing style in this file.

#ifdef ENABLE_CHECKED_BUILD
check_intrins_sorted (intrinsics, size);
#endif
guint16 *result = mono_binary_search (cmethod_name, intrinsics, size / sizeof (guint16), sizeof (guint16), &simd_intrinsic_compare_by_name);
if (result == NULL)
return -1;
else
return (int)*result;

[pavelsavara]

Which CI leg is running the new test ?

[lewing]

/backport to release/11.0-rc1

[github-actions]

Started backporting to release/11.0-rc1 (link to workflow run)