JIT: Include auxiliary types in hardware intrinsic value numbering - #134518
Merged
tannergooding merged 1 commit intoSep 23, 2026
Merged
tannergooding merged 1 commit into
tannergooding merged 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot stopped reviewing on behalf of
tannergooding due to an error
September 23, 2026 13:34
|
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. |
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
This PR updates JIT value numbering for HW intrinsics to distinguish overloads that share the same operand bits (via an “auxiliary type” encoded into the SIMD type VN) and adds a regression test to validate the behavior for gather and AddSaturateScalar.
Changes:
- Encode an additional
auxiliaryTypeintoVNF_SimdTypevalue numbers and update related decoding/dumping. - Update HW intrinsic value numbering to incorporate
GetAuxiliaryType()into the result type VN. - Add a JIT regression test covering AVX2 gather index width and Arm64 AddSaturateScalar overload distinction.
| File | Description |
|---|---|
| src/tests/JIT/Regression_ro_2/Runtime_134486.cs | Adds regression coverage to catch incorrect VN/CSE across intrinsic overloads. |
| src/coreclr/jit/valuenum.h | Introduces SIMD type encoding constants and extends the VNForSimdType API to accept an auxiliary type. |
| src/coreclr/jit/valuenum.cpp | Implements auxiliary-type encoding/decoding, updates simd-type dumping, and uses auxiliary type for HW intrinsic VNs. |
tannergooding
commented
Sep 23, 2026
AndyAyersMS
approved these changes
Sep 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Hardware intrinsic value numbering omitted the auxiliary type even when it changes the operation's semantics. For example, AVX2 gathers using the same index bits as
intandlongindices received the same liberal VN, allowing the JIT to incorrectly fold a comparison of their different results.Pack the auxiliary type into the upper byte of the existing SIMD base-type constant and update decoding and diagnostic output.
SimdTyperetains two operands, and intrinsics without an auxiliary type retain their original type constant, avoiding an additional VN argument and constant lookup. A compile-time assertion bounds the encoding.Add focused regressions for gather index width on xarch and saturating-add operand signedness on Arm64.
Validation: checked native and cross-target JIT builds; 368 regression tests passed; the gather repro fails against the baseline and passes with the fix, including under JitStress2; JIT formatting passed. Arm64 crossgen2 output preserves distinct
sqaddandsuqaddinstructions, but the Arm64 test was not executed on matching hardware.Resolves #134486
Note
This PR description and changes were prepared with GitHub Copilot.