Removing the special handling for atan2(±∞, ±∞) and pow(-1.0, ±∞)#3498
Removing the special handling for atan2(±∞, ±∞) and pow(-1.0, ±∞)#3498tijoytom-zz merged 2 commits intodotnet:masterfrom
Conversation
The coreclr S.P.CoreLIB do not have this check and return -2.3561... for this call like Math.Atan2(double.NegativeInfinity, double.NegativeInfinity) but on CoreRT / N this return NaN because of this check. This check has been there for ever , for reasons that i don't know of.
|
Thanks @tijoytom |
|
Should we port all of dotnet/coreclr#10295 if we're doing this? |
|
@jkotas @MichalStrehovsky That makes sense , i will see if we can port the whole change then |
Other changes in #10295 are not applicable to corert since it's either test or math.cpp changes.
|
This LGTM. As context, the special handling was doing one of a few different things, depending on the specific scenario. The general case was that it was attempting to workaround legacy CRT bugs and the code wasn't relevant anymore (I fixed most of these cases here dotnet/coreclr#4847). The other case was that it was attempting to workaround what was thought to be a CRT bug (this was the case for Pow and NaN, for example). However, these workaround weren't actually valid if you read the entire IEEE spec. For example, normally NaN payloads are preserved, but the spec indicates The current CoreCLR implementations, on all the platforms we support should be fully IEEE compliant now (modulo handling of -0 for Floor and Ceiling on Windows -- https://github.com/dotnet/coreclr/issues/10288 and https://github.com/dotnet/coreclr/issues/10287). CoreCLR and CoreFX should additionally have a full suite of unit tests that validates we don't break this functionality in the future. If possible, CoreRT should probably have or run these tests as well. |
|
@tijoytom, I just looked through the rest of corert/Math.cs and it looks like there are a few other places where it needs updating to ensure it matches CoreCLR (mostly around the changes made in dotnet/coreclr#4847). Exp is doing additional checks (https://github.com/tijoytom/corert/blob/1fc641a152963474aa35ccc1ae760036bf01292b/src/System.Private.CoreLib/src/System/Math.cs#L248) Abs(float) should be calling Round is doing something other than |
The coreclr S.P.CoreLIB do not have this check and return -2.3561... for call like Math.Atan2(double.NegativeInfinity, double.NegativeInfinity) but on CoreRT / N
this return NaN because of this check. This check has been there for ever , for reasons that
i don't know of.
@danmosemsft @joshfree @atsushikan @jkotas