[mono] Handle nint/nuint in Crc32 and ArmBase intrinsic dispatch#127787
[mono] Handle nint/nuint in Crc32 and ArmBase intrinsic dispatch#127787kotlarmilos wants to merge 1 commit intodotnet:mainfrom
Conversation
PR dotnet#127327 added new nint/nuint overloads for System.Runtime.Intrinsics.Arm (Crc32.ComputeCrc32(uint, nuint), ComputeCrc32C(uint, nuint), and ArmBase.ReverseElementBits(nint/nuint)). The Mono SIMD intrinsic dispatch in src/mono/mono/mini/simd-intrinsics.c only matched MONO_TYPE_U1/U2/U4/U8 and tripped g_assert_not_reached at line 4981 during AOT compilation of System.Private.CoreLib.dll on tvos-arm64 (and any 64-bit ARM AOT target). Changes: - MONO_CPU_ARM64_CRC switch now maps MONO_TYPE_I/U to U4 or U8 by pointer size and selects OP_XOP_I4_I4_I4 vs OP_XOP_I4_I4_I8 based on the actual data type rather than the enclosing-class is_64bit flag (since the new Crc32.ComputeCrc32(uint, nuint) overload lives on Crc32, not Crc32.Arm64). - ArmBase.ReverseElementBits now keys op selection off arg0_i32 so the new ArmBase.ReverseElementBits(nuint) overload picks the correct 32/64-bit intrinsic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @steveisok, @vitek-karas |
There was a problem hiding this comment.
Pull request overview
This PR fixes Mono ARM64 intrinsic dispatch for newly-added nint/nuint overloads by selecting the instruction/opcode based on the actual argument type (and pointer size), instead of relying on whether the method is in a nested *.Arm64 class. This prevents g_assert_not_reached() during AOT when compiling methods like Crc32.ComputeCrc32(uint, nuint) and ArmBase.ReverseElementBits(nint/nuint).
Changes:
- Update
ArmBase.ReverseElementBitsdispatch to choose 32-bit vs 64-bit lowering based on the argument’s underlying type. - Update
Crc32.ComputeCrc32/ComputeCrc32Cdispatch to recognizeMONO_TYPE_I/Uand select CRC32W vs CRC32X lowering based on pointer size / operand width.
Show a summary per file
| File | Description |
|---|---|
src/mono/mono/mini/simd-intrinsics.c |
Fixes ARM64 intrinsic opcode selection for nint/nuint signatures to avoid AOT-time asserts and ensure correct 32/64-bit instruction choice. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
tannergooding
left a comment
tannergooding
left a comment
There was a problem hiding this comment.
Sorry that I missed this!
Had taken a glance and it had looked like it was generally supported since we already handled Vector128<nint> for the xplat APIs, missed that we had individual paths that needed to check for TYPE_I/U as well
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description
The Mono SIMD intrinsic dispatch in
simd-intrinsics.cdid not matchMONO_TYPE_I/U, so the newnint/nuintoverloads added by #127327 (Crc32.ComputeCrc32(uint, nuint),Crc32.ComputeCrc32C(uint, nuint),ArmBase.ReverseElementBits(nint/nuint)) trippedg_assert_not_reached()at line 4981 during AOT ofSystem.Private.CoreLib.dllon tvos-arm64 (build 1406826).This unblocks AOT precompilation of
System.Private.CoreLib.dllon 64-bit ARM Mono targets and recovers four tvos appletv work items (System.Configuration.ConfigurationManager.Tests,System.Reflection.MetadataLoadContext.Tests,System.Security.Cryptography.Xml.Tests,System.Text.Json.Tests) that previously exited 21 before any test ran.