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

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 24 07:45:04 PDT 2020


spatel added a comment.

In D87835#2291094 <https://reviews.llvm.org/D87835#2291094>, @efriedma wrote:

> I think IEEE754's convertFormat operation transforms sNaN->qNaN; I think it makes sense for APFloat to do the same.

There's a potential snag:
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.
That means the following -instsimplify test calls APFloat::convert() with SNaN, so the naive fix would set the quiet bit as shown in the CHECK line before we ever get to -instsimplify in this test:

  define float @fsub_snan_op1(float %x) {
  ; CHECK-LABEL: @fsub_nan_op1(
  ; CHECK-NEXT:    ret float 0x7FF9000000000000
  ;
    %r = fsub float %x, 0x7FF1000000000000
    ret float %r
  }

We could say that's as intended, but we will then be inconsistent for `float` vs. every other FP type which can specify some form of SNaN in hex. (That's because -instsimplify does not explicitly set the quiet bit in its normal transforms currently.)
It's possible that we can fall back to the argument that non-constrained LLVM FP ignores FP exceptions, so it doesn't matter.
There are a few other regression tests failing, and I'm not sure what to make of them. I'll post the alternate patch alongside this one.


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

https://reviews.llvm.org/D87835



More information about the llvm-commits mailing list