This revision was automatically updated to reflect the committed changes.
Closed by commit rG149f5b573c79: [APFloat] convert SNaN to QNaN in convert()
and raise Invalid signal (authored by spatel).
Herald added subscribers: cfe-commits, jrtc27.
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88238/new/
https://reviews.llvm.org/D88238
Files:
clang/test/CodeGen/builtin-nan-exception.c
clang/test/CodeGen/builtin-nan-legacy.c
clang/test/CodeGen/mips-unsupported-nan.c
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/Support/APFloat.cpp
llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
llvm/unittests/ADT/APFloatTest.cpp
Index: llvm/unittests/ADT/APFloatTest.cpp
===================================================================
--- llvm/unittests/ADT/APFloatTest.cpp
+++ llvm/unittests/ADT/APFloatTest.cpp
@@ -1816,11 +1816,12 @@
EXPECT_FALSE(losesInfo);
test = APFloat::getSNaN(APFloat::IEEEsingle());
- APFloat X87SNaN = APFloat::getSNaN(APFloat::x87DoubleExtended());
APFloat::opStatus status = test.convert(APFloat::x87DoubleExtended(), APFloat::rmNearestTiesToEven, &losesInfo);
- EXPECT_TRUE(test.bitwiseIsEqual(X87SNaN));
+ // Conversion quiets the SNAN, so now 2 bits of the 64-bit significand should be set.
+ APInt topTwoBits(64, 0x6000000000000000);
+ EXPECT_TRUE(test.bitwiseIsEqual(APFloat::getQNaN(APFloat::x87DoubleExtended(), false, &topTwoBits)));
EXPECT_FALSE(losesInfo);
- EXPECT_EQ(status, APFloat::opOK);
+ EXPECT_EQ(status, APFloat::opInvalidOp);
test = APFloat::getQNaN(APFloat::IEEEsingle());
APFloat X87QNaN = APFloat::getQNaN(APFloat::x87DoubleExtended());
@@ -1832,6 +1833,7 @@
test = APFloat::getSNaN(APFloat::x87DoubleExtended());
test.convert(APFloat::x87DoubleExtended(), APFloat::rmNearestTiesToEven,
&losesInfo);
+ APFloat X87SNaN = APFloat::getSNaN(APFloat::x87DoubleExtended());
EXPECT_TRUE(test.bitwiseIsEqual(X87SNaN));
EXPECT_FALSE(losesInfo);
@@ -1841,13 +1843,13 @@
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.
+ // The payload is lost in truncation, but we retain NaN by setting the quiet bit.
APInt payload(52, 1);
test = APFloat::getSNaN(APFloat::IEEEdouble(), false, &payload);
status = test.convert(APFloat::IEEEsingle(), APFloat::rmNearestTiesToEven, &losesInfo);
- EXPECT_EQ(0x7fa00000, test.bitcastToAPInt());
+ EXPECT_EQ(0x7fc00000, test.bitcastToAPInt());
EXPECT_TRUE(losesInfo);
- EXPECT_EQ(status, APFloat::opOK);
+ EXPECT_EQ(status, APFloat::opInvalidOp);
// The payload is lost in truncation. QNaN remains QNaN.
test = APFloat::getQNaN(APFloat::IEEEdouble(), false, &payload);
Index: llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
===================================================================
--- llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/nancvt.ll
@@ -18,6 +18,9 @@
@var = external global i32
+; SNAN becomes QNAN on fptrunc:
+; 2147228864 = 0x7ffc1cc0 : QNAN
+
define i32 @main() {
; CHECK-LABEL: @main(
; CHECK-NEXT: entry:
@@ -30,15 +33,15 @@
; CHECK-NEXT: store volatile i32 2147228864, i32* @var, align 4
; CHECK-NEXT: store volatile i32 2147228864, i32* @var, align 4
; CHECK-NEXT: store volatile i32 2147228864, i32* @var, align 4
-; CHECK-NEXT: store volatile i32 2146502828, i32* @var, align 4
+; CHECK-NEXT: store volatile i32 2147027116, i32* @var, align 4
; CHECK-NEXT: store volatile i32 -1610612736, i32* @var, align 4
-; CHECK-NEXT: store volatile i32 2146502828, i32* @var, align 4
+; CHECK-NEXT: store volatile i32 2147027116, i32* @var, align 4
; CHECK-NEXT: store volatile i32 -2147483648, i32* @var, align 4
-; CHECK-NEXT: store volatile i32 2146502828, i32* @var, align 4
+; CHECK-NEXT: store volatile i32 2147027116, i32* @var, align 4
; CHECK-NEXT: store volatile i32 -1073741824, i32* @var, align 4
-; CHECK-NEXT: store volatile i32 2143034560, i32* @var, align 4
-; CHECK-NEXT: store volatile i32 2143034560, i32* @var, align 4
-; CHECK-NEXT: store volatile i32 2143034560, i32* @var, align 4
+; CHECK-NEXT: store volatile i32 2147228864, i32* @var, align 4
+; CHECK-NEXT: store volatile i32 2147228864, i32* @var, align 4
+; CHECK-NEXT: store volatile i32 2147228864, i32* @var, align 4
; CHECK-NEXT: ret i32 undef
;
entry:
Index: llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
===================================================================
--- llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
+++ llvm/test/Transforms/InstSimplify/ConstProp/cast.ll
@@ -40,24 +40,24 @@
}
; https://llvm.org/PR43907 - make sure that NaN doesn't morph into Inf.
-; SNaN remains SNaN.
+; SNaN becomes QNaN.
define float @nan_f64_trunc() {
; CHECK-LABEL: @nan_f64_trunc(
-; CHECK-NEXT: ret float 0x7FF4000000000000
+; CHECK-NEXT: ret float 0x7FF8000000000000
;
%f = fptrunc double 0x7FF0000000000001 to float
ret float %f
}
; Verify again with a vector and different destination type.
-; SNaN remains SNaN (first two elements).
+; SNaN becomes 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>
+; CHECK-NEXT: ret <3 x half> <half 0xH7E00, half 0xH7E00, half 0xH7E00>
;
%f = fptrunc <3 x double> <double 0x7FF0020000000000, double 0x7FF003FFFFFFFFFF, double 0x7FF8000000000001> to <3 x half>
ret <3 x half> %f
Index: llvm/lib/Support/APFloat.cpp
===================================================================
--- llvm/lib/Support/APFloat.cpp
+++ llvm/lib/Support/APFloat.cpp
@@ -2243,26 +2243,15 @@
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);
+ // Convert of sNaN creates qNaN and raises an exception (invalid op).
+ // This also guarantees that a sNaN does not become Inf on a truncation
+ // that loses all payload bits.
+ if (isSignaling()) {
+ makeQuiet();
+ fs = opInvalidOp;
+ } else {
+ fs = opOK;
}
-
- // 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
- // an invalid operation signal at runtime, but nobody does that.
- fs = opOK;
} else {
*losesInfo = false;
fs = opOK;
Index: llvm/lib/IR/AsmWriter.cpp
===================================================================
--- llvm/lib/IR/AsmWriter.cpp
+++ llvm/lib/IR/AsmWriter.cpp
@@ -1373,9 +1373,19 @@
"assuming that double is 64 bits!");
APFloat apf = APF;
// Floats are represented in ASCII IR as double, convert.
- if (!isDouble)
+ // FIXME: We should allow 32-bit hex float and remove this.
+ if (!isDouble) {
+ // A signaling NaN is quieted on conversion, so we need to recreate the
+ // expected value after convert (quiet bit of the payload is clear).
+ bool IsSNAN = apf.isSignaling();
apf.convert(APFloat::IEEEdouble(), APFloat::rmNearestTiesToEven,
- &ignored);
+ &ignored);
+ if (IsSNAN) {
+ APInt Payload = apf.bitcastToAPInt();
+ apf = APFloat::getSNaN(APFloat::IEEEdouble(), apf.isNegative(),
+ &Payload);
+ }
+ }
Out << format_hex(apf.bitcastToAPInt().getZExtValue(), 0, /*Upper=*/true);
return;
}
Index: llvm/lib/AsmParser/LLParser.cpp
===================================================================
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -5345,6 +5345,8 @@
// The lexer has no type info, so builds all half, bfloat, float, and double
// FP constants as double. Fix this here. Long double does not need this.
if (&ID.APFloatVal.getSemantics() == &APFloat::IEEEdouble()) {
+ // Check for signaling before potentially converting and losing that info.
+ bool IsSNAN = ID.APFloatVal.isSignaling();
bool Ignored;
if (Ty->isHalfTy())
ID.APFloatVal.convert(APFloat::IEEEhalf(), APFloat::rmNearestTiesToEven,
@@ -5355,6 +5357,14 @@
else if (Ty->isFloatTy())
ID.APFloatVal.convert(APFloat::IEEEsingle(), APFloat::rmNearestTiesToEven,
&Ignored);
+ if (IsSNAN) {
+ // The convert call above may quiet an SNaN, so manufacture another
+ // SNaN. The bitcast works because the payload (significand) parameter
+ // is truncated to fit.
+ APInt Payload = ID.APFloatVal.bitcastToAPInt();
+ ID.APFloatVal = APFloat::getSNaN(ID.APFloatVal.getSemantics(),
+ ID.APFloatVal.isNegative(), &Payload);
+ }
}
V = ConstantFP::get(Context, ID.APFloatVal);
Index: clang/test/CodeGen/mips-unsupported-nan.c
===================================================================
--- clang/test/CodeGen/mips-unsupported-nan.c
+++ clang/test/CodeGen/mips-unsupported-nan.c
@@ -39,7 +39,21 @@
// CHECK-MIPS64: warning: ignoring '-mnan=2008' option because the 'mips64' architecture does not support it
// CHECK-MIPS64R6: warning: ignoring '-mnan=legacy' option because the 'mips64r6' architecture does not support it
-// CHECK-NANLEGACY: float 0x7FF4000000000000
+// This call creates a QNAN double with an empty payload.
+// The quiet bit is inverted in legacy mode: it is clear to indicate QNAN,
+// so the next highest bit is set to maintain NAN (not infinity).
+// In regular (2008) mode, the quiet bit is set to indicate QNAN.
+
+// CHECK-NANLEGACY: double 0x7FF4000000000000
+// CHECK-NAN2008: double 0x7FF8000000000000
+
+double d = __builtin_nan("");
+
+// This call creates a QNAN double with an empty payload and then truncates.
+// llvm::APFloat does not know about the inverted quiet bit, so it sets the
+// quiet bit on conversion independently of the setting in clang.
+
+// CHECK-NANLEGACY: float 0x7FFC000000000000
// CHECK-NAN2008: float 0x7FF8000000000000
float f = __builtin_nan("");
Index: clang/test/CodeGen/builtin-nan-legacy.c
===================================================================
--- clang/test/CodeGen/builtin-nan-legacy.c
+++ clang/test/CodeGen/builtin-nan-legacy.c
@@ -1,7 +1,15 @@
// RUN: %clang -target mipsel-unknown-linux -mnan=legacy -emit-llvm -S %s -o - | FileCheck %s
-// CHECK: float 0x7FF4000000000000, float 0x7FF8000000000000
+// CHECK: float 0x7FFC000000000000, float 0x7FF8000000000000
// CHECK: double 0x7FF4000000000000, double 0x7FF8000000000000
+// The first line shows an unintended consequence.
+// __builtin_nan() creates a legacy QNAN double with an empty payload
+// (the first bit of the significand is clear to indicate quiet, so
+// the second bit of the payload is set to maintain NAN-ness).
+// The value is then truncated, but llvm::APFloat does not know about
+// the inverted quiet bit, so it sets the first bit on conversion
+// to indicate 'quiet' independently of the setting in clang.
+
float f[] = {
__builtin_nan(""),
__builtin_nans(""),
Index: clang/test/CodeGen/builtin-nan-exception.c
===================================================================
--- clang/test/CodeGen/builtin-nan-exception.c
+++ clang/test/CodeGen/builtin-nan-exception.c
@@ -17,8 +17,12 @@
// Doubles are created and converted to floats.
+// Converting (truncating) to float quiets the NaN (sets the MSB
+// of the significand) and raises the APFloat invalidOp exception
+// but that should not cause a compilation error in the default
+// (ignore FP exceptions) mode.
-// CHECK: float 0x7FF8000000000000, float 0x7FF4000000000000
+// CHECK: float 0x7FF8000000000000, float 0x7FFC000000000000
float converted_to_float[] = {
__builtin_nan(""),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
