[PATCH] D87835: [APFloat] prevent NaN morphing into Inf on conversion (PR43907)

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

Well, I went and did a whole investigation here, only to come around to the idea that what you're doing might be the most reasonable thing.

IEEE 754-2008 section 6.2.3 says:

> An operation that propagates a NaN operand to its result and has a single NaN as an input should produce a NaN with the payload of the input NaN if representable in the destination format.
> Conversion of a quiet NaN from a narrower format to a wider format in the same radix, and then back to the same narrower format, should not change the quiet NaN payload in any way except to make it canonical.
> Conversion of a quiet NaN to a floating-point format of the same or a different radix that does not allow the payload to be preserved, shall return a quiet NaN that should provide some language-defined diagnostic information.

Also, operations on sNaN are permitted to preserve sNaN (if the result can represent it) but are not permitted to substitute a qNaN without signaling.

The standard is not super-clear about how to think about payloads, but probably the most useful idea for language implementors would be that they're an unsigned number right-aligned in the significand, consisting of all the bits following the qNaN indicator bit (and thus cannot be zero in an sNaN).

If I were interpreting all this completely in the abstract, I'd say it collectively implies that we really cannot implement this correctly on top of a right-shift, and the right implementation would have to be:

- Check whether the payload is representable (i.e. whether truncating it would change any bits).
- If so, truncate it and then preserve the original qNaN indicator bit.
- If not, use a zero payload, preserve the original qNaN indicator bit, and then set the second-highest bit if we started with an sNaN (to preserve the sNaN).

But x86 double->float conversion does not keep the payload right-shifted or do overflow checks; it just shifts the payload around, dropping any extra bits.  And x86 is probably the most faithful implementation around, and we probably shouldn't have APFloat diverge from its behavior.

So I think the only quibble here is whether it's okay for this to silently produce a qNaN when it might've started with an sNaN.  And arguably the current contract of APFloat is to perform operations as if FP exceptions were disabled, and we'll need a slightly different interface if we want to model exceptions properly.  So I think this might just be fine as-is.

Steve Canon might have further thoughts.


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

https://reviews.llvm.org/D87835