JIT can reuse a comparison between two locals but fails to do so for two readonly fields

[Jorenkv]

Description

For the snippet below, MinMaxA has worse codegen than MinMaxB:

using System;

readonly struct Pair
{
    readonly int U, V;
    
    int MinMaxA() {
        var min = Math.Min(U, V);
        var max = Math.Max(U, V);
        return max ^ min;
    }
    
    int MinMaxB() {
        var (u, v) = (U, V); // Inserting a manual copy to locals improves the codegen
        var min = Math.Min(u, v);
        var max = Math.Max(u, v);
        return max ^ min;
    }
}

; Core CLR 8.0.23.53103 on x64

Pair.MinMaxA()
    L0000: mov eax, [rcx]
    L0002: mov edx, eax
    L0004: mov ecx, [rcx+4]
    L0007: mov r8d, ecx
    L000a: cmp edx, r8d
    L000d: cmovg edx, r8d
    L0011: cmp eax, ecx
    L0013: cmovl eax, ecx
    L0016: xor eax, edx
    L0018: ret

Pair.MinMaxB()
    L0000: mov eax, [rcx]
    L0002: mov ecx, [rcx+4]
    L0005: cmp eax, ecx
    L0007: mov edx, ecx
    L0009: cmovle edx, eax
    L000c: cmovl eax, ecx
    L000f: xor eax, edx
    L0011: ret

In MinMaxA, the JIT performs the comparison between the fields twice, while in MinMaxB it is able to see that the comparison between two locals only needs to happen once.

Configuration

Locally I'm running 8.0.100, Windows x64. The above disasm I took from Sharplab.

[ghost]

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

Issue Details

---

Description

For the snippet below, MinMaxA has worse codegen than MinMaxB:

using System;

readonly struct Pair
{
    readonly int U, V;
    
    int MinMaxA() {
        var min = Math.Min(U, V);
        var max = Math.Max(U, V);
        return max ^ min;
    }
    
    int MinMaxB() {
        var (u, v) = (U, V); // Inserting a manual copy to locals improves the codegen
        var min = Math.Min(u, v);
        var max = Math.Max(u, v);
        return max ^ min;
    }
}

; Core CLR 8.0.23.53103 on x64

Pair.MinMaxA()
    L0000: mov eax, [rcx]
    L0002: mov edx, eax
    L0004: mov ecx, [rcx+4]
    L0007: mov r8d, ecx
    L000a: cmp edx, r8d
    L000d: cmovg edx, r8d
    L0011: cmp eax, ecx
    L0013: cmovl eax, ecx
    L0016: xor eax, edx
    L0018: ret

Pair.MinMaxB()
    L0000: mov eax, [rcx]
    L0002: mov ecx, [rcx+4]
    L0005: cmp eax, ecx
    L0007: mov edx, ecx
    L0009: cmovle edx, eax
    L000c: cmovl eax, ecx
    L000f: xor eax, edx
    L0011: ret

In MinMaxA, the JIT performs the comparison between the fields twice, while in MinMaxB it is able to see that the comparison between two locals only needs to happen once.

Configuration

Locally I'm running 8.0.100, Windows x64. The above disasm I took from Sharplab.

Author:	Jorenkv
Assignees:	-
Labels:	tenet-performance, area-CodeGen-coreclr
Milestone:	-

[hez2010]

This is expected as a (non-static) readonly field is mutable at runtime.

[huoyaoyuan]

This is expected as a (non-static) readonly field is mutable at runtime.

No, there's no duplicated reads in generated code. Look at the cmp edx, r8d and cmp eax, ecx instructions.

[jakobbotsch]

We only get rid of the duplicated loads after CSE, but then we still have multiple locals that we'd need to copy propagate, but copy prop runs before CSE. We do end up with identical codegen if we run copy prop again after CSE (with DOTNET_JitOptRepeat=* in checked builds).