Repository navigation
Numerical < and == comparisons aren't fused into <= #78385
Description
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: retwhereas 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: retSimilarly 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
Activity
EgorBo commentedon Nov 15, 2022
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 commentedon Nov 15, 2022
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 commentedon Nov 15, 2022
Similar to #61940 in case if @drewnoakes is interested in extending that to this case
drewnoakes commentedon Nov 15, 2022
Pinging @pedrobsaila who provided the implementation for that issue.
EgorBo commentedon Nov 15, 2022
Ah, oops, pinged author of the issue instead of author of the PR 😄
pedrobsaila commentedon Nov 15, 2022
Interested :) You can assign the issue to me