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

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: efriedma, scanon, resistor, rjmccall.
Herald added subscribers: hiraditya, mcrosier.
Herald added a project: LLVM.
spatel requested review of this revision.

We shift the significand right on a truncation, but that needs to be made NaN-safe: always set at least 1 bit in the significand.
https://llvm.org/PR43907


https://reviews.llvm.org/D87835

Files:
  llvm/lib/Support/APFloat.cpp
  llvm/test/Transforms/InstSimplify/ConstProp/cast.ll


Index: llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
===================================================================
--- llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
+++ llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
@@ -39,19 +39,22 @@
   ret float %i
 }
 
-; https://llvm.org/PR43907
+; https://llvm.org/PR43907 - make sure that a NaN doesn't morph into Inf.
 
 define float @nan_f64_trunc() {
 ; CHECK-LABEL: @nan_f64_trunc(
-; CHECK-NEXT:    ret float 0x7FF0000000000000
+; CHECK-NEXT:    ret float 0x7FF8000000000000
 ;
   %f = fptrunc double 0x7FF0000000000001 to float
   ret float %f
 }
 
+; Verify again with a vector type. NaN value may depend on whether any
+; of the original payload bits are still set.
+
 define <2 x half> @nan_v2f32_trunc() {
 ; CHECK-LABEL: @nan_v2f32_trunc(
-; CHECK-NEXT:    ret <2 x half> <half 0xH7C00, half 0xH7C37>
+; CHECK-NEXT:    ret <2 x half> <half 0xH7E00, half 0xH7C37>
 ;
   %f = fptrunc <2 x float> <float 0x7FF0000100000000, float 0x7FF0DEAD00000000> to <2 x half>
   ret <2 x half> %f
Index: llvm/lib/Support/APFloat.cpp
===================================================================
--- llvm/lib/Support/APFloat.cpp
+++ llvm/lib/Support/APFloat.cpp
@@ -2242,6 +2242,16 @@
     if (!X86SpecialNan && semantics == &semX87DoubleExtended)
       APInt::tcSetBit(significandParts(), semantics->precision - 1);
 
+    // If we are truncating NaN, it is possible that we shifted out all of the
+    // set bits in the NaN payload. But NaN must remain NaN, so some bit in the
+    // significand must be set (otherwise it is Inf). Set the quiet bit (the MSB
+    // of the significand).
+    if (APInt::tcIsZero(significandParts(), newPartCount)) {
+      assert(shift < 0 && "Should not lose NaN payload on extend");
+      makeQuiet();
+      *losesInfo = true;
+    }
+
     // gcc forces the Quiet bit on, which means (float)(double)(float_sNan)
     // does not give you back the same bits.  This is dubious, and we
     // don't currently do it.  You're really supposed to get


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87835.292509.patch
Type: text/x-patch
Size: 2053 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200917/a95611fc/attachment.bin>