[llvm] f961b61 - [APFloat] Properly implement DoubleAPFloat::compareAbsoluteValue
David Majnemer via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 21 15:53:58 PDT 2025
Author: David Majnemer
Date: 2025-08-21T15:42:07-07:00
New Revision: f961b61f8841c8488ab2194ac7a82e557736a949
URL: https://github.com/llvm/llvm-project/commit/f961b61f8841c8488ab2194ac7a82e557736a949
DIFF: https://github.com/llvm/llvm-project/commit/f961b61f8841c8488ab2194ac7a82e557736a949.diff
LOG: [APFloat] Properly implement DoubleAPFloat::compareAbsoluteValue
The prior implementation would treat X+Y and X-Y as having equal
magnitude. Rework the implementation to be more resilient.
Added:
Modified:
llvm/include/llvm/ADT/APFloat.h
llvm/lib/Support/APFloat.cpp
llvm/unittests/ADT/APFloatTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h
index b756fc8ffce00..b5429db382e2c 100644
--- a/llvm/include/llvm/ADT/APFloat.h
+++ b/llvm/include/llvm/ADT/APFloat.h
@@ -1045,6 +1045,8 @@ class APFloat : public APFloatBase {
explicit APFloat(DoubleAPFloat F, const fltSemantics &S)
: U(std::move(F), S) {}
+ // Compares the absolute value of this APFloat with another. Both operands
+ // must be finite non-zero.
cmpResult compareAbsoluteValue(const APFloat &RHS) const {
assert(&getSemantics() == &RHS.getSemantics() &&
"Should only compare APFloats with the same semantics");
@@ -1408,6 +1410,8 @@ class APFloat : public APFloatBase {
return Res == cmpGreaterThan || Res == cmpEqual;
}
+ // IEEE comparison with another floating point number (NaNs compare unordered,
+ // 0==-0).
cmpResult compare(const APFloat &RHS) const {
assert(&getSemantics() == &RHS.getSemantics() &&
"Should only compare APFloats with the same semantics");
diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 986ac301da739..aa5b3c78ea5f5 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -5285,23 +5285,56 @@ void DoubleAPFloat::changeSign() {
APFloat::cmpResult
DoubleAPFloat::compareAbsoluteValue(const DoubleAPFloat &RHS) const {
- auto Result = Floats[0].compareAbsoluteValue(RHS.Floats[0]);
- if (Result != cmpEqual)
- return Result;
- Result = Floats[1].compareAbsoluteValue(RHS.Floats[1]);
- if (Result == cmpLessThan || Result == cmpGreaterThan) {
- auto Against = Floats[0].isNegative() ^ Floats[1].isNegative();
- auto RHSAgainst = RHS.Floats[0].isNegative() ^ RHS.Floats[1].isNegative();
- if (Against && !RHSAgainst)
- return cmpLessThan;
- if (!Against && RHSAgainst)
+ // Compare absolute values of the high parts.
+ const cmpResult HiPartCmp = Floats[0].compareAbsoluteValue(RHS.Floats[0]);
+ if (HiPartCmp != cmpEqual)
+ return HiPartCmp;
+
+ // Zero, regardless of sign, is equal.
+ if (Floats[1].isZero() && RHS.Floats[1].isZero())
+ return cmpEqual;
+
+ // At this point, |this->Hi| == |RHS.Hi|.
+ // The magnitude is |Hi+Lo| which is Hi+|Lo| if signs of Hi and Lo are the
+ // same, and Hi-|Lo| if signs are
diff erent.
+ const bool ThisIsSubtractive =
+ Floats[0].isNegative() != Floats[1].isNegative();
+ const bool RHSIsSubtractive =
+ RHS.Floats[0].isNegative() != RHS.Floats[1].isNegative();
+
+ // Case 1: The low part of 'this' is zero.
+ if (Floats[1].isZero())
+ // We are comparing |Hi| vs. |Hi| ± |RHS.Lo|.
+ // If RHS is subtractive, its magnitude is smaller.
+ // If RHS is additive, its magnitude is larger.
+ return RHSIsSubtractive ? cmpGreaterThan : cmpLessThan;
+
+ // Case 2: The low part of 'RHS' is zero (and we know 'this' is not).
+ if (RHS.Floats[1].isZero())
+ // We are comparing |Hi| ± |This.Lo| vs. |Hi|.
+ // If 'this' is subtractive, its magnitude is smaller.
+ // If 'this' is additive, its magnitude is larger.
+ return ThisIsSubtractive ? cmpLessThan : cmpGreaterThan;
+
+ // If their natures
diff er, the additive one is larger.
+ if (ThisIsSubtractive != RHSIsSubtractive)
+ return ThisIsSubtractive ? cmpLessThan : cmpGreaterThan;
+
+ // Case 3: Both are additive (Hi+|Lo|) or both are subtractive (Hi-|Lo|).
+ // The comparison now depends on the magnitude of the low parts.
+ const cmpResult LoPartCmp = Floats[1].compareAbsoluteValue(RHS.Floats[1]);
+
+ if (ThisIsSubtractive) {
+ // Both are subtractive (Hi-|Lo|), so the comparison of |Lo| is inverted.
+ if (LoPartCmp == cmpLessThan)
return cmpGreaterThan;
- if (!Against && !RHSAgainst)
- return Result;
- if (Against && RHSAgainst)
- return (cmpResult)(cmpLessThan + cmpGreaterThan - Result);
+ if (LoPartCmp == cmpGreaterThan)
+ return cmpLessThan;
}
- return Result;
+
+ // If additive, the comparison of |Lo| is direct.
+ // If equal, they are equal.
+ return LoPartCmp;
}
APFloat::fltCategory DoubleAPFloat::getCategory() const {
diff --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp
index a9aa55c9d50a2..b0435cc21da58 100644
--- a/llvm/unittests/ADT/APFloatTest.cpp
+++ b/llvm/unittests/ADT/APFloatTest.cpp
@@ -6207,6 +6207,102 @@ TEST(APFloatTest, PPCDoubleDoubleCompare) {
}
}
+namespace PPCDoubleDoubleCompareAbsoluteValueTestDetails {
+struct TestCase {
+ DD LHS;
+ DD RHS;
+ APFloat::cmpResult Result;
+};
+
+auto testCases() {
+ static constexpr auto CompareAbsoluteValueTestCases = std::array{
+ TestCase{
+ {1.0, 0.0},
+ {1.0, 0.0},
+ APFloat::cmpEqual,
+ },
+ TestCase{
+ {1.0, -0.0},
+ {1.0, +0.0},
+ APFloat::cmpEqual,
+ },
+ TestCase{
+ {1.0, 0.0},
+ {0x1.0000000000001p+0, 0.0},
+ APFloat::cmpLessThan,
+ },
+ TestCase{
+ {0x1.0000000000001p+0, 0.0},
+ {1.0, 0.0},
+ APFloat::cmpGreaterThan,
+ },
+ TestCase{
+ {0x1.0000000000001p+0, +0x1p-1074},
+ {1.0, -0x1p-1074},
+ APFloat::cmpGreaterThan,
+ },
+ TestCase{
+ {0x1.0000000000001p+0, -0x1p-1074},
+ {1.0, +0x1p-1074},
+ APFloat::cmpGreaterThan,
+ },
+ TestCase{
+ {1.0, 0.0},
+ {1.0, -0x1p-1074},
+ APFloat::cmpGreaterThan,
+ },
+ TestCase{
+ {1.0, 0.0},
+ {1.0, +0x1p-1074},
+ APFloat::cmpLessThan,
+ },
+ TestCase{
+ {1.0, +0x1p-1073},
+ {1.0, -0x1p-1074},
+ APFloat::cmpGreaterThan,
+ },
+ TestCase{
+ {1.0, +0x1p-1074},
+ {1.0, -0x1p-1074},
+ APFloat::cmpGreaterThan,
+ },
+ };
+ return CompareAbsoluteValueTestCases;
+}
+} // namespace PPCDoubleDoubleCompareAbsoluteValueTestDetails
+
+class PPCDoubleDoubleCompareAbsoluteValueValueTest
+ : public testing::Test,
+ public ::testing::WithParamInterface<
+ PPCDoubleDoubleCompareAbsoluteValueTestDetails::TestCase> {};
+
+INSTANTIATE_TEST_SUITE_P(
+ PPCDoubleDoubleCompareAbsoluteValueValueParamTests,
+ PPCDoubleDoubleCompareAbsoluteValueValueTest,
+ ::testing::ValuesIn(
+ PPCDoubleDoubleCompareAbsoluteValueTestDetails::testCases()));
+
+TEST_P(PPCDoubleDoubleCompareAbsoluteValueValueTest,
+ PPCDoubleDoubleCompareAbsoluteValue) {
+ auto Param = GetParam();
+ for (bool LHSNegate : {false, true}) {
+ auto LHS = llvm::detail::DoubleAPFloat{APFloat::PPCDoubleDouble(),
+ APFloat{Param.LHS.Hi},
+ APFloat{Param.LHS.Lo}};
+ if (LHSNegate)
+ LHS.changeSign();
+ for (bool RHSNegate : {false, true}) {
+ auto RHS = llvm::detail::DoubleAPFloat{APFloat::PPCDoubleDouble(),
+ APFloat{Param.RHS.Hi},
+ APFloat{Param.RHS.Lo}};
+ if (RHSNegate)
+ RHS.changeSign();
+
+ EXPECT_EQ(LHS.compareAbsoluteValue(RHS), Param.Result);
+ }
+ }
+}
+
TEST(APFloatTest, PPCDoubleDoubleBitwiseIsEqual) {
using DataType = std::tuple<uint64_t, uint64_t, uint64_t, uint64_t, bool>;
More information about the llvm-commits
mailing list