[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