Workaround an MSVC bug with __libm_sse2_sincos_

[tannergooding]

This resolves #98204

There is currently a bug in MSVCRT where __libm_sse2_sincos (which is used by fp:fast) is trying to rely on the fact that sin(x + PI / 2) == cos(x) and so calls into __vdecl_sin2 when it should instead call __vdecl_sincos2. This causes it to return sin for both results given a large enough input, which leads to incorrect results.

[ghost]

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

Issue Details

---

This resolves #98204

There is currently a bug in MSVCRT where __libm_sse2_sincos (which is used by fp:fast) calls __vdecl_sin2 when it should instead call __vdecl_sincos2. This causes it to return sin for both results, which leads to incorrect results.

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

[tannergooding]

CC. @jkoritzinsky and @jkotas, this will somewhat conflict with #98048.

I think it'd be good to get this in first as it likely needs backporting to .NET 8 (unintentional regression not caused by us, in API surface that's existed since .NET 6).

[jkotas]

There is currently a bug in MSVCRT where __libm_sse2_sincos (which is used by fp:fast) calls __vdecl_sin2 when it should instead call __vdecl_sincos2. This causes it to return sin for both results, which leads to incorrect results.

Why was this bug missed by our existing SinCos tests?

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This resolves #98204

There is currently a bug in MSVCRT where __libm_sse2_sincos (which is used by fp:fast) calls __vdecl_sin2 when it should instead call __vdecl_sincos2. This causes it to return sin for both results, which leads to incorrect results.

Author:	tannergooding
Assignees:	tannergooding
Labels:	area-System.Numerics, area-CodeGen-coreclr
Milestone:	-

[stephentoub]

customer reported

I'm not exactly "customer" :)

[tannergooding]

Why was this bug missed by our existing SinCos tests?

It looks like __libm_sse2_sincos is trying to rely on the fact that sin(x + PI / 2) == cos(x) and so adjusts the second input by that much. This is correct for infinitely precise arithmetic, but for floating-point means that cos is less accurate than it should be for regular values and will not work at all for large values due to the increasing gap between which numbers are representable (it breaks down entirely around 1e16, but starts showing issues far earlier).

Given that its just calling into SVML and SVML provides a dedicated __vdecl_sincos2, that should really be getting used instead.

We missed this because we were testing normal input ranges and not testing anything quite so large, effectively relying on the fact that the underlying libm implementation is expected to be largely correct, and that we simply want to minimally validate the bindings are accurate and that it matches any IEEE 754 required behavior (such as sin(+0) == +0 and sin(-0) == -0, with no exception)

[stephentoub]

For reference, I found it because I'd implemented a vectorized TensorPrimitives.SinCos and our tests that validate it against the scalar T.SinCos were failing a bunch of tests where we do exercise a wide range of values.

[jkotas]

Thanks

[stephentoub]

thanks