Revert "Vector128.WithElement codegen regression in .NET 9.0

[jakobbotsch]

Fix #115532
Fix #115487
Fix #115459
Fix #115644
Unfix #108197

[Copilot]

Pull Request Overview

This PR reverts a prior change in the code generation for the Vector128.WithElement intrinsic to address several reported regressions. It enforces that the second operand is constant, removes obsolete fallback paths, and adjusts intrinsic lowering and node creation accordingly.

• Enforces op2 to be constant in lowerings and node creation
• Removes legacy software fallback code for non-constant index handling
• Updates intrinsic handling for various vector sizes under different targets

Reviewed Changes

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

Show a summary per file

File	Description
src/coreclr/jit/rationalize.cpp	Adds a new case for Vector*_WithElement with constant operand checks
src/coreclr/jit/lowerxarch.cpp	Replaces non-constant branch with an assert to enforce op2 constant
src/coreclr/jit/hwintrinsicxarch.cpp	Adds handling for non-const index in a late-stage rewriting when optimizations apply
src/coreclr/jit/hwintrinsiccodegenxarch.cpp	Removes the old codegen fallback for non-constant indices
src/coreclr/jit/gentree.cpp	Introduces assertions and range checks assuming op2 is a constant

Comments suppressed due to low confidence (2)

src/coreclr/jit/lowerxarch.cpp:5661

• The assertion here enforces that op2 must be constant, which is critical for WithElement intrinsic handling. Consider adding explicit handling or validation for non-constant op2 in release builds to avoid unexpected behavior.

assert(op2->OperIsConst());

src/coreclr/jit/gentree.cpp:27870

• This assert depends on op2 being a constant immediate value. Please verify that all upstream transformations guarantee this invariant to prevent potential issues in release builds.

assert(op2->IsCnsIntOrI());

[jakobbotsch]

cc @dotnet/jit-contrib @kendall1997

Sorry @kendall1997, but I think for now it is best if we revert the change. Can you please investigate the failures and resubmit the change once fixed?

[dotnet-policy-service]

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

[tannergooding]

@jakobbotsch, I'm fine with this being reverted; however, it looks like these comes down to 2 very simple issues so it might be worth just resolving them. Worst case the PR can go right back up with the fixes and additional tests

#115532 is because we're missing the equivalent to this logic from LowerHWIntrinsicGetElement: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/lowerxarch.cpp#L5349-L5352. It should also exist in LowerHWIntrinsicWithElement here: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/lowerxarch.cpp#L5670-L5671. The general premise is that we insert a bounds check node for anything out of range but since we now carry these nodes all the way through to codegen to support non-constants, we can encounter something which became constant but was definitely out of range and which wasn't removed as dead code. Normalizing the imm8 to always be in range in lowering then avoids this issue and won't execute due to the bounds check failing.

#115487, #115459, and #115644 are because the following logic is using ins_Move_Extend when it should be using ins_Store instead: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/hwintrinsiccodegenxarch.cpp#L2008

[tannergooding]

I have a fix up for those issues here: #115648

[jakobbotsch]

@tannergooding It's great if you have an actual fix. I was mainly interested in resolving the blocked CI the quickest way possible.

[tannergooding]

I ended up finding some issues with Mono not handling this scenario as well. Working through fixing those but going to merge this to unblock CI and will update my pr to include the WithElement improvement again