Skip to content

[arm64] Use the scalar shift when scaling Vector64 shuffle indices - #131488

Open
lewing wants to merge 2 commits into
dotnet:mainfrom
lewing:fix-vector64-shuffle-1d
Open

[arm64] Use the scalar shift when scaling Vector64 shuffle indices#131488
lewing wants to merge 2 commits into
dotnet:mainfrom
lewing:fix-vector64-shuffle-1d

Conversation

@lewing

@lewing lewing commented Jul 28, 2026

Copy link
Copy Markdown
Member

Fixes #131487

gtNewSimdShuffleVariableNode scales variable shuffle indices into byte offsets with AdvSimd.ShiftLeftLogical. For a Vector64 with a 64-bit element type that is a 1D arrangement, which the vector form of the arm64 shift-by-immediate has no encoding for, so a checked JIT trips assert(opt != INS_OPTS_1D) in emitarm64.cpp.

gtNewSimdBinOpNode already selects the scalar form for exactly this case; the shuffle path builds the HWINTRINSIC node directly and so never got the guard. This applies the same condition.

-        cnsNode = gtNewIconNode(BitOperations::TrailingZeroCount(static_cast<uint64_t>(elementSize)), TYP_INT);
-        op2     = gtNewSimdHWIntrinsicNode(type, op2, cnsNode, NI_AdvSimd_ShiftLeftLogical, simdBaseType, simdSize);
+        NamedIntrinsic shiftIntrinsic =
+            ((simdSize == 8) && (elementSize == 8)) ? NI_AdvSimd_ShiftLeftLogicalScalar : NI_AdvSimd_ShiftLeftLogical;
+        cnsNode = gtNewIconNode(BitOperations::TrailingZeroCount(static_cast<uint64_t>(elementSize)), TYP_INT);
+        op2     = gtNewSimdHWIntrinsicNode(type, op2, cnsNode, shiftIntrinsic, simdBaseType, simdSize);

Impact

ILC instantiates ShuffleNative<nuint> when compiling CoreLib whole-program, so this fails any NativeAOT build using a checked arm64 JIT (ilc … exited with code 133), which blocks src/tests/build.sh nativeaot checked on an arm64 host. It escapes CI because windows-x64 Checked NativeAOT is the only checked NativeAOT leg and the arm64 NativeAOT legs are Release, where the assert is compiled out.

Tests

Four tests added to Vector64Tests.cs covering nint/nuint × Shuffle/ShuffleNative. Two details drove their shape:

  • nint/nuint are the only Vector64 shuffle overloads with a 64-bit element type — there are no long/ulong/double ones — and only on a 64-bit platform. The tests are written pointer-size agnostic.
  • Only the variable indices path emits the shift, so the indices are routed through a [MethodImpl(NoInlining)] wrapper. The existing constant/local-indices style folds and would not cover this.

Verified on osx-arm64, checked runtime:

result
new tests without the fix SIGABRT — Assertion failed 'opt != INS_OPTS_1D'
new tests with the fix 4/4 pass
full System.Runtime.Intrinsics.Tests 13008 total, 0 failed
ILC compiling CoreLib previously exit 133; now completes

Note

This change was prepared with GitHub Copilot assistance and reviewed by the submitting developer.

gtNewSimdShuffleVariableNode scales variable shuffle indices into byte offsets
with AdvSimd.ShiftLeftLogical. For a Vector64 with a 64-bit element type that is
a 1D arrangement, which the vector form of the shift has no encoding for, so the
JIT tripped 'opt != INS_OPTS_1D' in the arm64 emitter. gtNewSimdBinOpNode
already selects the scalar form for this case; do the same here.

nint/nuint are the only Vector64 shuffle overloads with a 64-bit element type,
and only on a 64-bit platform, so this needs variable indices on arm64 to hit.
ILC reached it compiling CoreLib, which blocked NativeAOT builds using a checked
JIT on arm64.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 77f83e48-ef2e-4f33-b6f4-5c20e251b20d
Copilot AI review requested due to automatic review settings July 28, 2026 19:58
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 28, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 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

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an arm64 checked-JIT assertion when compiling Vector64.Shuffle / Vector64.ShuffleNative with variable indices for a Vector64 whose element size is 64-bit (a 1D arrangement). The JIT now selects the scalar shift intrinsic when scaling shuffle indices in that specific Vector64/64-bit-element case, avoiding emission of a reserved vector shift encoding.

Changes:

  • Update gtNewSimdShuffleVariableNode (arm64) to use NI_AdvSimd_ShiftLeftLogicalScalar when (simdSize == 8) && (elementSize == 8) while scaling shuffle indices.
  • Add regression tests for nint/nuint covering Shuffle and ShuffleNative with variable indices (via non-inlined wrappers).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/coreclr/jit/gentree.cpp Selects scalar vs vector shift intrinsic for the Vector64+64-bit-element shuffle-index scaling case on arm64.
src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs Adds xUnit coverage to exercise the variable-indices shuffle path for Vector64<nint> / Vector64<nuint> (Shuffle + ShuffleNative).

Comment thread src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs Outdated
The nint/nuint shuffle overloads exist on 32-bit too; it is the element that is
64-bit only on a 64-bit platform.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 77f83e48-ef2e-4f33-b6f4-5c20e251b20d
Copilot AI review requested due to automatic review settings July 28, 2026 21:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs:2486

  • Same issue as above: the inserted indices are compile-time constants, so this may not reliably hit the JIT variable-indices shuffle path. Wrap the inserted values with GetNonConstant to keep them variable.
            for (int index = 0; index < Vector64<nuint>.Count; index++)
            {
                vector = vector.WithElement(index, (nuint)(index + 1));
                indices = indices.WithElement(index, (nuint)(Vector64<nuint>.Count - index - 1));
            }

src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs:2506

  • Same issue as above: constant indices may allow the JIT to take the constant-indices shuffle path and skip the index fix-up logic. Wrap inserted values with GetNonConstant so the indices remain variable.
            for (int index = 0; index < Vector64<nint>.Count; index++)
            {
                vector = vector.WithElement(index, (nint)(index + 1));
                indices = indices.WithElement(index, (nint)(Vector64<nint>.Count - index - 1));
            }

src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs:2526

  • Same issue as above: the indices are inserted as constants, which can prevent coverage of the variable-index shuffle path this regression is targeting. Wrap inserted values with GetNonConstant.
            for (int index = 0; index < Vector64<nuint>.Count; index++)
            {
                vector = vector.WithElement(index, (nuint)(index + 1));
                indices = indices.WithElement(index, (nuint)(Vector64<nuint>.Count - index - 1));
            }

src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs:2466

  • These tests intend to cover the JIT’s variable indices shuffle path, but the indices being inserted are compile-time constants. Especially on 64-bit where Vector64<nint>.Count == 1, the JIT can treat the indices as a constant vector and take the constant-indices path, which would not exercise the index fix-up/shift sequence this PR is guarding. Use the existing GetNonConstant helper when inserting elements to prevent constant folding.

This issue also appears in the following locations of the same file:

  • line 2482
  • line 2502
  • line 2522
            for (int index = 0; index < Vector64<nint>.Count; index++)
            {
                vector = vector.WithElement(index, (nint)(index + 1));
                indices = indices.WithElement(index, (nint)(Vector64<nint>.Count - index - 1));
            }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[arm64] Assertion failed 'opt != INS_OPTS_1D' shuffling Vector64&lt;nint&gt;/Vector64&lt;nuint&gt; with variable indices

2 participants