Optimize Math.Max/Min(X, C) in JIT

[EgorBo]

Math.Min and Math.Max are currently quite slow due to IEEE754 corner cases, here is the codegen for Math.Max(double, double) (it's not inlineable):

; Max(double,double):double
G_M17496_IG01:
       sub      rsp, 24
       vzeroupper 
G_M17496_IG02:
       vucomisd xmm0, xmm1
       ja       SHORT G_M17496_IG03
       vmovsd   qword ptr [rsp+10H], xmm0
       mov      rax, qword ptr [rsp+10H]
       mov      rdx, 0xD1FFAB1E
       and      rax, rdx
       mov      rdx, 0xD1FFAB1E
       cmp      rax, rdx
       jle      SHORT G_M17496_IG04
G_M17496_IG03:
       add      rsp, 24
       ret      
G_M17496_IG04:
       vucomisd xmm0, xmm1
       jp       SHORT G_M17496_IG08
       jne      SHORT G_M17496_IG08
       vmovsd   qword ptr [rsp+08H], xmm0
       cmp      qword ptr [rsp+08H], 0
       jl       SHORT G_M17496_IG06
G_M17496_IG05:
       add      rsp, 24
       ret      
G_M17496_IG06:
       vmovaps  xmm0, xmm1
G_M17496_IG07:
       add      rsp, 24
       ret      
G_M17496_IG08:
       vmovaps  xmm0, xmm1
G_M17496_IG09:
       add      rsp, 24
       ret      
; Total bytes of code: 102

The code looks HUGE, especially if you compare it with C/C++'s fmax (I know it doesn't really care about corner cases, e.g. fmax(-0.0, 0.0) => -0.0) but for the case when one of the arguments is a constant value this codegen can be just:

       vmaxsd  xmm0, xmm0, C

and still be IEEE754-2019 compliant (unlike dotnet/coreclr#22965) if C is a "normal" value like 100.0 (we can re-use FloatingPointUtils::isNormal from dotnet/coreclr#24584).
I found plenty of usages of Math.Max(X, C) pattern in open-source projects, e.g.: AvaloniaUI, Xenko.

category:cq
theme:floating-point
skill-level:expert
cost:large

[EgorBo]

A quick benchmark: https://gist.github.com/EgorBo/6409a69f57a7def02da46eebb7431346 (Core i7 8700K)

Method	Mean	Error	StdDev	Ratio
MathMax	19.876 us	0.1273 us	0.1190 us	1.00
Ternary	13.495 us	0.0802 us	0.0750 us	0.68
vmaxsd	9.799 us	0.1276 us	0.1193 us	0.49

[tannergooding]

still be IEEE 754-2019 compliant (unlike dotnet/coreclr#22965)

dotnet/coreclr#22965 was and is IEEE 754-2019 compliant. In IEEE 754-2008, the spec around minNum/maxNum was loosely defined for +/-0, but it required that NaN not be propagated.

In IEEE 754-2019, minNum/maxNum were changed from "required operations" to "recommended operations". They were also renamed to minimumNumber and maximumNumber respectively. They have the same behavior as IEEE 754-2008 but strictly define the behavior for +/-0. Additionally, the spec now defines and recommends minimum and maximum which have the same behavior but which propagate NaN inputs (matching the behavior that WPF expects and that .NET was exposing as part of Math.Min and Math.Max).

For .NET 5, I am still going to be proposing we expose the MinNum and MaxNum functions to ensure we are supporting both sides of the IEEE 754-2019 recommendation.

[tannergooding]

A PR similar to dotnet/coreclr#22965 should improve perf while still maintaining the NaN propagation. This should improve the perf of the case where neither input is constant.

[EgorBo]

@tannergooding thank you for the explanation, so will dotnet/coreclr#22965 fix this constant case? I see the method is marked with AggressiveInlining but I guess the const input will still be checked for IsNaN etc, am I right?

[tannergooding]

It would not. The JIT doesn't currently understand HWIntrinsics for the purposes of CSE.

[am11]

With C++' std::min(int, int):

#include <algorithm>
int GetMin(int a, int b)
{  
  return std::min(a, b);
}

clang++ 11 produces

mov eax, edi
cmp esi, edi
cmovl eax, esi
ret

with C#'s Math.Min(int, int):

static int GetMin(int a, int b) => System.Math.Min(a, b);

RyuJIT generates:

cmp ecx, edx
jle short L0006
jmp short L0008
mov edx, ecx
mov eax, edx
ret

can we also replace jump with cmovl for integer overload?

[EgorBo]

@am11 this issue is about the floating point Max/Min versions 🙂
for the int overload and cmov there is an on-going discussion in #32716 (comment)

[am11]

Thanks, I will follow the discussion. (title of this issue is bit /fp:permissive 😉)
Side note: clang++ 10 emits cmovle in the codegen shown above, while clang++ 11 emits cmovl, so there might be some corner case fixed (or regressed) due to that.