-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Some cleanup of the Math functions from #20788 #20912
Changes from all commits
0c87325
f4342d9
f58326b
975133d
b7198c0
e44edd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just as a note here: This optimization is completely fine for the JIT to do since the IEEE spec indicates that comparisons shall ignore the sign of zero and for I opted to respect the sign of zero here since the MSVC/GCC/Clang implementation does and since Annex F of the C Language specification (which dictates IEC 60559 compliance) has a footnote indicating that respecting the sign of zero here would be "ideal". |
||
// * 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] | ||
|
@@ -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)] | ||
|
@@ -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] | ||
|
@@ -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] | ||
|
@@ -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)] | ||
|
@@ -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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is super unfortunate that we went from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If we had the ability to standardize on one, without it being a breaking change, |
||
{ | ||
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)] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change, but matches what the C Runtime does and what the IEEE spec defines