Improve "vec == Vector128<>.Zero"

[EgorBo]

Follow up to #75864 to address @TamarChristinaArm's suggestion in #75849 (comment)
Btw, previous improvements seem to show nice benefits #75864 (comment)

static bool IsZero1(Vector128<int> v) => v == Vector128<int>.Zero;
static bool IsZero2(Vector64<int> v) => v == Vector64<int>.Zero;

Codegen diff:

; Method Tests:IsZero1
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp

-           umaxv   s16, v0.4s
-           umov    w0, v16.s[0]
-           cmp     w0, #0
+           umaxp   v16.4s, v0.4s, v0.4s
+           umov    x0, v16.d[0]
+           cmp     x0, #0
            cset    x0, eq

            ldp     fp, lr, [sp], #0x10
            ret     lr

; Method Tests:IsZero2
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp

-           umaxv   h16, v0.4h
-           umov    w0, v16.s[0]
-           cmp     w0, #0
+           umov    x0, v0.d[0]
+           cmp     x0, #0
            cset    x0, eq

            ldp     fp, lr, [sp], #0x10
            ret     lr

Should be a nice win for Vector64. For Vector128 I wasn't able to see noticeable improvements on my Apple M1 but we might see improvements on other (hopefully, on Ampere Altra?)

However, even on M1 it seems to be better:

UMAXV                                     3          0.25     1        -     -     1     u11-14
UMAXP                                     2          0.25     1        -     -     1     u11-14

(1st column is Latency, according to https://dougallj.github.io/applecpu/firestorm-simd.html)

[ghost]

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

Issue Details

---

Follow up to #75864 to address @TamarChristinaArm's suggestion in #75849 (comment)

static bool IsZero1(Vector128<int> v) => v == Vector128<int>.Zero;
static bool IsZero2(Vector64<int> v) => v == Vector64<int>.Zero;

Codegen diff:

; Method Tests:IsZero1
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp

-           umaxv   s16, v0.4s
-           umov    w0, v16.s[0]
-           cmp     w0, #0
+           umaxp   v16.4s, v0.4s, v0.4s
+           umov    x0, v16.d[0]
+           cmp     x0, #0
            cset    x0, eq

            ldp     fp, lr, [sp], #0x10
            ret     lr

; Method Tests:IsZero2
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp

-           umaxv   h16, v0.4h
-           umov    w0, v16.s[0]
-           cmp     w0, #0
+           umov    x0, v0.d[0]
+           cmp     x0, #0
            cset    x0, eq

            ldp     fp, lr, [sp], #0x10
            ret     lr

Should be a nice win for Vector64. For Vector128 I wasn't able to see noticeable improvements on my Apple M1 but we might see improvements on other (hopefully, on Ampere Altra?)

However, even on M1 it seems to be better:

UMAXV                                     3          0.25     1        -     -     1     u11-14
UMAXP                                     2          0.25     1        -     -     1     u11-14

(1st column is Latency, according to https://dougallj.github.io/applecpu/firestorm-simd.html)

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

[EgorBo]

Oh, from what I see Latency on Ampere for umaxv is 6 for byte while umaxp for int is 2 so should be visible

[TamarChristinaArm]

Oh, from what I see Latency on Ampere for umaxv is 6 for byte while umaxp for int is 2 so should be visible

The 6 cycles is I believe only for bytes, the zero special case I don't expect the same dramatic difference as it's reducing from .s which are fairly cheap. I only expect a single cycle difference in this case. But it has better throughput so I at least expect more consistent performance.

[EgorBo]

@dotnet/jit-contrib @kunalspathak PTAL, previously the logic was enabled for Vector3 (SIMD12) too and that was wrong, I'll try to come up with a bug repro and backport a quick fix to net7.0

[EgorBo]

@dotnet/jit-contrib @kunalspathak PTAL, previously the logic was enabled for Vector3 (SIMD12) too and that was wrong, I'll try to come up with a bug repro and backport a quick fix to net7.0

Ah, Vector3 is only used for floats so it won't pass the !varIsFloatingPoint so we're fine, but let's keep the check here in case if we introduce Vector3 for other primitives.

[kunalspathak]

Can you update the comment few lines above to have MaxPairWise instead of MaxAcross?

[EgorBo]

Done!

[EgorBo]

Seems to be also beneficial: