From 8898b2040a16df83a6d155145cdaa952ed65600c Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 8 Feb 2024 22:59:05 -0800 Subject: [PATCH 1/7] Workaround an MSVC bug with __libm_sse2_sincos_ --- .../classlibnative/float/floatdouble.cpp | 10 ++++++ .../classlibnative/float/floatsingle.cpp | 10 ++++++ .../JitBlue/Runtime_98204/Runtime_98204.cs | 35 +++++++++++++++++++ .../Runtime_98204/Runtime_98204.csproj | 8 +++++ 4 files changed, 63 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.csproj diff --git a/src/coreclr/classlibnative/float/floatdouble.cpp b/src/coreclr/classlibnative/float/floatdouble.cpp index d20b772eb2207c..e2034c8c812309 100644 --- a/src/coreclr/classlibnative/float/floatdouble.cpp +++ b/src/coreclr/classlibnative/float/floatdouble.cpp @@ -253,6 +253,12 @@ FCIMPL1_V(double, COMDouble::Sin, double x) return sin(x); FCIMPLEND +#if defined(_MSC_VER) && defined(TARGET_AMD64) +// The /fp:fast form of `sincos` for AMD64 returns sin twice, rather than sincos +#pragma float_control(push) +#pragma float_control(precise, on) +#endif + /*====================================SinCos==================================== ** ==============================================================================*/ @@ -268,6 +274,10 @@ FCIMPL3_VII(void, COMDouble::SinCos, double x, double* pSin, double* pCos) FCIMPLEND +#if defined(_MSC_VER) && defined(TARGET_AMD64) +#pragma float_control(pop) +#endif + /*=====================================Sinh===================================== ** ==============================================================================*/ diff --git a/src/coreclr/classlibnative/float/floatsingle.cpp b/src/coreclr/classlibnative/float/floatsingle.cpp index 1694fd78cb8467..67c377d41cb0d8 100644 --- a/src/coreclr/classlibnative/float/floatsingle.cpp +++ b/src/coreclr/classlibnative/float/floatsingle.cpp @@ -228,6 +228,12 @@ FCIMPL1_V(float, COMSingle::Sin, float x) return sinf(x); FCIMPLEND +#if defined(_MSC_VER) && defined(TARGET_AMD64) +// The /fp:fast form of `sincos` for AMD64 returns sin twice, rather than sincos +#pragma float_control(push) +#pragma float_control(precise, on) +#endif + /*====================================SinCos==================================== ** ==============================================================================*/ @@ -243,6 +249,10 @@ FCIMPL3_VII(void, COMSingle::SinCos, float x, float* pSin, float* pCos) FCIMPLEND +#if defined(_MSC_VER) && defined(TARGET_AMD64) +#pragma float_control(pop) +#endif + /*=====================================Sinh===================================== ** ==============================================================================*/ diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.cs b/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.cs new file mode 100644 index 00000000000000..ce9bb54eecb9d5 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.cs @@ -0,0 +1,35 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +public static class Runtime_98068 +{ + [Fact] + [InlineData(1e18)] + public static void TestSinCosDouble(double d) + { + double sin1 = Math.Sin(d); + double cos1 = Math.Cos(d); + + (double sin2, double cos2) = Math.SinCos(d); + + Assert.Equal(sin1, sin2); + Assert.Equal(cos1, cos2); + } + + [Fact] + [InlineData(1e18f)] + public static void TestSinCosSingle(float f) + { + float sin1 = MathF.Sin(f); + float cos1 = MathF.Cos(f); + + (float sin2, float cos2) = MathF.SinCos(f); + + Assert.Equal(sin1, sin2); + Assert.Equal(cos1, cos2); + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.csproj new file mode 100644 index 00000000000000..15edd99711a1a4 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.csproj @@ -0,0 +1,8 @@ + + + True + + + + + \ No newline at end of file From 475d96c9dede391023a4b82f895163efd3de374e Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 9 Feb 2024 08:27:10 -0800 Subject: [PATCH 2/7] Add the SinCos regression tests to the existing Math/MathF tests --- .../System/Math.cs | 2 ++ .../System/MathF.cs | 2 ++ .../JitBlue/Runtime_98204/Runtime_98204.cs | 35 ------------------- .../Runtime_98204/Runtime_98204.csproj | 8 ----- 4 files changed, 4 insertions(+), 43 deletions(-) delete mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.cs delete mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.csproj 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 index 86ca388b114217..2e890e7e513e93 100644 --- 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, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // 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)) @@ -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, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // 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) { 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 index 85e62a190ad777..59040cd799c47a 100644 --- 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://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)) @@ -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) { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.cs b/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.cs deleted file mode 100644 index ce9bb54eecb9d5..00000000000000 --- a/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.cs +++ /dev/null @@ -1,35 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Runtime.CompilerServices; -using Xunit; - -public static class Runtime_98068 -{ - [Fact] - [InlineData(1e18)] - public static void TestSinCosDouble(double d) - { - double sin1 = Math.Sin(d); - double cos1 = Math.Cos(d); - - (double sin2, double cos2) = Math.SinCos(d); - - Assert.Equal(sin1, sin2); - Assert.Equal(cos1, cos2); - } - - [Fact] - [InlineData(1e18f)] - public static void TestSinCosSingle(float f) - { - float sin1 = MathF.Sin(f); - float cos1 = MathF.Cos(f); - - (float sin2, float cos2) = MathF.SinCos(f); - - Assert.Equal(sin1, sin2); - Assert.Equal(cos1, cos2); - } -} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.csproj deleted file mode 100644 index 15edd99711a1a4..00000000000000 --- a/src/tests/JIT/Regression/JitBlue/Runtime_98204/Runtime_98204.csproj +++ /dev/null @@ -1,8 +0,0 @@ - - - True - - - - - \ No newline at end of file From ab6b2db2055dd026d8c92ffd9f3090900999612e Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 9 Feb 2024 11:46:33 -0800 Subject: [PATCH 3/7] Ensure the workaround also applies to x86 --- src/coreclr/classlibnative/float/floatdouble.cpp | 6 +++--- src/coreclr/classlibnative/float/floatsingle.cpp | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/classlibnative/float/floatdouble.cpp b/src/coreclr/classlibnative/float/floatdouble.cpp index e2034c8c812309..78de2152337aad 100644 --- a/src/coreclr/classlibnative/float/floatdouble.cpp +++ b/src/coreclr/classlibnative/float/floatdouble.cpp @@ -253,8 +253,8 @@ FCIMPL1_V(double, COMDouble::Sin, double x) return sin(x); FCIMPLEND -#if defined(_MSC_VER) && defined(TARGET_AMD64) -// The /fp:fast form of `sincos` for AMD64 returns sin twice, rather than sincos +#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 @@ -274,7 +274,7 @@ FCIMPL3_VII(void, COMDouble::SinCos, double x, double* pSin, double* pCos) FCIMPLEND -#if defined(_MSC_VER) && defined(TARGET_AMD64) +#if defined(_MSC_VER) #pragma float_control(pop) #endif diff --git a/src/coreclr/classlibnative/float/floatsingle.cpp b/src/coreclr/classlibnative/float/floatsingle.cpp index 67c377d41cb0d8..c9fc5e604f6a77 100644 --- a/src/coreclr/classlibnative/float/floatsingle.cpp +++ b/src/coreclr/classlibnative/float/floatsingle.cpp @@ -228,8 +228,8 @@ FCIMPL1_V(float, COMSingle::Sin, float x) return sinf(x); FCIMPLEND -#if defined(_MSC_VER) && defined(TARGET_AMD64) -// The /fp:fast form of `sincos` for AMD64 returns sin twice, rather than sincos +#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 @@ -249,7 +249,7 @@ FCIMPL3_VII(void, COMSingle::SinCos, float x, float* pSin, float* pCos) FCIMPLEND -#if defined(_MSC_VER) && defined(TARGET_AMD64) +#if defined(_MSC_VER) #pragma float_control(pop) #endif From 2054b916e606110c931423d2c48db2ed74c1b3ef Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 9 Feb 2024 16:41:21 -0800 Subject: [PATCH 4/7] Allow a larger amount of variance due to x86 Windows --- .../tests/System.Runtime.Extensions.Tests/System/Math.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 2e890e7e513e93..06b074801a146d 100644 --- 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,7 +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, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // https://github.com/dotnet/runtime/issues/98204 + [InlineData(-1e18, 0.9929693207404051, 0.11837199021871073, 0.0002, 0.0002)] // 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)) @@ -1529,7 +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, CrossPlatformMachineEpsilon, CrossPlatformMachineEpsilon)] // https://github.com/dotnet/runtime/issues/98204 + [InlineData( 1e18, -0.9929693207404051, 0.11837199021871073, 0.0002, 0.0002)] // 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) { From eec93fa79da35502ed428c17783cccd47172b27a Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Sat, 10 Feb 2024 09:31:47 -0800 Subject: [PATCH 5/7] Adjust the allowedVarianceCos for x86 --- .../tests/System.Runtime.Extensions.Tests/System/Math.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 06b074801a146d..bff81bdbe3858e 100644 --- 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,7 +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.0002)] // https://github.com/dotnet/runtime/issues/98204 + [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)) @@ -1529,7 +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.0002)] // https://github.com/dotnet/runtime/issues/98204 + [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) { From 1fb842a435877a07e8bfddeeeb3ab0b880f75e74 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Sat, 10 Feb 2024 12:41:39 -0800 Subject: [PATCH 6/7] Add a link to the MSVC issue --- src/coreclr/classlibnative/float/floatdouble.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/classlibnative/float/floatdouble.cpp b/src/coreclr/classlibnative/float/floatdouble.cpp index 78de2152337aad..b7c9932bffdd8d 100644 --- a/src/coreclr/classlibnative/float/floatdouble.cpp +++ b/src/coreclr/classlibnative/float/floatdouble.cpp @@ -255,6 +255,7 @@ 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 From cdfcb7037bddb06838857e2cdeadcd9e1f39569d Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Sat, 10 Feb 2024 12:42:06 -0800 Subject: [PATCH 7/7] Add a link to the MSVC issue --- src/coreclr/classlibnative/float/floatsingle.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/classlibnative/float/floatsingle.cpp b/src/coreclr/classlibnative/float/floatsingle.cpp index c9fc5e604f6a77..3f6290f12c43f3 100644 --- a/src/coreclr/classlibnative/float/floatsingle.cpp +++ b/src/coreclr/classlibnative/float/floatsingle.cpp @@ -230,6 +230,7 @@ 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