Vector128.WithElement codegen regression in .NET 9.0 by kendall1997 · Pull Request #115348 · dotnet/runtime

kendall1997 · 2025-05-06T20:51:44Z

Description

Removes the handling of NI_Vector*_WithElement as a user call and mirrors the behavior of GetElement to emit the spill, set the value and then reload the vector.

…nstant index

…ement

… user call.

…trinsicAsUserCall

…with ins_Move_Extend

…s in RewriteHWIntrinsicAsUserCall

Copilot

Pull Request Overview

This PR fixes the handling of NI_Vector*_WithElement intrinsics so that user calls are removed and the behavior is aligned with GetElement (spill, set value, then reload).

Removed the NI_Vector*_WithElement user call handling in Rationalizer.
Updated lowering, codegen, and tree construction functions to correctly manage non-constant index cases by using spill temporary variables and performing range checks.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/coreclr/jit/rationalize.cpp	Removed the special handling for NI_Vector*_WithElement to mirror GetElement behavior.
src/coreclr/jit/lowerxarch.cpp	Added special handling for non-constant op2 by obtaining a SIMD temp var and deferring containment.
src/coreclr/jit/hwintrinsicxarch.cpp	Removed the early return branch for non-constant index handling in favor of later processing.
src/coreclr/jit/hwintrinsiccodegenxarch.cpp	Added a branch for non-constant op2 that stores the vector, updates the element, and reloads it.
src/coreclr/jit/gentree.cpp	Updated node creation to perform a range check for non-constant op2 values before constructing the intrinsic node.

Copilot · 2025-05-06T20:52:10Z

src/coreclr/jit/lowerxarch.cpp

    GenTree* op3 = node->Op(3);

-    assert(op2->OperIsConst());
+    if (!op2->OperIsConst())


Consider adding an inline comment explaining why a SIMD initialization temp variable is obtained when op2 is not a constant for improved clarity.

Copilot · 2025-05-06T20:52:10Z

src/coreclr/jit/hwintrinsiccodegenxarch.cpp

+                unsigned offs = compiler->lvaFrameAddress(simdInitTempVarNum, &isEBPbased);
+
+#if !FEATURE_FIXED_OUT_ARGS
+                if (!isEBPbased)


Consider clarifying inline how the offset 'offs' is adjusted for non-EBP-based stacks to ensure the correct offset calculation across platforms.

Copilot · 2025-05-06T20:52:10Z

src/coreclr/jit/gentree.cpp

 #error Unsupported platform
 #endif // !TARGET_XARCH && !TARGET_ARM64

+    int  immUpperBound    = getSIMDVectorLength(simdSize, simdBaseType) - 1;


Add an explanatory comment regarding the rationale for using 'getSIMDVectorLength(simdSize, simdBaseType) - 1' as the upper bound to aid future maintainability.

dotnet-policy-service · 2025-05-06T20:52:32Z

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

kendall1997 · 2025-05-06T20:56:51Z

@dotnet-policy-service agree company="Intel"

kendall1997 · 2025-05-07T17:29:13Z

@dotnet/intel

src/coreclr/jit/hwintrinsiccodegenxarch.cpp

…re to load instruction

src/coreclr/jit/hwintrinsiccodegenxarch.cpp

tannergooding

LGTM. CC. @dotnet/jit-contrib, @BruceForstall for secondary review

kunalspathak

LGTM

AndyAyersMS · 2025-05-09T01:57:59Z

Looks like perhaps jit-format failed to run? I am getting formatting errors in this file in my PR

clang-format: there are formatting errors in D:\a\runtime\runtime\runtime\src\coreclr\jit\hwintrinsiccodegenxarch.cpp
At Line 2004 Before:
            GetEmitter()->emitIns_S_R(ins_Store(simdType, compiler->isSIMDTypeLocalAligned(simdInitTempVarNum)),
                                        emitTypeSize(simdType), op1Reg, simdInitTempVarNum, 0);
After:
            GetEmitter()->emitIns_S_R(ins_Store(simdType, compiler->isSIMDTypeLocalAligned(simdInitTempVarNum)),
                                      emitTypeSize(simdType), op1Reg, simdInitTempVarNum, 0);
At Line 2008 Before:
            GetEmitter()->emitIns_ARX_R(ins_Move_Extend(op3->TypeGet(), false),   // Store
After:
            GetEmitter()->emitIns_ARX_R(ins_Move_Extend(op3->TypeGet(), false), // Store
At Line 2009 Before:

kendall1997 · 2025-05-09T02:19:43Z

Created #115416 to try fix the issue.

…115348)" This reverts commit e42c072.

* Revert "JIT: Apply format suggested by automation (#115416)" This reverts commit df1eba9. * Revert "Vector128.WithElement codegen regression in .NET 9.0 (#115348)" This reverts commit e42c072.

kendall1997 added 8 commits April 9, 2025 20:43

JIT: Enhance range checking for SIMD element node creation for non co…

a7d647e

…nstant index

JIT: Enhance handling of non-constant index in LowerHWIntrinsicWithEl…

a6cbb1c

…ement

JIT: Remove conditional handling for non-constant index that caused a…

65b4dc8

… user call.

JIT: Optimize handling of non-constant index for SIMD WithElement

7179450

JIT: Comment out handling for NI_Vector128_WithElement in RewriteHWIn…

c55a160

…trinsicAsUserCall

JIT: Update instruction for storing SIMD values to use emitIns_ARX_R …

78829fc

…with ins_Move_Extend

JIT: Remove handling for NI_Vector*_WithElement and related SIMD case…

b2bd49e

…s in RewriteHWIntrinsicAsUserCall

Merge remote-tracking branch 'origin' into Kendall/Vector128.WithElement

25a7069

Copilot AI review requested due to automatic review settings May 6, 2025 20:51

ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 6, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 6, 2025

Copilot AI reviewed May 6, 2025

View reviewed changes

kendall1997 changed the title ~~Kendall/vector128.with element~~ Vector128.WithElement codegen regression in .NET 9.0 May 6, 2025

build-analysis bot mentioned this pull request May 7, 2025

/root/helix/work/correlation/scripts/<hash>/execute.sh: Permission denied dotnet/dnceng#3412

Open

3 tasks

tannergooding reviewed May 7, 2025

View reviewed changes

src/coreclr/jit/hwintrinsiccodegenxarch.cpp Outdated Show resolved Hide resolved

tannergooding reviewed May 7, 2025

View reviewed changes

src/coreclr/jit/hwintrinsiccodegenxarch.cpp Show resolved Hide resolved

tannergooding reviewed May 7, 2025

View reviewed changes

src/coreclr/jit/hwintrinsiccodegenxarch.cpp Outdated Show resolved Hide resolved

JIT: Assert op2 is not a constant for Vector*_WithElement and fix sto…

50329c2

…re to load instruction

tannergooding reviewed May 7, 2025

View reviewed changes

src/coreclr/jit/hwintrinsiccodegenxarch.cpp Outdated Show resolved Hide resolved

JIT: Remove if checking because of the previous assert

e2e118e

tannergooding approved these changes May 7, 2025

View reviewed changes

build-analysis bot mentioned this pull request May 8, 2025

Test failure: baseservices/exceptions/stackoverflow/stackoverflowtester/stackoverflowtester.cmd #110173

Open

kunalspathak approved these changes May 8, 2025

View reviewed changes

kunalspathak merged commit e42c072 into dotnet:main May 8, 2025
104 of 106 checks passed

AndyAyersMS mentioned this pull request May 9, 2025

JIT: escape analysis for object fields #115388

Closed

kendall1997 mentioned this pull request May 9, 2025

JIT: Fix format #115416

Merged

jakobbotsch mentioned this pull request May 16, 2025

JIT: x86 SPMI replay failing under JitStressRegs=8 with Assertion failed 'emitVerifyEncodable(ins, EA_SIZE(attr), reg)' during 'Generate code' (IL size 504; hash 0x20a38a17; MinOpts) #115644

Closed

jakobbotsch added a commit to jakobbotsch/runtime that referenced this pull request May 16, 2025

Revert "Vector128.WithElement codegen regression in .NET 9.0 (dotnet#…

f43ade8

…115348)" This reverts commit e42c072.

tannergooding mentioned this pull request May 16, 2025

Fix a couple issues with Vector.WithElement #115648

Merged

amanasifkhalid mentioned this pull request May 16, 2025

Test failure: _Avx512F_VL_Vector256_r::JIT.HardwareIntrinsics.X86._Avx512F_VL_Vector256.Program.RoundScaleSingle0() #115461

Closed

github-actions bot locked and limited conversation to collaborators Jun 8, 2025

Conversation

kendall1997 commented May 6, 2025

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 6, 2025

Choose a reason for hiding this comment

Uh oh!

dotnet-policy-service bot commented May 6, 2025

Uh oh!

kendall1997 commented May 6, 2025

Uh oh!

kendall1997 commented May 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tannergooding left a comment

Choose a reason for hiding this comment

Uh oh!

kunalspathak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AndyAyersMS commented May 9, 2025

Uh oh!

kendall1997 commented May 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants