From 06c61eea609c75e6fd7a9ec49f42f074f037b638 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 22 Mar 2017 22:44:40 -0700 Subject: [PATCH] =?utf8?q?Removing=20the=20special=20handling=20in=20class?= =?utf8?q?libnative=20for=20atan2(=C2=B1=E2=88=9E,=20=C2=B1=E2=88=9E)=20an?= =?utf8?q?d=20pow(-1.0,=20=C2=B1=E2=88=9E).=20(dotnet/coreclr#10295)?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * Removing the special handling in classlibnative for atan2(±∞, ±∞) and pow(-1.0, ±∞). * Fixing up the logic for HAVE_COMPATIBLE_POW in the PAL layer. Commit migrated from https://github.com/dotnet/coreclr/commit/72c57d9d9fd2d4cd8c96f124d132d316c8ef6013 --- .../src/classlibnative/float/floatdouble.cpp | 27 ----------------- .../src/classlibnative/float/floatsingle.cpp | 31 -------------------- src/coreclr/src/pal/src/configure.cmake | 6 ++-- src/coreclr/src/pal/src/cruntime/math.cpp | 34 +++++----------------- .../tests/palsuite/c_runtime/pow/test1/test1.cpp | 12 ++++---- .../tests/palsuite/c_runtime/powf/test1/test1.c | 14 ++++----- .../src/CoreMangLib/cti/system/math/mathpow.cs | 2 +- .../src/CoreMangLib/cti/system/mathf/mathfpow.cs | 2 +- .../tests/src/JIT/Directed/intrinsic/pow/pow1.cs | 2 +- .../tests/src/JIT/Methodical/NaN/intrinsic.cs | 8 ++--- .../tests/src/JIT/Methodical/NaN/intrinsic_nonf.il | 8 ++--- 11 files changed, 34 insertions(+), 112 deletions(-) diff --git a/src/coreclr/src/classlibnative/float/floatdouble.cpp b/src/coreclr/src/classlibnative/float/floatdouble.cpp index d9603c0..6f593e5 100644 --- a/src/coreclr/src/classlibnative/float/floatdouble.cpp +++ b/src/coreclr/src/classlibnative/float/floatdouble.cpp @@ -9,8 +9,6 @@ #include "floatdouble.h" -#define IS_DBL_INFINITY(x) (((*((INT64*)((void*)&x))) & I64(0x7FFFFFFFFFFFFFFF)) == I64(0x7FF0000000000000)) - // The default compilation mode is /fp:precise, which disables floating-point intrinsics. This // default compilation mode has previously caused performance regressions in floating-point code. // We enable /fp:fast semantics for the majority of the math functions, as it will speed up performance @@ -86,13 +84,6 @@ FCIMPLEND FCIMPL2_VV(double, COMDouble::Atan2, double y, double x) FCALL_CONTRACT; - // atan2(+/-INFINITY, +/-INFINITY) produces +/-0.78539816339744828 (x is +INFINITY) and - // +/-2.3561944901923448 (x is -INFINITY) instead of the expected value of NaN. We handle - // that case here ourselves. - if (IS_DBL_INFINITY(y) && IS_DBL_INFINITY(x)) { - return (double)(y / x); - } - return (double)atan2(y, x); FCIMPLEND @@ -174,24 +165,6 @@ FCIMPLEND FCIMPL2_VV(double, COMDouble::Pow, double x, double y) FCALL_CONTRACT; - // The CRT version of pow preserves the NaN payload of x over the NaN payload of y. - - if(_isnan(y)) { - return y; // IEEE 754-2008: NaN payload must be preserved - } - - if(_isnan(x)) { - return x; // IEEE 754-2008: NaN payload must be preserved - } - - // The CRT version of pow does not return NaN for pow(-1.0, +/-INFINITY) and - // instead returns +1.0. - - if(IS_DBL_INFINITY(y) && (x == -1.0)) { - INT64 result = CLR_NAN_64; - return (*((double*)((INT64*)&result))); - } - return (double)pow(x, y); FCIMPLEND diff --git a/src/coreclr/src/classlibnative/float/floatsingle.cpp b/src/coreclr/src/classlibnative/float/floatsingle.cpp index c84c0bf..c7b7d41 100644 --- a/src/coreclr/src/classlibnative/float/floatsingle.cpp +++ b/src/coreclr/src/classlibnative/float/floatsingle.cpp @@ -9,18 +9,12 @@ #include "floatsingle.h" -#define IS_FLT_INFINITY(x) (((*((INT32*)((void*)&x))) & 0x7FFFFFFF) == 0x7F800000) - // Windows x86 and Windows ARM/ARM64 may not define _isnanf() or _copysignf() but they do // define _isnan() and _copysign(). We will redirect the macros to these other functions if // the macro is not defined for the platform. This has the side effect of a possible implicit // upcasting for arguments passed in and an explicit downcasting for the _copysign() call. #if (defined(_TARGET_X86_) || defined(_TARGET_ARM_) || defined(_TARGET_ARM64_)) && !defined(FEATURE_PAL) -#if !defined(_isnanf) -#define _isnanf _isnan -#endif - #if !defined(_copysignf) #define _copysignf (float)_copysign #endif @@ -88,13 +82,6 @@ FCIMPLEND FCIMPL2_VV(float, COMSingle::Atan2, float y, float x) FCALL_CONTRACT; - // atan2f(+/-INFINITY, +/-INFINITY) produces +/-0.785398163f (x is +INFINITY) and - // +/-2.35619449f (x is -INFINITY) instead of the expected value of NaN. We handle - // that case here ourselves. - if (IS_FLT_INFINITY(y) && IS_FLT_INFINITY(x)) { - return (float)(y / x); - } - return (float)atan2f(y, x); FCIMPLEND @@ -176,24 +163,6 @@ FCIMPLEND FCIMPL2_VV(float, COMSingle::Pow, float x, float y) FCALL_CONTRACT; - // The CRT version of pow preserves the NaN payload of x over the NaN payload of y. - - if(_isnanf(y)) { - return y; // IEEE 754-2008: NaN payload must be preserved - } - - if(_isnanf(x)) { - return x; // IEEE 754-2008: NaN payload must be preserved - } - - // The CRT version of powf does not return NaN for powf(-1.0f, +/-INFINITY) and - // instead returns +1.0f. - - if(IS_FLT_INFINITY(y) && (x == -1.0f)) { - INT32 result = CLR_NAN_32; - return (*((float*)((INT32*)&result))); - } - return (float)powf(x, y); FCIMPLEND diff --git a/src/coreclr/src/pal/src/configure.cmake b/src/coreclr/src/pal/src/configure.cmake index 4f2bc57..4ca1fe9 100644 --- a/src/coreclr/src/pal/src/configure.cmake +++ b/src/coreclr/src/pal/src/configure.cmake @@ -728,9 +728,9 @@ check_cxx_source_runs(" int main(void) { double infinity = 1.0 / 0.0; if (pow(1.0, infinity) != 1.0 || pow(1.0, -infinity) != 1.0) { - exit(1) + exit(1); } - if (!isnan(pow(-1.0, infinity)) || !isnan(pow(-1.0, -infinity))) { + if (pow(-1.0, infinity) != 1.0 || pow(-1.0, -infinity) != 1.0) { exit(1); } if (pow(0.0, infinity) != 0.0) { @@ -742,7 +742,7 @@ int main(void) { if (pow(-1.1, infinity) != infinity || pow(1.1, infinity) != infinity) { exit(1); } - if (pow(-1.1, -infinity) != 0.0 || pow(1.1, infinity) != 0.0) { + if (pow(-1.1, -infinity) != 0.0 || pow(1.1, -infinity) != 0.0) { exit(1); } if (pow(-0.0, -1) != -infinity) { diff --git a/src/coreclr/src/pal/src/cruntime/math.cpp b/src/coreclr/src/pal/src/cruntime/math.cpp index d53dbe7..7b5175a 100644 --- a/src/coreclr/src/pal/src/cruntime/math.cpp +++ b/src/coreclr/src/pal/src/cruntime/math.cpp @@ -343,7 +343,7 @@ PALIMPORT double __cdecl PAL_pow(double x, double y) } else if (x == -1.0) { - ret = PAL_NAN_DBL; // NaN + ret = 1.0; } else if ((x > -1.0) && (x < 1.0)) { @@ -362,7 +362,7 @@ PALIMPORT double __cdecl PAL_pow(double x, double y) } else if (x == -1.0) { - ret = PAL_NAN_DBL; // NaN + ret = 1.0; } else if ((x > -1.0) && (x < 1.0)) { @@ -384,17 +384,7 @@ PALIMPORT double __cdecl PAL_pow(double x, double y) else #endif // !HAVE_COMPATIBLE_POW - if ((y == 0.0) && isnan(x)) - { - // Windows returns NaN for pow(NaN, 0), but POSIX specifies - // a return value of 1 for that case. We need to return - // the same result as Windows. - ret = PAL_NAN_DBL; - } - else - { - ret = pow(x, y); - } + ret = pow(x, y); #if !HAVE_VALID_NEGATIVE_INF_POW if ((ret == PAL_POSINF_DBL) && (x < 0) && isfinite(x) && (ceil(y / 2) != floor(y / 2))) @@ -706,7 +696,7 @@ PALIMPORT float __cdecl PAL_powf(float x, float y) } else if (x == -1.0f) { - ret = PAL_NAN_FLT; // NaN + ret = 1.0f; } else if ((x > -1.0f) && (x < 1.0f)) { @@ -725,7 +715,7 @@ PALIMPORT float __cdecl PAL_powf(float x, float y) } else if (x == -1.0f) { - ret = PAL_NAN_FLT; // NaN + ret = 1.0f; } else if ((x > -1.0f) && (x < 1.0f)) { @@ -747,18 +737,8 @@ PALIMPORT float __cdecl PAL_powf(float x, float y) else #endif // !HAVE_COMPATIBLE_POW - if ((y == 0.0f) && isnan(x)) - { - // Windows returns NaN for powf(NaN, 0), but POSIX specifies - // a return value of 1 for that case. We need to return - // the same result as Windows. - ret = PAL_NAN_FLT; - } - else - { - ret = powf(x, y); - } - + ret = powf(x, y); + #if !HAVE_VALID_NEGATIVE_INF_POW if ((ret == PAL_POSINF_FLT) && (x < 0) && isfinite(x) && (ceilf(y / 2) != floorf(y / 2))) { diff --git a/src/coreclr/src/pal/tests/palsuite/c_runtime/pow/test1/test1.cpp b/src/coreclr/src/pal/tests/palsuite/c_runtime/pow/test1/test1.cpp index 0a05cd5..7eea316 100644 --- a/src/coreclr/src/pal/tests/palsuite/c_runtime/pow/test1/test1.cpp +++ b/src/coreclr/src/pal/tests/palsuite/c_runtime/pow/test1/test1.cpp @@ -106,6 +106,9 @@ int __cdecl main(int argc, char **argv) { -2.7182818284590452, 1, -2.7182818284590452, PAL_EPSILON * 10 }, // x: -(e) expected: e { -2.7182818284590452, PAL_POSINF, PAL_POSINF, 0 }, // x: -(e) + { -1.0, PAL_NEGINF, 1.0, PAL_EPSILON * 10 }, + { -1.0, PAL_POSINF, 1.0, PAL_EPSILON * 10 }, + { -0.0, PAL_NEGINF, PAL_POSINF, 0 }, { -0.0, -1, PAL_NEGINF, 0 }, { -0.0, -0.0, 1, PAL_EPSILON * 10 }, @@ -113,6 +116,9 @@ int __cdecl main(int argc, char **argv) { -0.0, 1, -0.0, PAL_EPSILON }, { -0.0, PAL_POSINF, 0, PAL_EPSILON }, + { PAL_NAN, -0.0, 1.0, PAL_EPSILON * 10 }, + { PAL_NAN, 0, 1.0, PAL_EPSILON * 10 }, + { 0.0, PAL_NEGINF, PAL_POSINF, 0 }, { 0.0, -1, PAL_POSINF, 0 }, { 0, -0.0, 1, PAL_EPSILON * 10 }, @@ -211,12 +217,6 @@ int __cdecl main(int argc, char **argv) validate_isnan(-2.7182818284590452, 0.78539816339744828); // x: -(e) y: pi / 4 validate_isnan(-2.7182818284590452, 1.5707963267948966); // x: -(e) y: pi / 2 - validate_isnan(-1, PAL_NEGINF); - validate_isnan(-1, PAL_POSINF); - - validate_isnan(PAL_NAN, -0.0); - validate_isnan(PAL_NAN, 0); - validate_isnan(PAL_NEGINF, PAL_NAN); validate_isnan(PAL_NAN, PAL_NEGINF); diff --git a/src/coreclr/src/pal/tests/palsuite/c_runtime/powf/test1/test1.c b/src/coreclr/src/pal/tests/palsuite/c_runtime/powf/test1/test1.c index ca738e8..e8933c5 100644 --- a/src/coreclr/src/pal/tests/palsuite/c_runtime/powf/test1/test1.c +++ b/src/coreclr/src/pal/tests/palsuite/c_runtime/powf/test1/test1.c @@ -104,7 +104,10 @@ int __cdecl main(int argc, char **argv) { -2.71828183f, 0, 1, PAL_EPSILON * 10 }, // x: -(e) { -2.71828183f, 1, -2.71828183f, PAL_EPSILON * 10 }, // x: -(e) expected: e { -2.71828183f, PAL_POSINF, PAL_POSINF, 0 }, // x: -(e) - + + { -1.0, PAL_NEGINF, 1.0, PAL_EPSILON * 10 }, + { -1.0, PAL_POSINF, 1.0, PAL_EPSILON * 10 }, + { -0.0, PAL_NEGINF, PAL_POSINF, 0 }, { -0.0, -1, PAL_NEGINF, 0 }, { -0.0f, -0.0f, 1, PAL_EPSILON * 10 }, @@ -112,6 +115,9 @@ int __cdecl main(int argc, char **argv) { -0.0, 1, -0.0, PAL_EPSILON }, { -0.0, PAL_POSINF, 0, PAL_EPSILON }, + { PAL_NAN, -0.0, 1.0, PAL_EPSILON * 10 }, + { PAL_NAN, 0, 1.0, PAL_EPSILON * 10 }, + { 0.0, PAL_NEGINF, PAL_POSINF, 0 }, { 0.0, -1, PAL_POSINF, 0 }, { 0, -0.0f, 1, PAL_EPSILON * 10 }, @@ -210,12 +216,6 @@ int __cdecl main(int argc, char **argv) validate_isnan(-2.71828183f, 0.785398163f); // x: -(e) y: pi / 4 validate_isnan(-2.71828183f, 1.57079633f); // x: -(e) y: pi / 2 - validate_isnan(-1, PAL_NEGINF); - validate_isnan(-1, PAL_POSINF); - - validate_isnan(PAL_NAN, -0.0); - validate_isnan(PAL_NAN, 0); - validate_isnan(PAL_NEGINF, PAL_NAN); validate_isnan(PAL_NAN, PAL_NEGINF); diff --git a/src/coreclr/tests/src/CoreMangLib/cti/system/math/mathpow.cs b/src/coreclr/tests/src/CoreMangLib/cti/system/math/mathpow.cs index c9b02c3..10986a8 100644 --- a/src/coreclr/tests/src/CoreMangLib/cti/system/math/mathpow.cs +++ b/src/coreclr/tests/src/CoreMangLib/cti/system/math/mathpow.cs @@ -217,7 +217,7 @@ public class MathPow { double d = Math.Pow(- 1, double.NegativeInfinity); - if (!double.IsNaN(d)) + if (d != 1) { TestLibrary.TestFramework.LogError("007.1", "Return value is wrong!"); retVal = false; diff --git a/src/coreclr/tests/src/CoreMangLib/cti/system/mathf/mathfpow.cs b/src/coreclr/tests/src/CoreMangLib/cti/system/mathf/mathfpow.cs index 22019ac..deac1d8 100644 --- a/src/coreclr/tests/src/CoreMangLib/cti/system/mathf/mathfpow.cs +++ b/src/coreclr/tests/src/CoreMangLib/cti/system/mathf/mathfpow.cs @@ -216,7 +216,7 @@ public class MathFPow { float f = MathF.Pow(-1, float.NegativeInfinity); - if (!float.IsNaN(f)) + if (f != 1) { TestLibrary.TestFramework.LogError("007.1", "Return value is wrong!"); retVal = false; diff --git a/src/coreclr/tests/src/JIT/Directed/intrinsic/pow/pow1.cs b/src/coreclr/tests/src/JIT/Directed/intrinsic/pow/pow1.cs index dc6e1a1..6f2921d 100644 --- a/src/coreclr/tests/src/JIT/Directed/intrinsic/pow/pow1.cs +++ b/src/coreclr/tests/src/JIT/Directed/intrinsic/pow/pow1.cs @@ -129,7 +129,7 @@ internal class pow1 x = 1; y = Double.NaN; z = Math.Pow(x, y); - if (!Double.IsNaN(z)) + if (z != 1) { Console.WriteLine("x: {0}, y: {1}, Pow(x,y): {2}", x, y, z); pass = false; diff --git a/src/coreclr/tests/src/JIT/Methodical/NaN/intrinsic.cs b/src/coreclr/tests/src/JIT/Methodical/NaN/intrinsic.cs index 86020d8..f6a97e5 100644 --- a/src/coreclr/tests/src/JIT/Methodical/NaN/intrinsic.cs +++ b/src/coreclr/tests/src/JIT/Methodical/NaN/intrinsic.cs @@ -582,11 +582,11 @@ namespace JitTest TestAtan2(Double.NaN, Double.PositiveInfinity, Double.NaN); TestAtan2(Double.NaN, Double.NegativeInfinity, Double.NaN); TestAtan2(Double.PositiveInfinity, Double.NaN, Double.NaN); - TestAtan2(Double.PositiveInfinity, Double.PositiveInfinity, Double.NaN); - TestAtan2(Double.PositiveInfinity, Double.NegativeInfinity, Double.NaN); + TestAtan2(Double.PositiveInfinity, Double.PositiveInfinity, Math.PI / 4); + TestAtan2(Double.PositiveInfinity, Double.NegativeInfinity, 3 * Math.PI / 4); TestAtan2(Double.NegativeInfinity, Double.NaN, Double.NaN); - TestAtan2(Double.NegativeInfinity, Double.PositiveInfinity, Double.NaN); - TestAtan2(Double.NegativeInfinity, Double.NegativeInfinity, Double.NaN); + TestAtan2(Double.NegativeInfinity, Double.PositiveInfinity, -Math.PI / 4); + TestAtan2(Double.NegativeInfinity, Double.NegativeInfinity, -3 * Math.PI / 4); } catch (Exception ex) { diff --git a/src/coreclr/tests/src/JIT/Methodical/NaN/intrinsic_nonf.il b/src/coreclr/tests/src/JIT/Methodical/NaN/intrinsic_nonf.il index ef9bc0a..9c9faa7 100644 --- a/src/coreclr/tests/src/JIT/Methodical/NaN/intrinsic_nonf.il +++ b/src/coreclr/tests/src/JIT/Methodical/NaN/intrinsic_nonf.il @@ -2846,13 +2846,13 @@ float64) IL_0783: ldc.r8 (00 00 00 00 00 00 F0 7F) IL_078c: ldc.r8 (00 00 00 00 00 00 F0 7F) - IL_0795: ldc.r8 (FF FF FF FF FF FF FF 7F) + IL_0795: ldc.r8 0.78539816339744828 IL_079e: call void JitTest.Test::TestAtan2(float64, float64, float64) IL_07a3: ldc.r8 (00 00 00 00 00 00 F0 7F) IL_07ac: ldc.r8 (00 00 00 00 00 00 F0 FF) - IL_07b5: ldc.r8 (FF FF FF FF FF FF FF 7F) + IL_07b5: ldc.r8 2.3561944901923448 IL_07be: call void JitTest.Test::TestAtan2(float64, float64, float64) @@ -2864,13 +2864,13 @@ float64) IL_07e3: ldc.r8 (00 00 00 00 00 00 F0 FF) IL_07ec: ldc.r8 (00 00 00 00 00 00 F0 7F) - IL_07f5: ldc.r8 (FF FF FF FF FF FF FF 7F) + IL_07f5: ldc.r8 -0.78539816339744828 IL_07fe: call void JitTest.Test::TestAtan2(float64, float64, float64) IL_0803: ldc.r8 (00 00 00 00 00 00 F0 FF) IL_080c: ldc.r8 (00 00 00 00 00 00 F0 FF) - IL_0815: ldc.r8 (FF FF FF FF FF FF FF 7F) + IL_0815: ldc.r8 -2.3561944901923448 IL_081e: call void JitTest.Test::TestAtan2(float64, float64, float64) -- 2.7.4