[PATCH] D141863: [llvm][APFloat] Add NaN-in-negative-zero formats by AMD and GraphCore

[Krzysztof Drewniak via Phabricator via llvm-commits]

krzysz00 added inline comments.


================
Comment at: llvm/lib/Support/APFloat.cpp:2506
+             semantics->nanEncoding == fltNanEncoding::NegativeZero) {
+    *losesInfo = fromSemantics.nanEncoding != fltNanEncoding::NegativeZero;
+    fs = *losesInfo ? opInexact : opOK;
----------------
reedwm wrote:
> Don't we only lose info if the source of the conversion is -0?
I don't think so? If I understand `losesInfo` correctly, it's for (non-rounding-related?) cases where, for `x : T`, `convert(T, convert(U, x)) != x` is possible.

So since both +0 and -0 map to +0, we've lost information.


================
Comment at: llvm/unittests/ADT/APFloatTest.cpp:1871
     const unsigned long long bitPattern[2];
-    const unsigned bitPatternLength;
+    const unsigned bitPatternLength; // todo preserve sign flag
   } const GetZeroTest[] = {
----------------
reedwm wrote:
> What does this TODO mean?
Stray comment that was a note about adding what's currently `const bool signedZero;` above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141863