[arm64] Emit cmlt/cmle/fcmlt/fcmle for compares against zero#129819
[arm64] Emit cmlt/cmle/fcmlt/fcmle for compares against zero#129819AndyAyersMS wants to merge 2 commits into
Conversation
Contain a zero vector operand for CompareLessThan/CompareLessThanOrEqual so the JIT emits the 'less than [or equal to] zero' forms instead of materializing zero and swapping operands. Fixes dotnet#64785. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@dhartglassMSFT PTAL (I'm just scanning for "old, neglected" arm64 cq issues) |
There was a problem hiding this comment.
Pull request overview
This PR updates ARM64 HWIntrinsic lowering/codegen so AdvSimd.CompareLessThan* and CompareLessThanOrEqual* against a zero vector can be represented with a contained zero operand and emitted using the dedicated “compare vs zero” instruction forms (cmlt, cmle, fcmlt, fcmle) rather than materializing a zero vector and swapping operands.
Changes:
- Add containment support for
CompareLessThan*/CompareLessThanOrEqual*whenop2is a zero vector (excluding unsigned base types). - Teach ARM64 HWIntrinsic codegen to select
cmlt/cmle/fcmlt/fcmlewhen the second operand is a contained zero. - Extend the existing
Runtime_33972JIT regression test with ARM64 codegen-check coverage and runtime validation for representative vector and scalar cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/lowerarmarch.cpp | Adds containment for zero op2 on less-than / less-than-or-equal AdvSimd intrinsics (signed-only). |
| src/coreclr/jit/hwintrinsiclistarm64.h | Marks the relevant intrinsics as supporting containment / constant-prop to enable the new lowering path. |
| src/coreclr/jit/hwintrinsiccodegenarm64.cpp | Emits cmlt/cmle/fcmlt/fcmle when op2 is a contained zero; otherwise preserves the prior operand-swap emission. |
| src/tests/JIT/Regression/JitBlue/Runtime_33972/Runtime_33972.cs | Adds codegen-check assertions and functional validation for the new zero-compare instruction selection. |
|
|
||
| // CompareLessThan | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] |
There was a problem hiding this comment.
Nit, could add a testcase for
- CompareLessthan Vector128 with signed-byte types
- A negative test for CompareLessthan with an unsigned type, since lowering has that extra check in it
(I'm not able to "at-copilot" for this PR)
There was a problem hiding this comment.
Good call — added both in 6b0511d:
AdvSimd_CompareLessThan_Vector128_SByte_Zero(checkscmlt v0.16b, v0.16b, #0)AdvSimd_CompareLessThan_Vector128_Byte_Zero, anARM64-NOT: cmltnegative test for the unsigned path (it stays uncontained and folds tomovi #0, since unsigned< 0is always false).
Both pass locally. Mind re-approving when you get a chance?
Note
This reply was generated with the assistance of GitHub Copilot.
|
Change LGTM aside from extra test or two we might want. |
Cover CompareLessThan Vector128<sbyte> (cmlt .16b) and a negative test for the unsigned path, which is excluded from containment and so emits no cmlt. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contain a zero vector operand for
CompareLessThan/CompareLessThanOrEqualso the JIT emits the "less than [or equal to] zero" instruction forms
(
cmlt,cmle,fcmlt,fcmle) instead of materializing a zero vector andswapping the operands.
Before:
After:
These instructions were defined in the emitter but never selected in real
codegen. This mirrors the existing containment for
CompareGreaterThan/CompareGreaterThanOrEqual/CompareEqual. Unsigned base types are excludedsince the
cmlt/cmle"with zero" forms are signed-only.Codegen-check coverage is added to the existing
Runtime_33972test for thevector and scalar int32/int64/float/double cases.
Fixes #64785.
Note
This pull request was generated with the assistance of GitHub Copilot.