Numerical < and == comparisons aren't fused into <=

[stephentoub]

Repro:
SharpLab

#nullable disable
using System.Numerics;
using SharpLab.Runtime;

public class C
{
    [JitGeneric(typeof(int))]
    public static bool M1<T>(T value) where T : INumberBase<T> =>
        T.IsNegative(value) || T.IsZero(value);
    
    public static bool M2(int value) =>
        value < 0 || value == 0;
    
    [JitGeneric(typeof(int))]
    public static bool M3<T>(T value) where T : INumberBase<T>, IComparisonOperators<T,T,bool> =>
        value <= T.Zero;
    
    public static bool M4(int value) =>
        value <= 0;
}

Both M1 (generic) and M2 (non-generic) that do separate negative and zero checks produce:

    L0000: test ecx, ecx
    L0002: jl short L000c
    L0004: xor eax, eax
    L0006: test ecx, ecx
    L0008: sete al
    L000b: ret
    L000c: mov eax, 1
    L0011: ret

whereas both M3 (generic) and M4 (non-generic) that do combined negative and zero checks produce:

    L0000: xor eax, eax
    L0002: test ecx, ecx
    L0004: setle al
    L0007: ret

Similarly for >=.

The approved design of #78222 was based on the idea that these would be the same, and thus the generic methods there could constrain T to just being INumberBase<T> and not require IComparisonOperators<T,T,bool> as well, but if it's going to remain producing worse code, we should reconsider.

cc: @EgorBo, @tannergooding

[ghost]

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

Issue Details

---

Repro:
SharpLab

#nullable disable
using System.Numerics;
using SharpLab.Runtime;

public class C
{
    [JitGeneric(typeof(int))]
    public static bool M1<T>(T value) where T : INumberBase<T> =>
        T.IsNegative(value) || T.IsZero(value);
    
    public static bool M2(int value) =>
        value < 0 || value == 0;
    
    [JitGeneric(typeof(int))]
    public static bool M3<T>(T value) where T : INumberBase<T>, IComparisonOperators<T,T,bool> =>
        value <= T.Zero;
    
    public static bool M4(int value) =>
        value <= 0;
}

Both M1 (generic) and M2 (non-generic) that do separate negative and zero checks produce:

    L0000: test ecx, ecx
    L0002: jl short L000c
    L0004: xor eax, eax
    L0006: test ecx, ecx
    L0008: sete al
    L000b: ret
    L000c: mov eax, 1
    L0011: ret

whereas both M3 (generic) and M4 (non-generic) that do combined negative and zero checks produce:

    L0000: xor eax, eax
    L0002: test ecx, ecx
    L0004: setle al
    L0007: ret

Similarly for >=.

The approved design of #78222 was based on the idea that these would be the same, and thus the generic methods there could constrain T to just being INumberBase<T> and not require IComparisonOperators<T,T,bool> as well, but if it's going to remain producing worse code, we should reconsider.

cc: @EgorBo, @tannergooding

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

[EgorBo]

I have a branch that does this, it's just that I didn't see a lot of diffs from it and gave up, might give it a try again.
This might also be handled in optOptimizeBools. It looks similar to what we landed there recently by some community PR.

[stephentoub]

I didn't see a lot of diffs from it

I expect that will change once #78222 happens and rolls out. :)

It's good to know this is fixable such that we don't need to change the API shape.

[EgorBo]

Similar to #61940 in case if @drewnoakes is interested in extending that to this case

[drewnoakes]

Pinging @pedrobsaila who provided the implementation for that issue.

[EgorBo]

Ah, oops, pinged author of the issue instead of author of the PR 😄

[pedrobsaila]

Interested :) You can assign the issue to me