[clang] [llvm] APFloat: Fix maxnum and minnum with sNaN (PR #112854)
YunQiang Su via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 22 22:50:33 PDT 2024
https://github.com/wzssyqa updated https://github.com/llvm/llvm-project/pull/112854
>From ad7c6b648f3bb88ce075fa15cf8915350e4981ab Mon Sep 17 00:00:00 2001
From: YunQiang Su <yunqiang at isrc.iscas.ac.cn>
Date: Fri, 18 Oct 2024 16:33:19 +0800
Subject: [PATCH 1/2] APFloat: Fix maxnum and minnum with sNaN
See: https://github.com/llvm/llvm-project/pull/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.
---
clang/lib/AST/ExprConstant.cpp | 14 +-----
llvm/include/llvm/ADT/APFloat.h | 22 +++++---
llvm/unittests/ADT/APFloatTest.cpp | 81 +++++++++++++++++++++++++++++-
3 files changed, 98 insertions(+), 19 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 8e36cad2d2c6e7..6a336d880af5da 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -15338,16 +15338,11 @@ bool FloatExprEvaluator::VisitCallExpr(const CallExpr *E) {
case Builtin::BI__builtin_fmaxl:
case Builtin::BI__builtin_fmaxf16:
case Builtin::BI__builtin_fmaxf128: {
- // TODO: Handle sNaN.
APFloat RHS(0.);
if (!EvaluateFloat(E->getArg(0), Result, Info) ||
!EvaluateFloat(E->getArg(1), RHS, Info))
return false;
- // When comparing zeroes, return +0.0 if one of the zeroes is positive.
- if (Result.isZero() && RHS.isZero() && Result.isNegative())
- Result = RHS;
- else if (Result.isNaN() || RHS > Result)
- Result = RHS;
+ Result = maxnum(Result, RHS);
return true;
}
@@ -15356,16 +15351,11 @@ bool FloatExprEvaluator::VisitCallExpr(const CallExpr *E) {
case Builtin::BI__builtin_fminl:
case Builtin::BI__builtin_fminf16:
case Builtin::BI__builtin_fminf128: {
- // TODO: Handle sNaN.
APFloat RHS(0.);
if (!EvaluateFloat(E->getArg(0), Result, Info) ||
!EvaluateFloat(E->getArg(1), RHS, Info))
return false;
- // When comparing zeroes, return -0.0 if one of the zeroes is negative.
- if (Result.isZero() && RHS.isZero() && RHS.isNegative())
- Result = RHS;
- else if (Result.isNaN() || RHS < Result)
- Result = RHS;
+ Result = minnum(Result, RHS);
return true;
}
diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h
index 97547fb577e0ec..d8f08e4ed2cd9a 100644
--- a/llvm/include/llvm/ADT/APFloat.h
+++ b/llvm/include/llvm/ADT/APFloat.h
@@ -1515,11 +1515,16 @@ inline APFloat neg(APFloat X) {
return X;
}
-/// Implements IEEE-754 2019 minimumNumber semantics. Returns the smaller of the
-/// 2 arguments if both are not NaN. If either argument is a NaN, returns the
-/// other argument. -0 is treated as ordered less than +0.
+/// Implements IEEE-754 2008 minNum semantics. Returns the smaller of the
+/// 2 arguments if both are not NaN. If either argument is a qNaN, returns the
+/// other argument. If either argument is sNaN, return a qNaN.
+/// -0 is treated as ordered less than +0.
LLVM_READONLY
inline APFloat minnum(const APFloat &A, const APFloat &B) {
+ if (A.isSignaling())
+ return A.makeQuiet();
+ if (B.isSignaling())
+ return B.makeQuiet();
if (A.isNaN())
return B;
if (B.isNaN())
@@ -1529,11 +1534,16 @@ inline APFloat minnum(const APFloat &A, const APFloat &B) {
return B < A ? B : A;
}
-/// Implements IEEE-754 2019 maximumNumber semantics. Returns the larger of the
-/// 2 arguments if both are not NaN. If either argument is a NaN, returns the
-/// other argument. +0 is treated as ordered greater than -0.
+/// Implements IEEE-754 2008 maxNum semantics. Returns the larger of the
+/// 2 arguments if both are not NaN. If either argument is a qNaN, returns the
+/// other argument. If either argument is sNaN, return a qNaN.
+/// +0 is treated as ordered greater than -0.
LLVM_READONLY
inline APFloat maxnum(const APFloat &A, const APFloat &B) {
+ if (A.isSignaling())
+ return A.makeQuiet();
+ if (B.isSignaling())
+ return B.makeQuiet();
if (A.isNaN())
return B;
if (B.isNaN())
diff --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp
index 74aaf66973a19f..6a33dcd9fdf9f4 100644
--- a/llvm/unittests/ADT/APFloatTest.cpp
+++ b/llvm/unittests/ADT/APFloatTest.cpp
@@ -582,7 +582,46 @@ TEST(APFloatTest, MinNum) {
APFloat zp(0.0);
APFloat zn(-0.0);
EXPECT_EQ(-0.0, minnum(zp, zn).convertToDouble());
- EXPECT_EQ(-0.0, minnum(zn, zp).convertToDouble());
+
+ APInt intPayload_89ab(64, 0x89ab);
+ APInt intPayload_cdef(64, 0xcdef);
+ APFloat nan_0123[2] = {APFloat::getNaN(APFloat::IEEEdouble(), false, 0x0123),
+ APFloat::getNaN(APFloat::IEEEdouble(), false, 0x0123)};
+ APFloat mnan_4567[2] = {APFloat::getNaN(APFloat::IEEEdouble(), true, 0x4567),
+ APFloat::getNaN(APFloat::IEEEdouble(), true, 0x4567)};
+ APFloat nan_89ab[2] = {
+ APFloat::getSNaN(APFloat::IEEEdouble(), false, &intPayload_89ab),
+ APFloat::getNaN(APFloat::IEEEdouble(), false, 0x89ab)};
+ APFloat mnan_cdef[2] = {
+ APFloat::getSNaN(APFloat::IEEEdouble(), true, &intPayload_cdef),
+ APFloat::getNaN(APFloat::IEEEdouble(), true, 0xcdef)};
+
+ for (APFloat n : {nan_0123[0], mnan_4567[0]})
+ for (APFloat f : {f1, f2, zn, zp}) {
+ APFloat res = minnum(f, n);
+ EXPECT_FALSE(res.isNaN());
+ EXPECT_TRUE(res.bitwiseIsEqual(f));
+ res = minnum(n, f);
+ EXPECT_FALSE(res.isNaN());
+ EXPECT_TRUE(res.bitwiseIsEqual(f));
+ }
+ for (auto n : {nan_89ab, mnan_cdef})
+ for (APFloat f : {f1, f2, zn, zp}) {
+ APFloat res = minnum(f, n[0]);
+ EXPECT_TRUE(res.isNaN());
+ EXPECT_TRUE(res.bitwiseIsEqual(n[1]));
+ res = minnum(n[0], f);
+ EXPECT_TRUE(res.isNaN());
+ EXPECT_TRUE(res.bitwiseIsEqual(n[1]));
+ }
+
+ // When NaN vs NaN, we should keep payload/sign of either one.
+ for (auto n1 : {nan_0123, mnan_4567, nan_89ab, mnan_cdef})
+ for (auto n2 : {nan_0123, mnan_4567, nan_89ab, mnan_cdef}) {
+ APFloat res = minnum(n1[0], n2[0]);
+ EXPECT_TRUE(res.bitwiseIsEqual(n1[1]) || res.bitwiseIsEqual(n2[1]));
+ EXPECT_FALSE(res.isSignaling());
+ }
}
TEST(APFloatTest, MaxNum) {
@@ -599,6 +638,46 @@ TEST(APFloatTest, MaxNum) {
APFloat zn(-0.0);
EXPECT_EQ(0.0, maxnum(zp, zn).convertToDouble());
EXPECT_EQ(0.0, maxnum(zn, zp).convertToDouble());
+
+ APInt intPayload_89ab(64, 0x89ab);
+ APInt intPayload_cdef(64, 0xcdef);
+ APFloat nan_0123[2] = {APFloat::getNaN(APFloat::IEEEdouble(), false, 0x0123),
+ APFloat::getNaN(APFloat::IEEEdouble(), false, 0x0123)};
+ APFloat mnan_4567[2] = {APFloat::getNaN(APFloat::IEEEdouble(), true, 0x4567),
+ APFloat::getNaN(APFloat::IEEEdouble(), true, 0x4567)};
+ APFloat nan_89ab[2] = {
+ APFloat::getSNaN(APFloat::IEEEdouble(), false, &intPayload_89ab),
+ APFloat::getNaN(APFloat::IEEEdouble(), false, 0x89ab)};
+ APFloat mnan_cdef[2] = {
+ APFloat::getSNaN(APFloat::IEEEdouble(), true, &intPayload_cdef),
+ APFloat::getNaN(APFloat::IEEEdouble(), true, 0xcdef)};
+
+ for (APFloat n : {nan_0123[0], mnan_4567[0]})
+ for (APFloat f : {f1, f2, zn, zp}) {
+ APFloat res = maxnum(f, n);
+ EXPECT_FALSE(res.isNaN());
+ EXPECT_TRUE(res.bitwiseIsEqual(f));
+ res = maxnum(n, f);
+ EXPECT_FALSE(res.isNaN());
+ EXPECT_TRUE(res.bitwiseIsEqual(f));
+ }
+ for (auto n : {nan_89ab, mnan_cdef})
+ for (APFloat f : {f1, f2, zn, zp}) {
+ APFloat res = maxnum(f, n[0]);
+ EXPECT_TRUE(res.isNaN());
+ EXPECT_TRUE(res.bitwiseIsEqual(n[1]));
+ res = maxnum(n[0], f);
+ EXPECT_TRUE(res.isNaN());
+ EXPECT_TRUE(res.bitwiseIsEqual(n[1]));
+ }
+
+ // When NaN vs NaN, we should keep payload/sign of either one.
+ for (auto n1 : {nan_0123, mnan_4567, nan_89ab, mnan_cdef})
+ for (auto n2 : {nan_0123, mnan_4567, nan_89ab, mnan_cdef}) {
+ APFloat res = maxnum(n1[0], n2[0]);
+ EXPECT_TRUE(res.bitwiseIsEqual(n1[1]) || res.bitwiseIsEqual(n2[1]));
+ EXPECT_FALSE(res.isSignaling());
+ }
}
TEST(APFloatTest, Minimum) {
>From cc90c8a1ead851289c8b37a9f763037f23c1d230 Mon Sep 17 00:00:00 2001
From: YunQiang Su <yunqiang at isrc.iscas.ac.cn>
Date: Wed, 23 Oct 2024 13:49:51 +0800
Subject: [PATCH 2/2] remove clang changes: should be another patch
---
clang/lib/AST/ExprConstant.cpp | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6a336d880af5da..8e36cad2d2c6e7 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -15338,11 +15338,16 @@ bool FloatExprEvaluator::VisitCallExpr(const CallExpr *E) {
case Builtin::BI__builtin_fmaxl:
case Builtin::BI__builtin_fmaxf16:
case Builtin::BI__builtin_fmaxf128: {
+ // TODO: Handle sNaN.
APFloat RHS(0.);
if (!EvaluateFloat(E->getArg(0), Result, Info) ||
!EvaluateFloat(E->getArg(1), RHS, Info))
return false;
- Result = maxnum(Result, RHS);
+ // When comparing zeroes, return +0.0 if one of the zeroes is positive.
+ if (Result.isZero() && RHS.isZero() && Result.isNegative())
+ Result = RHS;
+ else if (Result.isNaN() || RHS > Result)
+ Result = RHS;
return true;
}
@@ -15351,11 +15356,16 @@ bool FloatExprEvaluator::VisitCallExpr(const CallExpr *E) {
case Builtin::BI__builtin_fminl:
case Builtin::BI__builtin_fminf16:
case Builtin::BI__builtin_fminf128: {
+ // TODO: Handle sNaN.
APFloat RHS(0.);
if (!EvaluateFloat(E->getArg(0), Result, Info) ||
!EvaluateFloat(E->getArg(1), RHS, Info))
return false;
- Result = minnum(Result, RHS);
+ // When comparing zeroes, return -0.0 if one of the zeroes is negative.
+ if (Result.isZero() && RHS.isZero() && RHS.isNegative())
+ Result = RHS;
+ else if (Result.isNaN() || RHS < Result)
+ Result = RHS;
return true;
}
More information about the cfe-commits
mailing list