Replace the last two SIMDIntrinsic in LIR with NamedIntrinsic and delete GT_SIMD

[tannergooding]

As per the title this moves the last two remaining SIMDIntrinsic cases to be NamedIntrinsic and thus allows us to remove GT_SIMD.

Unlike most of the other SIMD intrinsics, UpperSave and UpperRestore are "special" and exist where general SIMD support does for ABI reasons. However, GT_INTRINSIC is a large node and has additional cost that GT_SIMD did not. Because of this, we use GT_HWINTRINSIC when it is available and only fallback to GT_INTRINSIC when it is not.

It's possible we could merge FEATURE_SIMD and FEATURE_HW_INTRINSIC together, given how tightly coupled they otherwise are, but that would be a more complex work item and should be handled separately if at all. Doing so would allow us to always use GT_HWINTRINSIC.

Likewise, we have both GenTreeIntrinsic (GT_INTRINSIC) and GenTreeJitIntrinsic (which is only GenTreeHWIntrinsic/GT_HWINTRINSIC now) and these should probably be modified a bit to have a common base, but that is a larger refactoring and should be done separately if at all. -- It's a bit more complicated since moving fields around adds padding between the fields of the base and derived types and that can push things to become a large node unnecessarily.

[ghost]

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

Issue Details

---

As per the title this moves the last two remaining SIMDIntrinsic cases to be NamedIntrinsic and thus allows us to remove GT_SIMD.

Unlike most of the other SIMD intrinsics, UpperSave and UpperRestore are "special" and exist where general SIMD support does for ABI reasons.

It's possible we could merge FEATURE_SIMD and FEATURE_HW_INTRINSIC together, given how tightly coupled they otherwise are, but that would be a more complex work item and should be handled separately if at all.

Likewise, we have both GenTreeIntrinsic (GT_INTRINSIC) and GenTreeJitIntrinsic (which is only GenTreeHWIntrinsic/GT_HWINTRINSIC now) and these should probably be modified a bit to have a common base, but that is a larger refactoring and should be done separately if at all.

Author:	tannergooding
Assignees:	tannergooding
Labels:	area-CodeGen-coreclr
Milestone:	-

[tannergooding]

simdcodegenxarch.cpp is now "unique" and should probably just be merged into codegenxarch.cpp, just like the related code already is for Arm64 (there probably isn't "enough" here to warrant it being in its own separate file anymore)..

[tannergooding]

CC. @dotnet/jit-contrib. This should be ready for review, no diffs.

Probably need to determine why arm64 shows a TP regression (it is a TP improvement on x86/x64). Arm64 was triggering an assert related to gtSetEvalOrder so its possible something subtly different being caused there...

If I had to guess it has to do with the old logic setting SetOpLclRelatedToSIMDIntrinsic(op1); (via gtNewSIMDNode), but I'll take a closer look.

[tannergooding]

Hmmm, the Arm64 TP regression looks to actually be because GT_SIMD was a small node but GT_INTRINSIC is "large". Arm64 ends up getting quite a bit more UpperSave/Restore happening than x64 and that is impactful.

[tannergooding]

Not sure what's causing the TP regression for Arm64 after all.

There remained a 0.01% TP regression even after adjusting to use GT_HWINTRINSIC so we had a small node. Nothing else obvious pops out from the inscount trace and we are, by all means, executing less code on many core paths now regardless.

Likely still worth taking given the TP improvement for x86/x64, the removal of an unnecessary node type, and general simplification in the JIT.

[tannergooding]

/azp run runtime-coreclr jitstress-isas-x86, runtime-coreclr jitstress-isas-arm, runtime-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jakobbotsch]

LGTM, nice to see all the red!

[kunalspathak]

Not sure what's causing the TP regression for Arm64 after all.

Yes, that is little puzzling, and it is consistent across OS. Did you try running TP manually to see which method has more instructions?

nice to see all the red!

Indeed.

[tannergooding]

Yes, that is little puzzling, and it is consistent across OS. Did you try running TP manually to see which method has more instructions?

Yes, but unfortunately the change in node type makes this difficult to determine.

There are many functions where the ins_count has changed (some positive, some negative). The numbers were overall similar to x64, but in larger amounts since Arm64 creates more UpperSave/Restore nodes.

[tannergooding]

Single stress failure is unrelated: #75244