Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround an MSVC bug with __libm_sse2_sincos_ #98207

Merged
merged 7 commits into from
Feb 10, 2024
Merged

Workaround an MSVC bug with __libm_sse2_sincos_ #98207

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8898b20
Workaround an MSVC bug with __libm_sse2_sincos_
tannergooding Feb 9, 2024
475d96c
Add the SinCos regression tests to the existing Math/MathF tests
tannergooding Feb 9, 2024
ab6b2db
Ensure the workaround also applies to x86
tannergooding Feb 9, 2024
2054b91
Allow a larger amount of variance due to x86 Windows
tannergooding Feb 10, 2024
eec93fa
Adjust the allowedVarianceCos for x86
tannergooding Feb 10, 2024
1fb842a
Add a link to the MSVC issue
tannergooding Feb 10, 2024
cdfcb70
Add a link to the MSVC issue
tannergooding Feb 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/coreclr/classlibnative/float/floatdouble.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,12 @@ 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
#pragma float_control(push)
#pragma float_control(precise, on)
#endif

/*====================================SinCos====================================
**
==============================================================================*/
Expand All @@ -268,6 +274,10 @@ FCIMPL3_VII(void, COMDouble::SinCos, double x, double* pSin, double* pCos)

FCIMPLEND

#if defined(_MSC_VER)
#pragma float_control(pop)
#endif

/*=====================================Sinh=====================================
**
==============================================================================*/
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/classlibnative/float/floatsingle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,12 @@ 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
#pragma float_control(push)
#pragma float_control(precise, on)
#endif

/*====================================SinCos====================================
**
==============================================================================*/
Expand All @@ -243,6 +249,10 @@ FCIMPL3_VII(void, COMSingle::SinCos, float x, float* pSin, float* pCos)

FCIMPLEND

#if defined(_MSC_VER)
#pragma float_control(pop)
#endif

/*=====================================Sinh=====================================
**
==============================================================================*/
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/Math.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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://github.com/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))
Expand Down Expand Up @@ -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://github.com/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)
{
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Runtime/tests/System.Runtime.Extensions.Tests/System/MathF.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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://github.com/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))
Expand Down Expand Up @@ -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://github.com/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)
{
Expand Down