[llvm-branch-commits] [llvm] 60a2520 - [APFloat] prevent NaN morphing into Inf on conversion (PR43907)

[Hans Wennborg via llvm-branch-commits]

Author: Sanjay Patel
Date: 2020-09-30T13:28:43+02:00
New Revision: 60a25202a7dd1e00067fcfce512086ebf3788537

URL: https://github.com/llvm/llvm-project/commit/60a25202a7dd1e00067fcfce512086ebf3788537
DIFF: https://github.com/llvm/llvm-project/commit/60a25202a7dd1e00067fcfce512086ebf3788537.diff

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

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

See D88238 for the likely follow-up (but needs some plumbing fixes before it can proceed).

Differential Revision: https://reviews.llvm.org/D87835

(cherry picked from commit e34bd1e0b03d20a506ada156d87e1b3a96d82fa2)

Added: 
    

Modified: 
    llvm/lib/Support/APFloat.cpp
    llvm/test/Transforms/ConstProp/cast.ll
    llvm/unittests/ADT/APFloatTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 569cac790af9..362595d8f8b1 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -2242,6 +2242,21 @@ IEEEFloat::opStatus IEEEFloat::convert(const fltSemantics &toSemantics,
     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 a signalling NaN payload. But NaN must remain NaN, so some
+    // bit in the significand must be set (otherwise it is Inf).
+    // This can only happen with sNaN. Set the 1st bit after the quiet bit,
+    // so that we still have an sNaN.
+    // FIXME: Set quiet and return opInvalidOp (on convert of any sNaN).
+    //        But this requires fixing LLVM to parse 32-bit hex FP or ignoring
+    //        conversions while parsing IR.
+    if (APInt::tcIsZero(significandParts(), newPartCount)) {
+      assert(shift < 0 && "Should not lose NaN payload on extend");
+      assert(semantics->precision >= 3 && "Unexpectedly narrow significand");
+      assert(*losesInfo && "Missing payload should have set lost info");
+      APInt::tcSetBit(significandParts(), semantics->precision - 3);
+    }
+
     // 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

diff  --git a/llvm/test/Transforms/ConstProp/cast.ll b/llvm/test/Transforms/ConstProp/cast.ll
index 8377df17b3a8..c07fa1295b42 100644
--- a/llvm/test/Transforms/ConstProp/cast.ll
+++ b/llvm/test/Transforms/ConstProp/cast.ll
@@ -38,3 +38,26 @@ define float @overflow_sitofp() {
   ret float %i
 }
 
+; https://llvm.org/PR43907 - make sure that NaN doesn't morph into Inf.
+; SNaN remains SNaN.
+
+define float @nan_f64_trunc() {
+; CHECK-LABEL: @nan_f64_trunc(
+; CHECK-NEXT:    ret float 0x7FF4000000000000
+;
+  %f = fptrunc double 0x7FF0000000000001 to float
+  ret float %f
+}
+
+; Verify again with a vector and 
diff erent destination type.
+; SNaN remains SNaN (first two elements).
+; QNaN remains QNaN (third element).
+; Lower 42 bits of NaN source payload are lost.
+
+define <3 x half> @nan_v3f64_trunc() {
+; CHECK-LABEL: @nan_v3f64_trunc(
+; CHECK-NEXT:    ret <3 x half> <half 0xH7D00, half 0xH7D00, half 0xH7E00>
+;
+  %f = fptrunc <3 x double> <double 0x7FF0020000000000, double 0x7FF003FFFFFFFFFF, double 0x7FF8000000000001> to <3 x half>
+  ret <3 x half> %f
+}

diff  --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp
index b24b43d09a40..c798c95e05f6 100644
--- a/llvm/unittests/ADT/APFloatTest.cpp
+++ b/llvm/unittests/ADT/APFloatTest.cpp
@@ -1840,6 +1840,21 @@ TEST(APFloatTest, convert) {
                &losesInfo);
   EXPECT_TRUE(test.bitwiseIsEqual(X87QNaN));
   EXPECT_FALSE(losesInfo);
+
+  // The payload is lost in truncation, but we must retain NaN, so we set the bit after the quiet bit.
+  APInt payload(52, 1);
+  test = APFloat::getSNaN(APFloat::IEEEdouble(), false, &payload);
+  APFloat::opStatus status = test.convert(APFloat::IEEEsingle(), APFloat::rmNearestTiesToEven, &losesInfo);
+  EXPECT_EQ(0x7fa00000, test.bitcastToAPInt());
+  EXPECT_TRUE(losesInfo);
+  EXPECT_EQ(status, APFloat::opOK);
+
+  // The payload is lost in truncation. QNaN remains QNaN.
+  test = APFloat::getQNaN(APFloat::IEEEdouble(), false, &payload);
+  status = test.convert(APFloat::IEEEsingle(), APFloat::rmNearestTiesToEven, &losesInfo);
+  EXPECT_EQ(0x7fc00000, test.bitcastToAPInt());
+  EXPECT_TRUE(losesInfo);
+  EXPECT_EQ(status, APFloat::opOK);
 }
 
 TEST(APFloatTest, PPCDoubleDouble) {