Math.SinCos can be slower than Sin + Cos in some scenarios.

[tannergooding]

The codegen for Math.SinCos can currently be problematic on GCC and MSVC leading to decreased perf.

https://godbolt.org/z/K5dad3 shows the codegen for:

#include <math.h>

#ifdef _MSC_VER
#pragma float_control(precise, off)
#endif

void SinCos1(float x, float* pSin, float* pCos)
{
#ifdef _MSC_VER
    *pSin = sinf(x);
    *pCos = cosf(x);
#else
    sincosf(x, pSin, pCos);
#endif
}

In particular, GCC and MSVC end up with many shuffles between registers and to/from the stack causing a noticeable perf slowdown across hundreds of calls.

https://godbolt.org/z/sG6dTW shows comparison of several other possible variants, but none is "ideal" across all 3 compilers. We should investigate this further to determine what can be done to ensure that SinCos is at least as fast as calling Sin and Cos independently, possibly following up with the MSVC team to improve the codegen here.

category:performance
theme:expression-opts

[tannergooding]

This was reported by a customer in the .NET 6 Preview 1 blogpost here: https://devblogs.microsoft.com/dotnet/announcing-net-6-preview-1/comment-page-6/#comment-8548

[echesakov]

I was thinking how to mitigate the issue and one of the ideas was to have two separate calls to Math.Sin and Math.Cos (under TARGET_WINDOWS) in managed implementation of Math.SinCos in S.P.CoreLib to avoid the issue with MSVC codegen.

Unfortunately, such approach doesn't improve the situation much.

As an example, this is what Math.SinCos would have looked if we had that implementation instead of a native call to SinCos(double, double*, double*)

G_M57534_IG01:              ;; offset=0000H
       56                   push     rsi
       4883EC30             sub      rsp, 48
       C5F877               vzeroupper 
       488BF1               mov      rsi, rcx
       C5F828C1             vmovaps  xmm0, xmm1
						;; bbWeight=1    PerfScore 2.75
G_M57534_IG02:              ;; offset=000FH
       C5FB11442448         vmovsd   qword ptr [rsp+48H], xmm0
       E8E6CBB35F           call     System.Math:Sin(double):double
       C5FB11442428         vmovsd   qword ptr [rsp+28H], xmm0
       C5FB10442448         vmovsd   xmm0, qword ptr [rsp+48H]
       E815C0B35F           call     System.Math:Cos(double):double
       C5FB104C2428         vmovsd   xmm1, qword ptr [rsp+28H]
       C5FB110E             vmovsd   qword ptr [rsi], xmm1
       C5FB114608           vmovsd   qword ptr [rsi+8], xmm0
       488BC6               mov      rax, rsi
						;; bbWeight=1    PerfScore 8.25
G_M57534_IG03:              ;; offset=003DH
       4883C430             add      rsp, 48
       5E                   pop      rsi
       C3                   ret      

[tannergooding]

I was thinking how to mitigate the issue and one of the ideas was to have two separate calls to Math.Sin and Math.Cos (under TARGET_WINDOWS) in managed implementation of Math.SinCos in S.P.CoreLib to avoid the issue with MSVC codegen.

The two calls in C++ get intrinsified to a call to the internal __sincos helper method which does "less work". I think the issue is really due to the shuffling that is happening since C++ has to assume its being used a particular way that doesn't match up with the intrinsic signature (and which normally gets optimized out in "real world code" where its used as part of a larger algorithm, not via a "simple" wrapper method that can't be inlined).

[echesakov]

Yes, I understood this part. I was more thinking what could be done now (in 6.0.0) to mitigate that problem.

@tannergooding Do you know if the helper can be called directly? It looks that it has some "custom ABI" - the values of both sin and cos returned as components of xmm0 register. As an idea, we could introduce a "private" hardware intrinsic on win-x64 Vector128<double> SinCos(double) (that would unfold into a call to the helper), then we could use the intrinsic in the managed implementation of Math.SinCos. Basically, moving the unpacking of the values from xmm0 into managed code.

[echesakov]

We can tweak the native helper signature - only return a value of cos(x) by reference.

float SinCos(float x, float* pCos)
{
    *pCos = cosf(x);
    return sinf(x);
}

double SinCos(double x, double* pCos)
{
    *pCos = cos(x);
    return sin(x);
}

The resulting codegen for the helper is smaller https://godbolt.org/z/Ws7GPen8f on MSVC while it still uses the "faster" sincos. I would also expect some improvements in the codegen for managed portion of Math.SinCos.

[echesakov]

Chatted with Tanner offline and we both agreed that this will move to 7.0.0.