Workaround an MSVC bug with __libm_sse2_sincos_ (#98207)

* Workaround an MSVC bug with __libm_sse2_sincos_ * Add the SinCos regression tests to the existing Math/MathF tests * Ensure the workaround also applies to x86 * Allow a larger amount of variance due to x86 Windows * Adjust the allowedVarianceCos for x86 * Add a link to the MSVC issue * Add a link to the MSVC issue
dotnet · Feb 10, 2024 · dfcbcb4 · dfcbcb4
1 parent 4acd106
commit dfcbcb4
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 0 deletions.
diff --git a/src/coreclr/classlibnative/float/floatdouble.cpp b/src/coreclr/classlibnative/float/floatdouble.cpp
@@ -253,6 +253,13 @@ FCIMPL1_V(double, COMDouble::Sin, double x)
  return sin(x);
 FCIMPLEND
 
+#if defined(_MSC_VER)
+// The /fp:fast form of `sincos` for xarch returns sin twice, rather than sincos
+// https://developercommunity.visualstudio.com/t/MSVCs-sincos-implementation-is-incorrec/10582378
+#pragma float_control(push)
+#pragma float_control(precise, on)
+#endif
+
 /*====================================SinCos====================================
 **
 ==============================================================================*/
@@ -270,6 +277,10 @@ FCIMPL3_VII(void, COMDouble::SinCos, double x, double* pSin, double* pCos)
 
 FCIMPLEND
 
+#if defined(_MSC_VER)
+#pragma float_control(pop)
+#endif
+
 /*=====================================Sinh=====================================
 **
 ==============================================================================*/

diff --git a/src/coreclr/classlibnative/float/floatsingle.cpp b/src/coreclr/classlibnative/float/floatsingle.cpp
@@ -228,6 +228,13 @@ FCIMPL1_V(float, COMSingle::Sin, float x)
  return sinf(x);
 FCIMPLEND
 
+#if defined(_MSC_VER)
+// The /fp:fast form of `sincos` for xarch returns sin twice, rather than sincos
+// https://developercommunity.visualstudio.com/t/MSVCs-sincos-implementation-is-incorrec/10582378
+#pragma float_control(push)
+#pragma float_control(precise, on)
+#endif
+
 /*====================================SinCos====================================
 **
 ==============================================================================*/
@@ -245,6 +252,10 @@ FCIMPL3_VII(void, COMSingle::SinCos, float x, float* pSin, float* pCos)
 
 FCIMPLEND
 
+#if defined(_MSC_VER)
+#pragma float_control(pop)
+#endif
+
 /*=====================================Sinh=====================================
 **
 ==============================================================================*/

diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/Math.cs b/src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/Math.cs
@@ -1497,6 +1497,7 @@ public static void Sin(double value, double expectedResult, double allowedVarian
 
  [Theory]
  [InlineData( double.NegativeInfinity, double.NaN, double.NaN, 0.0, 0.0)]
+ [InlineData(-1e18, 0.9929693207404051, 0.11837199021871073, 0.0002, 0.002)] // https:/dotnet/runtime/issues/98204
  [InlineData(-3.1415926535897932, -0.0, -1.0, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon * 10)] // value: -(pi)
  [InlineData(-2.7182818284590452, -0.41078129050290870, -0.91173391478696510, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // value: -(e)
  [InlineData(-2.3025850929940457, -0.74398033695749319, -0.66820151019031295, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // value: -(ln(10))
@@ -1528,6 +1529,7 @@ public static void Sin(double value, double expectedResult, double allowedVarian
  [InlineData( 2.3025850929940457, 0.74398033695749319, -0.66820151019031295, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // value: (ln(10))
  [InlineData( 2.7182818284590452, 0.41078129050290870, -0.91173391478696510, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // value: (e)
  [InlineData( 3.1415926535897932, 0.0, -1.0, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon * 10)] // value: (pi)
+ [InlineData( 1e18, -0.9929693207404051, 0.11837199021871073, 0.0002, 0.002)] // https:/dotnet/runtime/issues/98204
  [InlineData( double.PositiveInfinity, double.NaN, double.NaN, 0.0, 0.0)]
  public static void SinCos(double value, double expectedResultSin, double expectedResultCos, double allowedVarianceSin, double allowedVarianceCos)
  {

diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/MathF.cs b/src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/MathF.cs
@@ -1677,6 +1677,7 @@ public static void Sin(float value, float expectedResult, float allowedVariance)
 
  [Theory]
  [InlineData( float.NegativeInfinity, float.NaN, float.NaN, 0.0f, 0.0f)]
+ [InlineData(-1e8f, -0.931639, -0.36338508, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // https:/dotnet/runtime/issues/98204
  [InlineData(-3.14159265f, -0.0f, -1.0f, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon * 10)] // value: -(pi)
  [InlineData(-2.71828183f, -0.410781291f, -0.911733918f, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // value: -(e)
  [InlineData(-2.30258509f, -0.743980337f, -0.668201510f, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // value: -(ln(10))
@@ -1708,6 +1709,7 @@ public static void Sin(float value, float expectedResult, float allowedVariance)
  [InlineData( 2.30258509f, 0.743980337f, -0.668201510f, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // value: (ln(10))
  [InlineData( 2.71828183f, 0.410781291f, -0.911733918f, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // value: (e)
  [InlineData( 3.14159265f, 0.0f, -1.0f, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon * 10)] // value: (pi)
+ [InlineData( 1e8f, 0.931639, -0.36338508, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // https:/dotnet/runtime/issues/98204
  [InlineData( float.PositiveInfinity, float.NaN, float.NaN, 0.0f, 0.0f)]
  public static void SinCos(float value, float expectedResultSin, float expectedResultCos, float allowedVarianceSin, float allowedVarianceCos)
  {