[libcxx-commits] [PATCH] D107036: [WIP] [libc++] [P1614] Various unimplemented parts of <compare>. [WIP]

[Kent Ross via Phabricator via libcxx-commits]

mumbleskates added inline comments.


================
Comment at: libcxx/include/__compare/cmp_alg.h:66
+            // These two NaNs should be ordered by their payload bits,
+            // but there's no way to get at that information portably.
+            return _VSTD::signbit(__f) <=> _VSTD::signbit(__e);
----------------
mumbleskates wrote:
> I would hope that we can order them by union view of the data in memory if such a thing is allowed (or memcpy, if that's better) once we know that the sign and signaling bits are the same, because the specifics of the payload ordering are unspecified by totalOrder. Any way that we can look at the raw data in memory without UB should be sufficient.
Ah I see, there are literally no standard implementation for the `isSignaling` predicate for iec559 types in C++. However, it does look like ISO C has an `issignaling` macro as part of the Floating-point extensions part 1, which I gather was accepted into the C23 standard in 2019 (ISO/IEC TS 60559-1, of interest section 7.12.3.7). Complete implementation of C++ `strong_order` for iec559 floating point types might be considered to be blocked on the implementation of this part of the standard as that seems like the best place to get it.

Implementing this just for `strong_order` is messy, very complex, and generally inadvisable. (Even for binary floating point values, ignoring the existence of decimal floating point types, some platforms -- notably PA-RISC -- use the opposite interpretation for the signaling bit in IEC559 floating point types, even though it's in the same location.) Unless we can strongly declare that we don't support any platforms other than say ARM and x86 this seems like a no-go, and it just seems better to take the platform support directly from the llvm C standard implementation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107036/new/

https://reviews.llvm.org/D107036