Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #370 - implement proper ER-mode comparison for floating point types #373

Closed
wants to merge 2 commits into from
Closed

Fix #370 - implement proper ER-mode comparison for floating point types #373

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a693443
Implement proper ER-mode comparison for floating point types
latkin Apr 21, 2015
3404c8a
Optimization to remove a branch from comparison
latkin Apr 21, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions src/fsharp/FSharp.Core/prim-types.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1214,8 +1214,14 @@ namespace Microsoft.FSharp.Core
when 'T : uint64 = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when 'T : unativeint = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
// Note, these bail out to GenericComparisonWithComparerIntrinsic if called with NaN values, because clt and cgt and ceq all return "false" for that case.
when 'T : float = if (# "clt" x y : bool #) then (-1) else if (# "cgt" x y : bool #) then 1 else if (# "ceq" x y : bool #) then 0 else GenericComparisonWithComparerIntrinsic comp x y
when 'T : float32 = if (# "clt" x y : bool #) then (-1) else if (# "cgt" x y : bool #) then 1 else if (# "ceq" x y : bool #) then 0 else GenericComparisonWithComparerIntrinsic comp x y
when 'T : float = if (# "clt" x y : bool #) then (-1)
elif (# "cgt" x y : bool #) then (1)
elif (# "ceq" x y : bool #) then (0)
else GenericComparisonWithComparerIntrinsic comp x y
when 'T : float32 = if (# "clt" x y : bool #) then (-1)
elif (# "cgt" x y : bool #) then (1)
elif (# "ceq" x y : bool #) then (0)
else GenericComparisonWithComparerIntrinsic comp x y
when 'T : char = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when 'T : string =
// NOTE: we don't have to null check here because System.String.CompareOrdinal
Expand Down Expand Up @@ -1284,8 +1290,18 @@ namespace Microsoft.FSharp.Core
when 'T : uint32 = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when 'T : uint64 = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when 'T : unativeint = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when 'T : float = if (# "clt" x y : bool #) then (-1) else (# "cgt" x y : int #)
when 'T : float32 = if (# "clt" x y : bool #) then (-1) else (# "cgt" x y : int #)
when 'T : float = if (# "clt" x y : bool #) then (-1)
elif (# "cgt" x y : bool #) then (1)
elif (# "ceq" x y : bool #) then (0)
elif (# "ceq" x x : bool #) then (1)
elif (# "ceq" y y : bool #) then (-1)
else (0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Reordering the nan comparisons we can eliminate a branch

when 'T : float  = if   (# "clt" x y : bool #) then (-1)
                   elif (# "cgt" x y : bool #) then (1)
                   elif (# "ceq" x y : bool #) then (0)
                   elif (# "ceq" y y : bool #) then (-1)
                   else (# "ceq" x x : bool #)

It's similar to what was done here previously, but doesn't avoid any extra IL calls and is arguably less clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! I think you mean (# "ceq" x x : int #) on the last line (return the ceq result as int value 1 or 0, not bool value)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's what I meant 😊

when 'T : float32 = if (# "clt" x y : bool #) then (-1)
elif (# "cgt" x y : bool #) then (1)
elif (# "ceq" x y : bool #) then (0)
elif (# "ceq" x x : bool #) then (1)
elif (# "ceq" y y : bool #) then (-1)
else (0)
when 'T : char = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when 'T : string =
// NOTE: we don't have to null check here because System.String.CompareOrdinal
Expand Down Expand Up @@ -4611,6 +4627,7 @@ namespace Microsoft.FSharp.Core
when ^T : decimal = System.Decimal.op_Inequality((# "" x:decimal #), (# "" y:decimal #))


// static comparison (ER mode) with static optimizations for some well-known cases
[<CompiledName("Compare")>]
let inline compare (x:^T) (y:^T) : int =
(if x < y then -1 elif x > y then 1 else 0)
Expand All @@ -4625,8 +4642,18 @@ namespace Microsoft.FSharp.Core
when ^T : uint32 = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when ^T : uint64 = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when ^T : unativeint = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when ^T : float = if (# "clt" x y : bool #) then (-1) else (# "cgt" x y : int #)
when ^T : float32 = if (# "clt" x y : bool #) then (-1) else (# "cgt" x y : int #)
when ^T : float = if (# "clt" x y : bool #) then (-1)
elif (# "cgt" x y : bool #) then (1)
elif (# "ceq" x y : bool #) then (0)
elif (# "ceq" x x : bool #) then (1)
elif (# "ceq" y y : bool #) then (-1)
else (0)
when ^T : float32 = if (# "clt" x y : bool #) then (-1)
elif (# "cgt" x y : bool #) then (1)
elif (# "ceq" x y : bool #) then (1)
elif (# "ceq" x x : bool #) then (1)
elif (# "ceq" y y : bool #) then (-1)
else (0)
when ^T : char = if (# "clt.un" x y : bool #) then (-1) else (# "cgt.un" x y : int #)
when ^T : string =
// NOTE: we don't have to null check here because System.String.CompareOrdinal
Expand Down
Loading