APFloat: Fix maxnum and minnum with sNaN #112854
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: YunQiang Su (wzssyqa) ChangesSee: #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. Full diff: https://github.com/llvm/llvm-project/pull/112854.diff 3 Files Affected:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 8544052d5e4924..040538f28e44df 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -15324,16 +15324,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;
}
@@ -15342,16 +15337,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 665cff9c424594..b21f5ff4400f32 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) {
|
|
@llvm/pr-subscribers-llvm-adt Author: YunQiang Su (wzssyqa) ChangesSee: #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. Full diff: https://github.com/llvm/llvm-project/pull/112854.diff 3 Files Affected:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 8544052d5e4924..040538f28e44df 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -15324,16 +15324,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;
}
@@ -15342,16 +15337,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 665cff9c424594..b21f5ff4400f32 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) {
|
arsenm
reviewed
Oct 21, 2024
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.
wzssyqa
force-pushed
the
apfloat-maxnum
branch
from
9b164c7 to
cc90c8a
Compare
|
@arsenm ping |
arsenm
approved these changes
Feb 27, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/4769 Here is the relevant piece of the build log for the reference |
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Mar 3, 2025
See: llvm#112852 Fixes: llvm#111991 We have reclarify llvm.maxnum and llvm.minnum to follow IEEE-754 2008's maxNum and minNum with +0.0>-0.0. So let's make APFloat::maxnum and APFloat::minnum to follow it, too.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See: #112852
Fixes: #111991
We have reclarify llvm.maxnum and llvm.minnum to follow IEEE-754 2008's maxNum and minNum with +0.0>-0.0.
So let's make APFloat::maxnum and APFloat::minnum to follow it, too.