Repository navigation
Fix scalar-typed SIMD operands in Dot and mask inversion lowering - #133709
Merged
tannergooding merged 1 commit intoSep 12, 2026
Merged
tannergooding merged 1 commit into
tannergooding merged 1 commit into
Conversation
Construct Dot's vector multiply directly using the intrinsic-ID lookup, without introducing an unthreaded scalar broadcast. Accept floating-point register operands in LIR comparison shape assertions after CreateScalarUnsafe elision. Co-authored-by: Copilot App <[email protected]>
|
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.
Copilot review overview
🔵 Needs a closer look
Add AggressiveOptimization to the regression helpers so they reliably exercise optimized lowering.
Review tier: Lite
Findings: None
What changed in this PR
Fixes CoreCLR JIT lowering for scalar-typed SIMD operands in Dot and mask-inversion paths.
Changes:
- Constructs Dot multiplication intrinsics with vector semantics.
- Accepts scalar floating-point register operands during comparison lookup.
- Adds Dot and Vector512 select regressions.
| File | Summary |
|---|---|
src/tests/JIT/Regression_ro_2/Runtime_133554.cs |
Adds Vector512 select regression coverage. |
src/tests/JIT/Regression_ro_2/Runtime_133551.cs |
Adds Dot and scalar arithmetic regression coverage. |
src/coreclr/jit/lowerxarch.cpp |
Corrects Dot multiplication lowering. |
src/coreclr/jit/gentree.cpp |
Allows valid scalar FP register operands in LIR comparisons. |
EgorBo
approved these changes
Sep 11, 2026
Member
Author
|
/ba-g unrelated mono failures, also in main |
jtschuster
pushed a commit
to jtschuster/runtime
that referenced
this pull request
Sep 18, 2026
…tnet#133709) Fixes dotnet#133551. Fixes dotnet#133554. Floating-point `CreateScalarUnsafe` elision leaves scalar-typed operands in LIR while hardware-intrinsic consumers still read them as vector registers. Two downstream paths did not account for that representation: - Dot lowering used `gtNewSimdBinOpNode`, which interprets scalar-typed multiply operands as requesting broadcasts. That could create a child outside the LIR sequence. Select the vector multiply with `GetHWIntrinsicIdForBinOp` and construct the intrinsic directly instead. The shared builder's scalar-broadcast semantics remain unchanged. - Comparison-ID lookup rejected scalar FP operands when mask inversion attempted to enable embedded zeroing. Accept FP/SIMD register representations in its LIR assertions, retaining exact vector types in HIR. Instruction selection, NaN-aware inversion, scalar spill sizes, and memory-width containment remain unchanged. Add focused regressions for float/double Dot at 128/256 bits in both operand orders, scalar multiplication/division controls, and 512-bit selects with register/byref inputs covering equal, unequal, and NaN values. Dot tests intentionally do not assert results derived from undefined upper lanes. ### Validation - Reproduced both assertions with the original Checked JIT. - Built the final Windows x64 Checked JIT and passed all eight focused cases with native ISA, AVX-512 disabled, AVX disabled, hardware intrinsics disabled, and register stress. Rebuilt and repeated native/AVX runs after formatting. - All 14 captured leaf disassemblies match the direct-intrinsic-ID implementation in native and AVX configurations. Scalar multiply/divide controls also match the original baseline. Native disassembly confirms packed multiplies, scalar-width byref loads, and masked zeroing. - JIT formatting passed. Other JIT targets were compile-checked before the final instruction-lookup refactoring; execution was Windows x64 only. No throughput timing or full-suite result is claimed. > [!NOTE] > This PR description was generated by GitHub Copilot. Co-authored-by: Copilot App <[email protected]>
Member
Author
|
/backport to release/11.0 Note This backport request was posted by GitHub Copilot at the user's direction. |
Contributor
|
Started backporting to |
2 of 4 tasks
tannergooding
added a commit
that referenced
this pull request
Sep 20, 2026
…on lowering (#134215) Backport of #133709 to release/11.0 /cc @tannergooding ## Customer Impact - [ ] Customer reported - [x] Found internally This fixes #133551, where `Vector128.Dot` or `Vector256.Dot` with a `CreateScalarUnsafe` operand leaves an invalid scalar/vector shape during lowering and can fail JIT compilation. The repro fails on .NET 11 RC2 but behaves correctly on .NET 8, 9, and 10. The same lowering-shape fix also addresses the Checked-JIT assertion reported in #133554 for a `Vector512` conditional select. ## Regression - [x] Yes - [ ] No The #133551 failure was introduced during the .NET 11 development cycle; the exact introducing change was not identified. #133554 is included because it exercises the same scalar-typed SIMD representation invariant. ## Testing The source PR added focused `float` and `double` Dot regressions at 128 and 256 bits in both operand orders, scalar multiply/divide controls, and 512-bit conditional-select cases covering register and byref inputs, equal and unequal values, and NaNs. All eight focused cases passed with native ISA support, AVX-512 disabled, AVX disabled, hardware intrinsics disabled, and register stress. Fourteen captured leaf disassemblies confirmed the intended packed operations, scalar-width loads, and masked zeroing without changing the scalar controls. ## Risk Low. The product diff is small and corrects established scalar/SIMD representation invariants in two downstream lowering paths without changing the shared builder semantics. Focused coverage exercises operand orders, vector widths, ISA fallbacks, memory and register forms, NaNs, and register stress, with before/after disassembly confirming the intended codegen. **IMPORTANT**: If this backport is for a servicing release, please verify that: - For .NET 8 and .NET 9: The PR target branch is `release/X.0-staging`, not `release/X.0`. - For .NET 10+: The PR target branch is `release/X.0` (no `-staging` suffix). ## Package authoring no longer needed in .NET 9 **IMPORTANT**: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. > [!NOTE] > This backport template was completed by GitHub Copilot at the user's direction. Co-authored-by: Tanner Gooding <[email protected]> Co-authored-by: Copilot App <[email protected]>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #133551.
Fixes #133554.
Floating-point
CreateScalarUnsafeelision leaves scalar-typed operands in LIR while hardware-intrinsic consumers still read them as vector registers. Two downstream paths did not account for that representation:gtNewSimdBinOpNode, which interprets scalar-typed multiply operands as requesting broadcasts. That could create a child outside the LIR sequence. Select the vector multiply withGetHWIntrinsicIdForBinOpand construct the intrinsic directly instead. The shared builder's scalar-broadcast semantics remain unchanged.Add focused regressions for float/double Dot at 128/256 bits in both operand orders, scalar multiplication/division controls, and 512-bit selects with register/byref inputs covering equal, unequal, and NaN values. Dot tests intentionally do not assert results derived from undefined upper lanes.
Validation
Note
This PR description was generated by GitHub Copilot.