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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 24 10:04:47 PDT 2020


efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM

>> LLVM IR doesn't allow single-precision (32-bit) FP hex values (not sure why) - we convert those from/to 64-bit as shown in the first test diff in this patch.
>
> Okay. That honestly seems like something that should just be fixed — it's easy to sniff the literal length to figure out which format we have. But it's probably reasonable to preserve sNaNs in the short term and then come back to this.

Currently, we use prefixes: "H", "K", "L", "M", "R" to indicate the floating-point format.  We have a prefix for every format except double and single.  An unprefixed hex constant is assumed to be in double-precision format; if the constant is in some narrower format, it's converted.  (Note this is an issue specifically with the text format; bitcode doesn't do this sort of conversion.)

I think we should do two things here:

1. For reading IR, the conversion shouldn't be using IEEE semantics. We should preserve SNaNs, and forbid any conversion that's not exact.
2. Maybe add a prefix for single-precision constants, so we can write them with 32 bits, and avoid this sort of issue in the future.

But we can deal with that as a followup.


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

https://reviews.llvm.org/D87835



More information about the llvm-commits mailing list