Skip to content

Commit

Permalink
Some cleanup of the Math functions from #20788 (dotnet/coreclr#20912)
Browse files Browse the repository at this point in the history 
* Fixing Max, MaxMagnitude, Min, and MinMagnitude for Math/MathF to be IEEE compliant

* Disabling the System.Math.Max/Min tests

* Adding the new c_runtime PAL tests to the CMakeLists to ensure they actually get run.

* Fixing the casing of IlogB to ILogB

* Fixing the new PAL tests to match the correct/expected outputs

* Fixing up PAL_ilogb to correctly handle 0 and NaN

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
  • Loading branch information
@tannergooding @jlennox
tannergooding authored and jlennox committed Dec 16, 2018
1 parent ba8f21a commit 8d2e167
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 20 deletions.
140 changes: 122 additions & 18 deletions src/Common/src/CoreLib/System/Math.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,17 +538,31 @@ public static decimal Max(decimal val1, decimal val2)

public static double Max(double val1, double val2)
{
if (val1 > val2)
// When val1 and val2 are both finite or infinite, return the larger
// * We count +0.0 as larger than -0.0 to match MSVC
// When val1 or val2, but not both, are NaN return the opposite
// * We return the opposite if either is NaN to match MSVC

if (double.IsNaN(val1))
{
return val1;
return val2;
}

if (double.IsNaN(val1))
if (double.IsNaN(val2))
{
return val1;
}

return val2;
// We do this comparison first and separately to handle the -0.0 to +0.0 comparision
// * Doing (val1 < val2) first could get transformed into (val2 >= val1) by the JIT
// which would then return an incorrect value

if (val1 == val2)
{
return double.IsNegative(val1) ? val2 : val1;
}

return (val1 < val2) ? val2 : val1;
}

[NonVersionable]
Expand Down Expand Up @@ -578,17 +592,31 @@ public static sbyte Max(sbyte val1, sbyte val2)

public static float Max(float val1, float val2)
{
if (val1 > val2)
// When val1 and val2 are both finite or infinite, return the larger
// * We count +0.0 as larger than -0.0 to match MSVC
// When val1 or val2, but not both, are NaN return the opposite
// * We return the opposite if either is NaN to match MSVC

if (float.IsNaN(val1))
{
return val1;
return val2;
}

if (float.IsNaN(val1))
if (float.IsNaN(val2))
{
return val1;
}

return val2;
// We do this comparison first and separately to handle the -0.0 to +0.0 comparision
// * Doing (val1 < val2) first could get transformed into (val2 >= val1) by the JIT
// which would then return an incorrect value

if (val1 == val2)
{
return float.IsNegative(val1) ? val2 : val1;
}

return (val1 < val2) ? val2 : val1;
}

[CLSCompliant(false)]
Expand All @@ -614,7 +642,31 @@ public static ulong Max(ulong val1, ulong val2)

public static double MaxMagnitude(double x, double y)
{
return Max(Abs(x), Abs(y));
// When x and y are both finite or infinite, return the larger magnitude
// * We count +0.0 as larger than -0.0 to match MSVC
// When x or y, but not both, are NaN return the opposite
// * We return the opposite if either is NaN to match MSVC

if (double.IsNaN(x))
{
return y;
}

if (double.IsNaN(y))
{
return x;
}

// We do this comparison first and separately to handle the -0.0 to +0.0 comparision
// * Doing (x < y) first could get transformed into (y >= x) by the JIT which would
// then return an incorrect value

if (x == y)
{
return double.IsNegative(x) ? y : x;
}

return (Abs(x) < Abs(y)) ? y : x;
}

[NonVersionable]
Expand All @@ -631,17 +683,31 @@ public static decimal Min(decimal val1, decimal val2)

public static double Min(double val1, double val2)
{
if (val1 < val2)
// When val1 and val2 are both finite or infinite, return the smaller
// * We count -0.0 as smaller than -0.0 to match MSVC
// When val1 or val2, but not both, are NaN return the opposite
// * We return the opposite if either is NaN to match MSVC

if (double.IsNaN(val1))
{
return val1;
return val2;
}

if (double.IsNaN(val1))
if (double.IsNaN(val2))
{
return val1;
}

return val2;
// We do this comparison first and separately to handle the -0.0 to +0.0 comparision
// * Doing (val1 < val2) first could get transformed into (val2 >= val1) by the JIT
// which would then return an incorrect value

if (val1 == val2)
{
return double.IsNegative(val1) ? val1 : val2;
}

return (val1 < val2) ? val1 : val2;
}

[NonVersionable]
Expand Down Expand Up @@ -671,17 +737,31 @@ public static sbyte Min(sbyte val1, sbyte val2)

public static float Min(float val1, float val2)
{
if (val1 < val2)
// When val1 and val2 are both finite or infinite, return the smaller
// * We count -0.0 as smaller than -0.0 to match MSVC
// When val1 or val2, but not both, are NaN return the opposite
// * We return the opposite if either is NaN to match MSVC

if (float.IsNaN(val1))
{
return val1;
return val2;
}

if (float.IsNaN(val1))
if (float.IsNaN(val2))
{
return val1;
}

return val2;
// We do this comparison first and separately to handle the -0.0 to +0.0 comparision
// * Doing (val1 < val2) first could get transformed into (val2 >= val1) by the JIT
// which would then return an incorrect value

if (val1 == val2)
{
return float.IsNegative(val1) ? val1 : val2;
}

return (val1 < val2) ? val1 : val2;
}

[CLSCompliant(false)]
Expand All @@ -707,7 +787,31 @@ public static ulong Min(ulong val1, ulong val2)

public static double MinMagnitude(double x, double y)
{
return Min(Abs(x), Abs(y));
// When x and y are both finite or infinite, return the smaller magnitude
// * We count -0.0 as smaller than -0.0 to match MSVC
// When x or y, but not both, are NaN return the opposite
// * We return the opposite if either is NaN to match MSVC

if (double.IsNaN(x))
{
return y;
}

if (double.IsNaN(y))
{
return x;
}

// We do this comparison first and separately to handle the -0.0 to +0.0 comparision
// * Doing (x < y) first could get transformed into (y >= x) by the JIT which would
// then return an incorrect value

if (x == y)
{
return double.IsNegative(x) ? x : y;
}

return (Abs(x) < Abs(y)) ? x : y;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down
52 changes: 50 additions & 2 deletions src/Common/src/CoreLib/System/MathF.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,31 @@ public static float Max(float x, float y)

public static float MaxMagnitude(float x, float y)
{
return Max(Abs(x), Abs(y));
// When x and y are both finite or infinite, return the larger magnitude
// * We count +0.0 as larger than -0.0 to match MSVC
// When x or y, but not both, are NaN return the opposite
// * We return the opposite if either is NaN to match MSVC

if (float.IsNaN(x))
{
return y;
}

if (float.IsNaN(y))
{
return x;
}

// We do this comparison first and separately to handle the -0.0 to +0.0 comparision
// * Doing (x < y) first could get transformed into (y >= x) by the JIT which would
// then return an incorrect value

if (x == y)
{
return float.IsNegative(x) ? y : x;
}

return (Abs(x) < Abs(y)) ? y : x;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand All @@ -201,7 +225,31 @@ public static float Min(float x, float y)

public static float MinMagnitude(float x, float y)
{
return Min(Abs(x), Abs(y));
// When x and y are both finite or infinite, return the smaller magnitude
// * We count -0.0 as smaller than -0.0 to match MSVC
// When x or y, but not both, are NaN return the opposite
// * We return the opposite if either is NaN to match MSVC

if (float.IsNaN(x))
{
return y;
}

if (float.IsNaN(y))
{
return x;
}

// We do this comparison first and separately to handle the -0.0 to +0.0 comparision
// * Doing (x < y) first could get transformed into (y >= x) by the JIT which would
// then return an incorrect value

if (x == y)
{
return float.IsNegative(x) ? x : y;
}

return (Abs(x) < Abs(y)) ? x : y;
}

[Intrinsic]
Expand Down

0 comments on commit 8d2e167

Please sign in to comment.