Idiomatic implementation of operator== (and equality) should generate better codegen

[ahsonkhan]

Let's say I have a class with a single field. It would be great if the JIT could take the naiive/idiomatic C# implementation of Equals/==/!= operators that handle nulls correctly and produce better codegen to avoid using "tricks" and workarounds.

public class Foo
{
    int _field;

    public Foo(int field)
    {
        _field = field;
    }

    public override bool Equals(object obj)
        => obj is Foo otherVersion && Equals(otherVersion);

    public bool Equals(Foo other)
    {
        if (other == null)
        {
            return false;
        }
        return _field == other._field;
    }

    public override int GetHashCode() => _field;

    // This is what I want to be able to write (or something similar).
    public static bool operator ==(Foo left, Foo right)
        => left?._field == right?._field;
    
    // This produces much better codegen but requires contortions
    public static bool operator ==(Foo left, Foo right)
    {
        // Test "right" first to allow branch elimination when inlined for null checks (== null) 
        // so it can become a simple test 
        if (right is null)
        {
            // return true/false not the test result https://github.com/dotnet/coreclr/issues/914 
            return (left is null) ? true : false;
        }

        return right.Equals(left);
    }

    public static bool operator !=(Foo left, Foo right)
        => !(left == right);
}

sharplab.io

// BEFORE:
Foo.op_Equality(Foo, Foo)
    L0000: test rcx, rcx
    L0003: jnz L000c
    L0005: xor eax, eax
    L0007: xor r8d, r8d
    L000a: jmp L0015
    L000c: mov r8d, [rcx+0x8]
    L0010: mov eax, 0x1
    L0015: test rdx, rdx
    L0018: jnz L0024
    L001a: xor edx, edx
    L001c: movzx ecx, dl
    L001f: xor r9d, r9d
    L0022: jmp L002d
    L0024: mov r9d, [rdx+0x8]
    L0028: mov ecx, 0x1
    L002d: cmp r8d, r9d
    L0030: setz dl
    L0033: movzx edx, dl
    L0036: movzx eax, al
    L0039: movzx ecx, cl
    L003c: cmp eax, ecx
    L003e: setz al
    L0041: movzx eax, al
    L0044: and eax, edx
    L0046: ret

// AFTER:
Foo.op_Equality(Foo, Foo)
    L0000: test rdx, rdx
    L0003: jnz L0013
    L0005: test rcx, rcx
    L0008: jz L000d
    L000a: xor eax, eax
    L000c: ret
    L000d: mov eax, 0x1
    L0012: ret
    L0013: mov [rsp+0x8], rcx
    L0018: mov rcx, rdx
    L001b: mov rdx, [rsp+0x8]
    L0020: mov rax, 0x7ffe6fd40530
    L002a: jmp rax

Got highlighted here: dotnet/corefx#40107 (comment)

We end up doing this in a few places (from @benaadams here dotnet/coreclr#21765). For example:
https://github.com/dotnet/coreclr/blob/4e960d72ad19086a9615229b24b50dbd9ccb143c/src/System.Private.CoreLib/shared/System/Globalization/SortVersion.cs#L65-L78
https://github.com/dotnet/coreclr/blob/4e960d72ad19086a9615229b24b50dbd9ccb143c/src/System.Private.CoreLib/shared/System/Reflection/Assembly.cs#L157-L175

cc @AndyAyersMS, @benaadams, @kasiabulat

category:cq
theme:basic-cq
skill-level:expert
cost:medium
impact:medium