Skip to content

Tracking Issue for total_cmp (on f32/f64) #72599

Closed

Description

@golddranks

This is a tracking issue for the APIs f32::total_cmp, f64::total_cmp ( #72568 )
The feature gate for the issue is #![feature(total_cmp)].

Overview of the API

  • Implements method total_cmp on f32 and f64. This method implements a float comparison that, unlike the standard partial_cmp, is total (defined on all values) in accordance to the IEEE 754 (rev 2008) §5.10 totalOrder predicate.
  • The method has an API similar to cmp: pub fn total_cmp(&self, other: &Self) -> crate::cmp::Ordering { ... }.

Steps

  • Implement the RFC
  • FCP
  • Stabilization PR
    • Update docs to not promise more than we can.

Unresolved Questions

  • The exact ordering required by IEEE 754, revisions 2008 and 2019, isn't followed on some Tier 2 platforms, namely, older MIPS systems. The problem is that on those platforms, the meaning of the "signaling NaN" and "quiet NaN" bit is reversed. This means the order of those NaNs is reverse on those platfroms. Newer MIPS chips have a flag for the user to choose the behaviour. Should we add a special mention about this in the documentation, or just ignore the problem, as it's 1) not very common (being Tier 2 and all) 2) minor (reversal of order of sNaN and qNaN is unlikely to matter in real-life situations, and in those kinds of situations, the user is usually aware of the problem) 3) going away. (The newer systems addressing this problem.)

Implementation history

#72568

Activity

  1. added
    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
    on May 26, 2020
  2. added
    B-unstableBlocker: Implemented in the nightly compiler and unstable.
    T-libs-api[DEPRECATED; DO NOT USE]
    on May 26, 2020
  3. hanna-kruppe commented on May 26, 2020

    @hanna-kruppe
    Contributor

    Another reason to ignore the "signaling bit is flipped on older MIPS" issue: as far as I know, there is essentially no way to actually tell signaling and quiet NaNs apart in current or near-future Rust. You can of course inspect the bit in question with to_bits, but nothing tells you how that bit is interpreted by the hardware. Debug printing, f32::classify, etc. do not distinguish sNaN from qNaN, and Rust neither exposes the distinction between quiet/signaling comparisons not any way to check for floating point exceptions those predicates may raise depending on sNaN/qNaN operands.

  4. sfackler commented on May 27, 2020

    @sfackler
    Member

    Yeah, I think that the best we can do is just stick something in the docs noting the issue.

  5. est31 commented on Jun 14, 2020

    @est31
    Member

    The exact ordering required by IEEE 754, revisions 2008 and 2019, isn't followed on some Tier 2 platforms, namely, older MIPS systems. The problem is that on those platforms, the meaning of the "signaling NaN" and "quiet NaN" bit is reversed

    There are multiple bit pattern definitions of "signaling NaN" and "quiet NaN":

    • The "flipped" definition of older MIPS standards
    • The definitions of IEEE 754-2008 and IEEE 754-2019

    The two definitions disagree, and no implementation of totalOrder can be conformant with both. But the current implementation of totalOrder is conformant with the IEEE definition. Why should you use the MIPS standard definition to determine conformance to an IEEE standard?

    I'd filed a PR in 2017 to add some docs on NaN to the {from,to}_bits functions but closed it because I thought it wouldn't matter due to those devices being phased out. #46246

  6. ecstatic-morse commented on Jun 15, 2020

    @ecstatic-morse
    Contributor

    At present, the behavior of this comparator is unspecified, since the bits inside of a NaN are not guaranteed to be preserved across operations (including loads and stores) by LLVM (#73328). This includes the sign bit as well (#55131). We will need to fix these issues this before stabilizing total_cmp, and we should call this out explicitly in the documentation.

  7. added
    A-floating-pointArea: Floating point numbers and arithmetic
    I-libs-radarLibs issues that are tracked on the team's radar.
    on Jul 29, 2020
  8. RalfJung commented on Sep 9, 2020

    @RalfJung
    Member

    The exact ordering required by IEEE 754, revisions 2008 and 2019, isn't followed on some Tier 2 platforms, namely, older MIPS systems. The problem is that on those platforms, the meaning of the "signaling NaN" and "quiet NaN" bit is reversed. This means the order of those NaNs is reverse on those platfroms.

    What does this mean? Does the same bit pattern compare differently? Or is the ordering, when implemented uniformly across platforms, specified in terms of whether a NaN is signalling or not, and thus that cross-platform implementation is inconsistent with the signaling vs quiet distinction on that platform?

    Basically, I am wondering which spec is being violated when we just ignore the fact that these MIPS systems have a "weird" definition of signaling NaNs.


    Cc rust-lang/unsafe-code-guidelines#237 as the place where I am trying to collect all open questions around floating-point semantics.

  9. RalfJung commented on Sep 9, 2020

    @RalfJung
    Member

    Oh I see... the order is specified in terms of whether NaNs are signaling or not, but implemented disregarding whatever "signalling NaN" means on the current platform (assuming the standardized meaning instead).

    So I guess this technically means that the implementation is not quite conforming (but this seems rather minor compared to how badly behaved i686 floats are^^ see #72327, #73288). Not sure what would be more surprising, documenting this as "not fully spec-compliant on that platform" or trying to fix the implementation. Does any other language try to work around this by changing the implementation on that platform?

  10. est31 commented on Sep 9, 2020

    @est31
    Member

    Oh I see... the order is specified in terms of whether NaNs are signaling or not, but implemented disregarding whatever "signalling NaN" means on the current platform (assuming the standardized meaning instead).

    So I guess this technically means that the implementation is not quite conforming

    Not conforming to which spec? All IEEE 754 spec versions that define totalOrder (versions 2008 and onwards) also define what signaling NaN means. If you see a definition foo in a document A reference another definition bar, did they mean the definition bar from document B, or did they mean the definition bar that's also contained in A? I'd say the latter. Note that to my knowledge, there is no totalOrder definition in the document B (older versions of the MIPS spec in this instance). If that were the case, one could arguably say that Rust's implementation conflicts the platform.

    So I believe that Rust is always spec compliant on older mips platforms, but yes there is possible confusions for readers of the spec of totalOrder.

  11. clarfonthey commented on Nov 22, 2020

    @clarfonthey
    Contributor

    Has there been any discussion of adding a Total wrapper like Wrapping for integers?

  12. leonardo-m commented on Mar 29, 2021

    @leonardo-m

    Has there been any discussion of adding a Total wrapper like Wrapping for integers?

    An alternative is to add f64/f32 methods like:

    fn total_fp_ord(self) -> i64 {
        let bi = self.to_bits() as i64;
        bi^ (((bi >> 63) as u64) >> 1) as i64
    }

    With this method you can use max_by_key instead of max_by.
    But the wrapper idea could be better.

  13. scottmcm commented on Mar 29, 2021

    @scottmcm
    Member

    Has there been any discussion of adding a Total wrapper like Wrapping for integers?

    That was brought up in my original PR: #53938 (comment)

  14. 25 remaining items

  15. golddranks commented on Feb 26, 2022

    @golddranks
    ContributorAuthor

    A big thank you ya'll!!!

    As the contributor of the initial PR and this issue two years back, (and unfortunately, not having much time to work on Rust in between) I'd very much like to push it over the goal line! I'll get back to this tomorrow.
    @rustbot claim

  16. golddranks commented on Mar 22, 2022

    @golddranks
    ContributorAuthor

    Just a note, I've been extremely busy at work for some weeks now for deadlines at end of this week. I'll get back to this next week. I want to do a thorough review about the points discussed about the documented / to be documented guarantees before sending the PR. @RalfJung also had some concerns; I don't believe that this API is a bigger cause for concern than some other API's (I'm thinking of to_bits/from_bits) we've already stabilized, but I want to review them nevertheless to be sure of the full picture.

  17. golddranks commented on Mar 22, 2022

    @golddranks
    ContributorAuthor

    (As a further note the reason we want to be careful is because the semantics of floats in general are a complicated interplay at least between:

    • guarantees / lack of guarantees (guarantees by Rust, guarantees by LLVM, guarantees by target platforms)
    • compiler optimizations
    • platform-specific hardware implementation differences
    • differences between runtime and const evaluation)
  18. RalfJung commented on Mar 22, 2022

    @RalfJung
    Member

    My concerns are mostly around what is mentioned at the top -- that some platforms are not spec-compliant. I don't quite see how that relates to to/from_bits.

    However, since you mention to/from_bits -- total_cmp should not be a const fn for the same reason that to/from_bits are not. (Currently total_cmp is not even unstably const so this is fine.)

  19. golddranks commented on Mar 29, 2022

    @golddranks
    ContributorAuthor

    So, I read a bunch of GitHub threads and the existing documentation. Here's my current understanding:

    About the behaviour of floats:

    The runtime behaviour isn't always guaranteed with NaNs. (And because of LLVM-specific stuff, with subnormals too.) This can be because:

    • Differences in hardware implementations
    • LLVM not guaranteeing to keep the payload (the sign bit or the signaling bit or the "data" payload)
      • Especially LLVM doing const folding and other optimizations might affect different code paths differently
      • Some arithmetic operations (sign flipping comes to mind) don't care preserving NaN signs to achieve efficient implementation; not wanting to separate logic for NaNs

    What do we do about it?

    While unfortunate, the LLVM shenanigans haven't historically prevented Rust from stabilizing operations that expose NaN payloads: to/from_bits and is_sign_negative are earlier examples, where the payloads are exposed. total_cmp is just another such an example. However, they DO prevent making them const, where determinism and platform-independence are requirements, but that is out of consideration here. (So, this was a point I was worried about when I said I want to review the points discussed, but it turns out that only the constification is considered a problem.)

    As for the differences in hardware implementations, we want to be sure to document them to not over-promise and reduce surprises.

    What's still a bit unclear for me, how strongly we should warn ALSO against the indeterminism introduced by LLVM in the docs of operations that easily expose the payload? The issue #73328 is about this.

    About the current documentation:

    The current documentation mentions about platform differences, but only in some items (namely, from_bits). However, even with a cursory read, I identified multiple items that could benefit at least a mention about the issue, redirecting to the explanation of from_bits for further explanation.

    The current documentation doesn't mention about issues caused by LLVM. Again, that's #73328 .

    My proposal:

    • The current blocker for stabilization of total_cmp is updating the documentation WRT the platform differences, so first of all, I'm just going do that to get this in to Rust 1.61. from_bits already provides a good reference, so I think that referring mostly to that is going to be fine for the "main" explanation. However, I'm going to add, how the issue affects total_cmp. Expect a documentation PR later today.
    • As I said, some other items could benefit from a similar explanation. I'm going to open an issue, and then work on that.
    • After that, we should tackle Document guarantees (or lack thereof) regarding sign, quietness, and payload of NaNs #73328 . While not being overly specific about the details, an explanation about LLVM's non-guarantees of conserving/propagating NaN payload is definitely warranted. I now see that this was considered as a totally separate issue from stabilizing total_cmp. (Another point that wasn't clear to me), but I see it as worth solving, especially as we are adding operations that allow observing/poking the NaN payload bits.
  20. golddranks commented on Mar 29, 2022

    @golddranks
    ContributorAuthor

    Oops, so looking into that, it seems that #93403 was just that missing documentation PR! Sorry for the fuss :D I'll send a stabilization PR then.

  21. golddranks commented on Apr 4, 2022

    @golddranks
    ContributorAuthor

    There's not a lot of time left to merge the stabilization PR ( #95431 ) if we want to get total_cmp in 1.61. It seems like @dtolnay is busy, is there anyone here with merge rights that could review instead? Also, since there are people that might have insight about the guarantees and semantics around floats, I'd appreciate comments in #95483 too!

  22. added 2 commits that reference this issue on Apr 4, 2022
  23. JohnTitor commented on May 22, 2022

    @JohnTitor
    Member

    Triage: I'm going to close this as the stabilization and doc improvement PRs were merged already 🎉

  24. paulabrudanandrei commented on May 29, 2023

    @paulabrudanandrei

    What will happen to PartialCmp and PartialEq?

  25. golddranks commented on May 29, 2023

    @golddranks
    ContributorAuthor

    What will happen to PartialCmp and PartialEq?

    Nothing. They continued existing happily ever after.

    In case you are confused what this issue was about, it was about adding method total_cmp for floats. The existence of that method doesn't affect the semantics of PartialCmp and PartialEq whatsoever. The method is there if you explicitly decide to use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-floating-pointArea: Floating point numbers and arithmeticB-unstableBlocker: Implemented in the nightly compiler and unstable.C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCE-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.I-libs-radarLibs issues that are tracked on the team's radar.T-libs-api[DEPRECATED; DO NOT USE]disposition-mergeThis issue / PR is in PFCP or FCP with a disposition to merge it.finished-final-comment-periodThe final comment period is finished for this PR / Issue.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions