Skip to content

Commit ad7c6b6

Browse files
wzssyqawzssyqa
committed
APFloat: Fix maxnum and minnum with sNaN
See: llvm#112852 We have reclarify llvm.maxnum and llvm.minnum to follow libc's fmax(3) and fmin(3). So let's make APFloat::maxnum and APFloat::minnum to follow IEEE-754 2008's maxNum and minNum. maxNum is a more restircted version of fmax(3) with -0.0 < + 0.0. minNum is a more restircted version of fmin(3) with -0.0 < + 0.0.
1 parent 0fbf91a commit ad7c6b6

File tree

3 files changed

+98
-19
lines changed

3 files changed

+98
-19
lines changed

‎clang/lib/AST/ExprConstant.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15338,16 +15338,11 @@ bool FloatExprEvaluator::VisitCallExpr(const CallExpr *E) {
1533815338
case Builtin::BI__builtin_fmaxl:
1533915339
case Builtin::BI__builtin_fmaxf16:
1534015340
case Builtin::BI__builtin_fmaxf128: {
15341-
// TODO: Handle sNaN.
1534215341
APFloat RHS(0.);
1534315342
if (!EvaluateFloat(E->getArg(0), Result, Info) ||
1534415343
!EvaluateFloat(E->getArg(1), RHS, Info))
1534515344
return false;
15346-
// When comparing zeroes, return +0.0 if one of the zeroes is positive.
15347-
if (Result.isZero() && RHS.isZero() && Result.isNegative())
15348-
Result = RHS;
15349-
else if (Result.isNaN() || RHS > Result)
15350-
Result = RHS;
15345+
Result = maxnum(Result, RHS);
1535115346
return true;
1535215347
}
1535315348

@@ -15356,16 +15351,11 @@ bool FloatExprEvaluator::VisitCallExpr(const CallExpr *E) {
1535615351
case Builtin::BI__builtin_fminl:
1535715352
case Builtin::BI__builtin_fminf16:
1535815353
case Builtin::BI__builtin_fminf128: {
15359-
// TODO: Handle sNaN.
1536015354
APFloat RHS(0.);
1536115355
if (!EvaluateFloat(E->getArg(0), Result, Info) ||
1536215356
!EvaluateFloat(E->getArg(1), RHS, Info))
1536315357
return false;
15364-
// When comparing zeroes, return -0.0 if one of the zeroes is negative.
15365-
if (Result.isZero() && RHS.isZero() && RHS.isNegative())
15366-
Result = RHS;
15367-
else if (Result.isNaN() || RHS < Result)
15368-
Result = RHS;
15358+
Result = minnum(Result, RHS);
1536915359
return true;
1537015360
}
1537115361

‎llvm/include/llvm/ADT/APFloat.h

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,11 +1515,16 @@ inline APFloat neg(APFloat X) {
15151515
return X;
15161516
}
15171517

1518-
/// Implements IEEE-754 2019 minimumNumber semantics. Returns the smaller of the
1519-
/// 2 arguments if both are not NaN. If either argument is a NaN, returns the
1520-
/// other argument. -0 is treated as ordered less than +0.
1518+
/// Implements IEEE-754 2008 minNum semantics. Returns the smaller of the
1519+
/// 2 arguments if both are not NaN. If either argument is a qNaN, returns the
1520+
/// other argument. If either argument is sNaN, return a qNaN.
1521+
/// -0 is treated as ordered less than +0.
15211522
LLVM_READONLY
15221523
inline APFloat minnum(const APFloat &A, const APFloat &B) {
1524+
if (A.isSignaling())
1525+
return A.makeQuiet();
1526+
if (B.isSignaling())
1527+
return B.makeQuiet();
15231528
if (A.isNaN())
15241529
return B;
15251530
if (B.isNaN())
@@ -1529,11 +1534,16 @@ inline APFloat minnum(const APFloat &A, const APFloat &B) {
15291534
return B < A ? B : A;
15301535
}
15311536

1532-
/// Implements IEEE-754 2019 maximumNumber semantics. Returns the larger of the
1533-
/// 2 arguments if both are not NaN. If either argument is a NaN, returns the
1534-
/// other argument. +0 is treated as ordered greater than -0.
1537+
/// Implements IEEE-754 2008 maxNum semantics. Returns the larger of the
1538+
/// 2 arguments if both are not NaN. If either argument is a qNaN, returns the
1539+
/// other argument. If either argument is sNaN, return a qNaN.
1540+
/// +0 is treated as ordered greater than -0.
15351541
LLVM_READONLY
15361542
inline APFloat maxnum(const APFloat &A, const APFloat &B) {
1543+
if (A.isSignaling())
1544+
return A.makeQuiet();
1545+
if (B.isSignaling())
1546+
return B.makeQuiet();
15371547
if (A.isNaN())
15381548
return B;
15391549
if (B.isNaN())

‎llvm/unittests/ADT/APFloatTest.cpp

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,46 @@ TEST(APFloatTest, MinNum) {
582582
APFloat zp(0.0);
583583
APFloat zn(-0.0);
584584
EXPECT_EQ(-0.0, minnum(zp, zn).convertToDouble());
585-
EXPECT_EQ(-0.0, minnum(zn, zp).convertToDouble());
585+
586+
APInt intPayload_89ab(64, 0x89ab);
587+
APInt intPayload_cdef(64, 0xcdef);
588+
APFloat nan_0123[2] = {APFloat::getNaN(APFloat::IEEEdouble(), false, 0x0123),
589+
APFloat::getNaN(APFloat::IEEEdouble(), false, 0x0123)};
590+
APFloat mnan_4567[2] = {APFloat::getNaN(APFloat::IEEEdouble(), true, 0x4567),
591+
APFloat::getNaN(APFloat::IEEEdouble(), true, 0x4567)};
592+
APFloat nan_89ab[2] = {
593+
APFloat::getSNaN(APFloat::IEEEdouble(), false, &intPayload_89ab),
594+
APFloat::getNaN(APFloat::IEEEdouble(), false, 0x89ab)};
595+
APFloat mnan_cdef[2] = {
596+
APFloat::getSNaN(APFloat::IEEEdouble(), true, &intPayload_cdef),
597+
APFloat::getNaN(APFloat::IEEEdouble(), true, 0xcdef)};
598+
599+
for (APFloat n : {nan_0123[0], mnan_4567[0]})
600+
for (APFloat f : {f1, f2, zn, zp}) {
601+
APFloat res = minnum(f, n);
602+
EXPECT_FALSE(res.isNaN());
603+
EXPECT_TRUE(res.bitwiseIsEqual(f));
604+
res = minnum(n, f);
605+
EXPECT_FALSE(res.isNaN());
606+
EXPECT_TRUE(res.bitwiseIsEqual(f));
607+
}
608+
for (auto n : {nan_89ab, mnan_cdef})
609+
for (APFloat f : {f1, f2, zn, zp}) {
610+
APFloat res = minnum(f, n[0]);
611+
EXPECT_TRUE(res.isNaN());
612+
EXPECT_TRUE(res.bitwiseIsEqual(n[1]));
613+
res = minnum(n[0], f);
614+
EXPECT_TRUE(res.isNaN());
615+
EXPECT_TRUE(res.bitwiseIsEqual(n[1]));
616+
}
617+
618+
// When NaN vs NaN, we should keep payload/sign of either one.
619+
for (auto n1 : {nan_0123, mnan_4567, nan_89ab, mnan_cdef})
620+
for (auto n2 : {nan_0123, mnan_4567, nan_89ab, mnan_cdef}) {
621+
APFloat res = minnum(n1[0], n2[0]);
622+
EXPECT_TRUE(res.bitwiseIsEqual(n1[1]) || res.bitwiseIsEqual(n2[1]));
623+
EXPECT_FALSE(res.isSignaling());
624+
}
586625
}
587626

588627
TEST(APFloatTest, MaxNum) {
@@ -599,6 +638,46 @@ TEST(APFloatTest, MaxNum) {
599638
APFloat zn(-0.0);
600639
EXPECT_EQ(0.0, maxnum(zp, zn).convertToDouble());
601640
EXPECT_EQ(0.0, maxnum(zn, zp).convertToDouble());
641+
642+
APInt intPayload_89ab(64, 0x89ab);
643+
APInt intPayload_cdef(64, 0xcdef);
644+
APFloat nan_0123[2] = {APFloat::getNaN(APFloat::IEEEdouble(), false, 0x0123),
645+
APFloat::getNaN(APFloat::IEEEdouble(), false, 0x0123)};
646+
APFloat mnan_4567[2] = {APFloat::getNaN(APFloat::IEEEdouble(), true, 0x4567),
647+
APFloat::getNaN(APFloat::IEEEdouble(), true, 0x4567)};
648+
APFloat nan_89ab[2] = {
649+
APFloat::getSNaN(APFloat::IEEEdouble(), false, &intPayload_89ab),
650+
APFloat::getNaN(APFloat::IEEEdouble(), false, 0x89ab)};
651+
APFloat mnan_cdef[2] = {
652+
APFloat::getSNaN(APFloat::IEEEdouble(), true, &intPayload_cdef),
653+
APFloat::getNaN(APFloat::IEEEdouble(), true, 0xcdef)};
654+
655+
for (APFloat n : {nan_0123[0], mnan_4567[0]})
656+
for (APFloat f : {f1, f2, zn, zp}) {
657+
APFloat res = maxnum(f, n);
658+
EXPECT_FALSE(res.isNaN());
659+
EXPECT_TRUE(res.bitwiseIsEqual(f));
660+
res = maxnum(n, f);
661+
EXPECT_FALSE(res.isNaN());
662+
EXPECT_TRUE(res.bitwiseIsEqual(f));
663+
}
664+
for (auto n : {nan_89ab, mnan_cdef})
665+
for (APFloat f : {f1, f2, zn, zp}) {
666+
APFloat res = maxnum(f, n[0]);
667+
EXPECT_TRUE(res.isNaN());
668+
EXPECT_TRUE(res.bitwiseIsEqual(n[1]));
669+
res = maxnum(n[0], f);
670+
EXPECT_TRUE(res.isNaN());
671+
EXPECT_TRUE(res.bitwiseIsEqual(n[1]));
672+
}
673+
674+
// When NaN vs NaN, we should keep payload/sign of either one.
675+
for (auto n1 : {nan_0123, mnan_4567, nan_89ab, mnan_cdef})
676+
for (auto n2 : {nan_0123, mnan_4567, nan_89ab, mnan_cdef}) {
677+
APFloat res = maxnum(n1[0], n2[0]);
678+
EXPECT_TRUE(res.bitwiseIsEqual(n1[1]) || res.bitwiseIsEqual(n2[1]));
679+
EXPECT_FALSE(res.isSignaling());
680+
}
602681
}
603682

604683
TEST(APFloatTest, Minimum) {

0 commit comments

Comments
 (0)