Treat Guid and Int128 types as bitwise equatable

[EgorBo]

Treat Guid, Int128, and UInt128 as bitwise equatable, enabling SequenceEqual to use the existing memcmp unrolling. Also correct the integer comparison pseudo-name used by JitDisasm.

void Foo(Guid g1, Guid g2)
{
    if (((Span<Guid>)[g1]).SequenceEqual((Span<Guid>)[g2]))
    {
        Console.WriteLine("Equal");
    }
}

-       lea      rdi, [rbp-0x30]
-       lea      rsi, [rbp-0x40]
-       mov      edx, 1
-       call     [System.SpanHelpers:SequenceEqual[System.Guid](byref,byref,int):bool]
-       test     eax, eax
+       vmovups  xmm0, xmmword ptr [rbp-0x30]
+       vpcmpnequq k1, xmm0, xmmword ptr [rbp-0x40]
+       kortestb k1, k1
+       sete     dil
+       movzx    rdi, dil
+       test     edi, edi

The goal is just to be able to theoretically implement Guid.Equals entirely in 100% memory safe code. It would be nice if C# could allow us to write it like this: [g1].SequenceEqual([g2])

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR updates CoreCLR’s EE-provided intrinsic for RuntimeHelpers.IsBitwiseEquatable<T>() so that Guid is treated as bitwise-equatable, allowing downstream span/sequence equality paths to use memcmp-based optimizations for Guid element comparisons.

Changes:

• Extend the hardcoded “known bitwise-equatable” type allowlist to include System.Guid when the EE substitutes IL for RuntimeHelpers.IsBitwiseEquatable<T>().

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

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

[huoyaoyuan]

I wonder whether we can implement such optimization without explicit JIT support, namely for record struct when every field is bitwise equatable and there's no gap. It may be complex to (recursively) identify the field-wise equation tests.

[jkotas]

I wonder whether we can implement such optimization without explicit JIT support, namely for record struct when every field is bitwise equatable and there's no gap.

Records allow overriding of equality. I do not think you can reliably tell whether the default was overridden without analyzing the IL.

[huoyaoyuan]

Records allow overriding of equality. I do not think you can reliably tell whether the default was overridden without analyzing the IL.

Yes, and that's what makes it hard. The IL has to be analyzed recursively to ensure every field are indeed bitwise equatable.

[EgorBo]

@jkotas anything else needed here?

[jkotas]

Yes, and that's what makes it hard. The IL has to be analyzed recursively to ensure every field are indeed bitwise equatable.

It does not sound particularly hard to me...

[EgorBo]

Yes, and that's what makes it hard. The IL has to be analyzed recursively to ensure every field are indeed bitwise equatable.

It does not sound particularly hard to me...

Technically, JIT could optimize with SIMD too (with some caveats), e.g.

public record Foo(long A, long B, long C, long D);

today emits:

       mov      rcx, qword ptr [rsi+0x08]
       cmp      rcx, qword ptr [rbx+0x08]
       jne      SHORT G_M13556_IG06
       mov      rcx, qword ptr [rsi+0x10]
       cmp      rcx, qword ptr [rbx+0x10]
       jne      SHORT G_M13556_IG06
       mov      rcx, qword ptr [rsi+0x18]
       cmp      rcx, qword ptr [rbx+0x18]
       jne      SHORT G_M13556_IG06
       mov      rcx, qword ptr [rsi+0x20]
       cmp      rcx, qword ptr [rbx+0x20]
       sete     al
       movzx    rax, al

(although, current code is faster if two records have different values in the first field)

[jkotas]

although, current code is faster if two records have different values in the first field

Is this PR regressing Guid performance in some cases then?

[jkotas]

This is performance optimization. It should come with numbers.

[EgorBo]

Is this PR regressing Guid performance in some cases then?

@jkotas um.. not sure how, Guid.Equals is already SIMDified. All it changes is when we call SequenceEqual on a span of Guids with this change we can compare 4 of them at a time (via avx512) while in baseline only 1 at a time with SSE2 (also, with this change, SequenceEqual becomes unrolling friendly when JIT sees the length). I can kick off a EgorBot, but I don't see a scenario it might regress tbh

[EgorBo]

@EgorBot -arm -amd

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

// Case 1: Jit doesn't see the length of the span

public class GuidSequenceEqualBenchmarks
{
    private Guid[] _left = null!;
    private Guid[] _same = null!;
    private Guid[] _completelyDifferent = null!;

    [Params(1, 2, 4, 64)]
    public int Length { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        _left = new Guid[Length];
        _same = new Guid[Length];
        _completelyDifferent = _same.Select(i => new Guid("ffffffff-ffff-ffff-ffff-ffffffffffff")).ToArray();
    }

    [Benchmark]
    public bool Same()
    {
        Span<Guid> left = _left;
        Span<Guid> right = _same;
        return left.SequenceEqual(right);
    }

    [Benchmark]
    public bool CompletelyDifferent()
    {
        Span<Guid> left = _left;
        Span<Guid> right = _completelyDifferent;
        return left.SequenceEqual(right);
    }
}

public class Int128SequenceEqualBenchmarks
{
    private Int128[] _left = null!;
    private Int128[] _same = null!;
    private Int128[] _completelyDifferent = null!;

    [Params(1, 2, 4, 64)]
    public int Length { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        _left = new Int128[Length];
        _same = new Int128[Length];
        _completelyDifferent = _same.Select(i => new Int128(0xffffffffffffffff, 0xffffffffffffffff)).ToArray();
    }

    [Benchmark]
    public bool Same()
    {
        Span<Int128> left = _left;
        Span<Int128> right = _same;
        return left.SequenceEqual(right);
    }

    [Benchmark]
    public bool CompletelyDifferent()
    {
        Span<Int128> left = _left;
        Span<Int128> right = _completelyDifferent;
        return left.SequenceEqual(right);
    }
}

// Case 2: JIT-friendly (JIT sees the length of the span)

public class SingleGuidSequenceEqualBenchmarks
{
    private Guid _left;
    private Guid _same;
    private Guid _completelyDifferent;

    [GlobalSetup]
    public void Setup()
    {
        _left = Guid.Empty;
        _same = Guid.Empty;
        _completelyDifferent = new Guid("ffffffff-ffff-ffff-ffff-ffffffffffff");
    }

    [Benchmark]
    public bool SameGuid()
    {
        Guid g1 = _left;
        Guid g2 = _same;

        return ((Span<Guid>)[g1]).SequenceEqual([g2]);
    }

    [Benchmark]
    public bool CompletelyDifferentGuid()
    {
        Guid g1 = _left;
        Guid g2 = _completelyDifferent;

        return ((Span<Guid>)[g1]).SequenceEqual([g2]);
    }
}

public class SingleInt128SequenceEqualBenchmarks
{
    private Int128 _left;
    private Int128 _same;
    private Int128 _completelyDifferent;

    [GlobalSetup]
    public void Setup()
    {
        _left = 0;
        _same = 0;
        _completelyDifferent = -1;
    }

    [Benchmark]
    public bool SameInt128()
    {
        Int128 i1 = _left;
        Int128 i2 = _same;
        return ((Span<Int128>)[i1]).SequenceEqual([i2]);
    }

    [Benchmark]
    public bool CompletelyDifferentInt128()
    {
        Int128 i1 = _left;
        Int128 i2 = _completelyDifferent;
        return ((Span<Int128>)[i1]).SequenceEqual([i2]);
    }
}

[jkotas]

Do we know how to explain this?

CompletelyDifferentInt128	PR #130644	5.893 ns	0.0294 ns	1.00
CompletelyDifferentInt128	main	1.548 ns	0.0166 ns	0.26

[tannergooding]

It does not sound particularly hard to me...

I've kicked off a local copilot task to look at it. It seems to have done a good job and to have handled the key cases for an MVP at this point. I'm doing some more testing and local review before I'll put up a PR

[EgorBo]

Do we know how to explain this?

CompletelyDifferentInt128 PR #130644 5.893 ns 0.0294 ns 1.00
CompletelyDifferentInt128 main 1.548 ns 0.0166 ns 0.26

Not sure, the codegen for main is:

; Assembly listing for method SingleInt128SequenceEqualBenchmarks:CompletelyDifferentInt128():bool:this (Tier1)
; Emitting BLENDED_CODE for x64 + VEX + EVEX on Windows
; Tier1 code
; optimized code
; optimized using Synthesized PGO
; rsp based frame
; partially interruptible
; with Synthesized PGO: fgCalledCount is 100
; No PGO data
; 1 inlinees with PGO data; 9 single block inlinees; 0 inlinees without PGO data

G_M000_IG01:                ;; offset=0x0000
       sub      rsp, 72
       xor      eax, eax
       mov      qword ptr [rsp+0x28], rax
       vxorps   xmm4, xmm4, xmm4
       vmovdqa  xmmword ptr [rsp+0x30], xmm4
       mov      qword ptr [rsp+0x40], rax
 
G_M000_IG02:                ;; offset=0x001A
       mov      rdx, qword ptr [rcx+0x08]
       mov      r8, qword ptr [rcx+0x10]
       mov      rax, qword ptr [rcx+0x28]
       mov      rcx, qword ptr [rcx+0x30]
       mov      qword ptr [rsp+0x38], rdx
       mov      qword ptr [rsp+0x40], r8
       mov      qword ptr [rsp+0x28], rax
       mov      qword ptr [rsp+0x30], rcx
       lea      rcx, bword ptr [rsp+0x38]
       lea      rdx, bword ptr [rsp+0x28]
       mov      r8d, 1
       call     [System.SpanHelpers:SequenceEqual[System.Int128](byref,byref,int):bool]
       nop      
 
G_M000_IG03:                ;; offset=0x0055
       add      rsp, 72
       ret    

for PR it is:

G_M41373_IG01:  ;; offset=0x0000
       sub      rsp, 40
       xor      eax, eax
       mov      qword ptr [rsp+0x08], rax
       vxorps   xmm4, xmm4, xmm4
       vmovdqa  xmmword ptr [rsp+0x10], xmm4
       mov      qword ptr [rsp+0x20], rax
						;; size=26 bbWeight=1 PerfScore 3.83
G_M41373_IG02:  ;; offset=0x001A
       mov      rax, qword ptr [rcx+0x08]
       mov      rdx, qword ptr [rcx+0x10]
       mov      r8, qword ptr [rcx+0x28]
       mov      rcx, qword ptr [rcx+0x30]
       mov      qword ptr [rsp+0x18], rax
       mov      qword ptr [rsp+0x20], rdx
       mov      qword ptr [rsp+0x08], r8
       mov      qword ptr [rsp+0x10], rcx
       lea      rax, bword ptr [rsp+0x18]
       lea      rcx, bword ptr [rsp+0x08]
       cmp      rax, rcx
       je       SHORT G_M41373_IG04
       vmovups  xmm0, xmmword ptr [rsp+0x18]
       vpcmpeqb k1, xmm0, xmmword ptr [rsp+0x08]
       kortestw k1, k1
       jb       SHORT G_M41373_IG04
       xor      eax, eax
						;; size=76 bbWeight=1 PerfScore 19.50
G_M41373_IG03:  ;; offset=0x0066
       add      rsp, 40
       ret      
						;; size=5 bbWeight=1 PerfScore 1.25
G_M41373_IG04:  ;; offset=0x006B
       mov      eax, 1
       jmp      SHORT G_M41373_IG03
						;; size=7 bbWeight=0 PerfScore 0.00

I'd assume Int128's:

public static bool operator ==(Int128 left, Int128 right) => (left._lower == right._lower) && (left._upper == right._upper);

would be indeed faster than SIMD (potentially, unaligned with a penalty) for case when _lower is different, but in this case we also pay for SequenceEqual and size checks inside it 🤔 (unless it's inlined and my codegen from win-x64 is not the same)

[EgorBo]

@EgorBot -intel -amd -profiler --envvars DOTNET_JitDisasm:CompletelyDifferentInt128

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

public class SingleInt128SequenceEqualBenchmarks
{
    private Int128 _left;
    private Int128 _same;
    private Int128 _completelyDifferent;

    [GlobalSetup]
    public void Setup()
    {
        _left = 0;
        _same = 0;
        _completelyDifferent = -1;
    }

    [Benchmark]
    public bool CompletelyDifferentInt128()
    {
        Int128 i1 = _left;
        Int128 i2 = _completelyDifferent;
        return ((Span<Int128>)[i1]).SequenceEqual([i2]);
    }
}

[EgorBo]

@jkotas so the PR struggles from stall-forwarding as @jakobbotsch suspected offline. We have a legacy struct promotion that we eventually want to remove, e.g. with DOTNET_JitStressModeNames=STRESS_NO_OLD_PROMOTION we end up emitting

       vmovups  xmm0, xmmword ptr [rcx+0x08]
       vmovups  xmm1, xmmword ptr [rcx+0x28]
       vmovups  xmmword ptr [rsp+0x18], xmm0
       vmovups  xmmword ptr [rsp+0x08], xmm1
       vmovups  xmm0, xmmword ptr [rsp+0x18]
       vpcmpnequq k1, xmm0, xmmword ptr [rsp+0x08]
       kortestb k1, k1
       sete     al
       movzx    rax, al

instead of current:

       mov      rax, qword ptr [rcx+0x08]
       mov      rdx, qword ptr [rcx+0x10]
       mov      r8, qword ptr [rcx+0x28]
       mov      rcx, qword ptr [rcx+0x30]
       mov      qword ptr [rsp+0x18], rax
       mov      qword ptr [rsp+0x20], rdx
       mov      qword ptr [rsp+0x08], r8
       mov      qword ptr [rsp+0x10], rcx
       lea      rax, bword ptr [rsp+0x18]
       lea      rcx, bword ptr [rsp+0x08]
       cmp      rax, rcx
       je       SHORT G_M41373_IG04
       vmovups  xmm0, xmmword ptr [rsp+0x18]
       vpcmpeqb k1, xmm0, xmmword ptr [rsp+0x08]
       kortestw k1, k1
       jb       SHORT G_M41373_IG04
       xor      eax, eax

So 2 options:

1. We merge as is and eventually this problem goes away as we remove the legacy struct promoter (that promotes based on metadata fields)
2. We remove Int128/UInt128 from this PR. I do not think Guid can struggle from the same problem (the old promoter would not handle it I guess)

[EgorBo]

I think for most other benchmarks for Int128 PR still makes it faster judging by the results.

[jkotas]

ok