Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Cleanup on the new Math APIs#20993

Merged
tannergooding merged 2 commits intodotnet:masterfrom
tannergooding:math
Nov 14, 2018
Merged

Cleanup on the new Math APIs#20993
tannergooding merged 2 commits intodotnet:masterfrom
tannergooding:math

Conversation

@tannergooding
Copy link
Copy Markdown
Member

The new tests, dotnet/corefx#33359, show that ARM is not being handled correctly and that the wrong result is being chosen for MinMag and MaxMag when Abx(x) == Abs(y).

@tannergooding
Copy link
Copy Markdown
Member Author

CC. @eerhardt, @ViktorHofer

Copy link
Copy Markdown
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@tannergooding tannergooding merged commit 4d2be96 into dotnet:master Nov 14, 2018
dotnet-maestro-bot pushed a commit to dotnet-maestro-bot/corefx that referenced this pull request Nov 14, 2018
* Updating the cached HAVE_COMPATIBLE_ILOGB0_EXITCODE and HAVE_COMPATIBLE_ILOGBNAN_EXITCODE to 1

* Fixing MaxMagnitude and MinMagnitude to correctly handle the case when ax and ay are equal

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
dotnet-maestro-bot pushed a commit to dotnet-maestro-bot/corert that referenced this pull request Nov 14, 2018
* Updating the cached HAVE_COMPATIBLE_ILOGB0_EXITCODE and HAVE_COMPATIBLE_ILOGBNAN_EXITCODE to 1

* Fixing MaxMagnitude and MinMagnitude to correctly handle the case when ax and ay are equal

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
}

return (Abs(x) < Abs(y)) ? y : x;
return (ax < ay) ? y : x;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the spec in https://github.com/dotnet/corefx/issues/31903

public static double MinMagnitude(double, double);             // IEEE `maxNumMag` - Equivalent to `Math.Max(Math.Abs(x), Math.Abs(y))`

Shouldn't this be returning ax or ay?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same response as dotnet/corefx#33359 (comment), the speclet is incorrect and doesn't correctly indicate that it returns the original (non absolute) value of x or y.

jkotas pushed a commit to dotnet/corert that referenced this pull request Nov 14, 2018
* Updating the cached HAVE_COMPATIBLE_ILOGB0_EXITCODE and HAVE_COMPATIBLE_ILOGBNAN_EXITCODE to 1

* Fixing MaxMagnitude and MinMagnitude to correctly handle the case when ax and ay are equal

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corefx that referenced this pull request Nov 14, 2018
* Updating the cached HAVE_COMPATIBLE_ILOGB0_EXITCODE and HAVE_COMPATIBLE_ILOGBNAN_EXITCODE to 1

* Fixing MaxMagnitude and MinMagnitude to correctly handle the case when ax and ay are equal

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jlennox pushed a commit to jlennox/corefx that referenced this pull request Dec 16, 2018
* Updating the cached HAVE_COMPATIBLE_ILOGB0_EXITCODE and HAVE_COMPATIBLE_ILOGBNAN_EXITCODE to 1

* Fixing MaxMagnitude and MinMagnitude to correctly handle the case when ax and ay are equal

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* Updating the cached HAVE_COMPATIBLE_ILOGB0_EXITCODE and HAVE_COMPATIBLE_ILOGBNAN_EXITCODE to 1

* Fixing MaxMagnitude and MinMagnitude to correctly handle the case when ax and ay are equal


Commit migrated from dotnet/coreclr@4d2be96
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants