[APFloat] Overload comparison operators

[foad]

Summary

These implement the usual IEEE-style floating point comparison
semantics, e.g. +0.0 == -0.0 and all operators except != return false
if either argument is NaN.

[arsenm]

llvm/unittests/ADT/APFloatTest.cpp

4273–4274

This should probably be as before, with separate tests for the full range of operators

[ekatz]

I like this change!
However, I think (as @arsenm) that the tests shouldn't be changed, but add separate new ones for the newly overloaded operators.

[foad]

Add unit tests instead of changing existing tests.

[ekatz]

Generally it LGTM.
But this change may be split into 2 as well (like the neg change).

I know that some times you see patches that change and add a lot of code (in the same patch), while in other cases they split into smaller ones. Both approaches are valid, but I think the better one is to split (if possible); especially in cases (like this) that change apply to files, from separate projects/modules, outside the one in which the new API is added.

So, I suggest split it into 2, where one adds the new operators and tests; and the other changes the compares in the different locations.

[foad]

Split changes to callers out into a separate patch.

[ekatz]

LGTM

llvm/include/llvm/ADT/APFloat.h

public:
   }
   Expected<opStatus> convertFromString(StringRef, roundingMode);
   APInt bitcastToAPInt() const {
     APFLOAT_DISPATCH_ON_SEMANTICS(bitcastToAPInt());
   }
   double convertToDouble() const { return getIEEE().convertToDouble(); }
   float convertToFloat() const { return getIEEE().convertToFloat(); }
 
-  bool operator==(const APFloat &) const = delete;
+  bool operator==(const APFloat &RHS) const { return compare(RHS) == cmpEqual; }
+
+  bool operator!=(const APFloat &RHS) const { return compare(RHS) != cmpEqual; }
+
+  bool operator<(const APFloat &RHS) const {
+    return compare(RHS) == cmpLessThan;
+  }
+
+  bool operator>(const APFloat &RHS) const {
+    return compare(RHS) == cmpGreaterThan;
+  }
+
+  bool operator<=(const APFloat &RHS) const {
+    cmpResult Res = compare(RHS);
+    return Res == cmpLessThan || Res == cmpEqual;
+  }
+
+  bool operator>=(const APFloat &RHS) const {
+    cmpResult Res = compare(RHS);
+    return Res == cmpGreaterThan || Res == cmpEqual;
+  }
 
   cmpResult compare(const APFloat &RHS) const {
     assert(&getSemantics() == &RHS.getSemantics() &&
            "Should only compare APFloats with the same semantics");
     if (usesLayout<IEEEFloat>(getSemantics()))
       return U.IEEE.compare(RHS.U.IEEE);
     if (usesLayout<DoubleAPFloat>(getSemantics()))
       return U.Double.compare(RHS.U.Double);

llvm/unittests/ADT/APFloatTest.cpp

TEST(APFloatTest, operatorOverloads) {
   APFloat One = APFloat(APFloat::IEEEsingle(), "0x1p+0");
   APFloat Two = APFloat(APFloat::IEEEsingle(), "0x2p+0");
   EXPECT_TRUE(Two.bitwiseIsEqual(One + One));
   EXPECT_TRUE(One.bitwiseIsEqual(Two - One));
   EXPECT_TRUE(Two.bitwiseIsEqual(One * Two));
   EXPECT_TRUE(One.bitwiseIsEqual(Two / Two));
 }
 
+TEST(APFloatTest, Comparisons) {
+  enum {MNan, MInf, MBig, MOne, MZer, PZer, POne, PBig, PInf, PNan, NumVals};
+  APFloat Vals[NumVals] = {
+    APFloat::getNaN(APFloat::IEEEsingle(), true),
+    APFloat::getInf(APFloat::IEEEsingle(), true),
+    APFloat::getLargest(APFloat::IEEEsingle(), true),
+    APFloat(APFloat::IEEEsingle(), "-0x1p+0"),
+    APFloat::getZero(APFloat::IEEEsingle(), true),
+    APFloat::getZero(APFloat::IEEEsingle(), false),
+    APFloat(APFloat::IEEEsingle(), "0x1p+0"),
+    APFloat::getLargest(APFloat::IEEEsingle(), false),
+    APFloat::getInf(APFloat::IEEEsingle(), false),
+    APFloat::getNaN(APFloat::IEEEsingle(), false),
+  };
+  using Relation = void (*)(const APFloat &, const APFloat &);
+  Relation LT = [](const APFloat &LHS, const APFloat &RHS) {
+    EXPECT_FALSE(LHS == RHS);
+    EXPECT_TRUE(LHS != RHS);
+    EXPECT_TRUE(LHS < RHS);
+    EXPECT_FALSE(LHS > RHS);
+    EXPECT_TRUE(LHS <= RHS);
+    EXPECT_FALSE(LHS >= RHS);
+  };
+  Relation EQ = [](const APFloat &LHS, const APFloat &RHS) {
+    EXPECT_TRUE(LHS == RHS);
+    EXPECT_FALSE(LHS != RHS);
+    EXPECT_FALSE(LHS < RHS);
+    EXPECT_FALSE(LHS > RHS);
+    EXPECT_TRUE(LHS <= RHS);
+    EXPECT_TRUE(LHS >= RHS);
+  };
+  Relation GT = [](const APFloat &LHS, const APFloat &RHS) {
+    EXPECT_FALSE(LHS == RHS);
+    EXPECT_TRUE(LHS != RHS);
+    EXPECT_FALSE(LHS < RHS);
+    EXPECT_TRUE(LHS > RHS);
+    EXPECT_FALSE(LHS <= RHS);
+    EXPECT_TRUE(LHS >= RHS);
+  };
+  Relation UN = [](const APFloat &LHS, const APFloat &RHS) {
+    EXPECT_FALSE(LHS == RHS);
+    EXPECT_TRUE(LHS != RHS);
+    EXPECT_FALSE(LHS < RHS);
+    EXPECT_FALSE(LHS > RHS);
+    EXPECT_FALSE(LHS <= RHS);
+    EXPECT_FALSE(LHS >= RHS);
+  };
+  Relation Relations[NumVals][NumVals] = {
+    //          -N  -I  -B  -1  -0  +0  +1  +B  +I  +N
+    /* MNan */ {UN, UN, UN, UN, UN, UN, UN, UN, UN, UN},
+    /* MInf */ {UN, EQ, LT, LT, LT, LT, LT, LT, LT, UN},
+    /* MBig */ {UN, GT, EQ, LT, LT, LT, LT, LT, LT, UN},
+    /* MOne */ {UN, GT, GT, EQ, LT, LT, LT, LT, LT, UN},
+    /* MZer */ {UN, GT, GT, GT, EQ, EQ, LT, LT, LT, UN},
+    /* PZer */ {UN, GT, GT, GT, EQ, EQ, LT, LT, LT, UN},
+    /* POne */ {UN, GT, GT, GT, GT, GT, EQ, LT, LT, UN},
+    /* PBig */ {UN, GT, GT, GT, GT, GT, GT, EQ, LT, UN},
+    /* PInf */ {UN, GT, GT, GT, GT, GT, GT, GT, EQ, UN},
+    /* PNan */ {UN, UN, UN, UN, UN, UN, UN, UN, UN, UN},
+  };
+  for (unsigned I = 0; I < NumVals; ++I)
+    for (unsigned J = 0; J < NumVals; ++J)
+      Relations[I][J](Vals[I], Vals[J]);
+}
+
 TEST(APFloatTest, abs) {
   APFloat PInf = APFloat::getInf(APFloat::IEEEsingle(), false);
   APFloat MInf = APFloat::getInf(APFloat::IEEEsingle(), true);
   APFloat PZero = APFloat::getZero(APFloat::IEEEsingle(), false);
   APFloat MZero = APFloat::getZero(APFloat::IEEEsingle(), true);
   APFloat PQNaN = APFloat::getNaN(APFloat::IEEEsingle(), false);
   APFloat MQNaN = APFloat::getNaN(APFloat::IEEEsingle(), true);
   APFloat PSNaN = APFloat::getSNaN(APFloat::IEEEsingle(), false);

 }
 
 TEST(APFloatTest, PPCDoubleDoubleFMA) {
   // Sanity check for now.
   APFloat A(APFloat::PPCDoubleDouble(), "2");
   A.fusedMultiplyAdd(APFloat(APFloat::PPCDoubleDouble(), "3"),
                      APFloat(APFloat::PPCDoubleDouble(), "4"),
                      APFloat::rmNearestTiesToEven);
   EXPECT_EQ(APFloat::cmpEqual,
             APFloat(APFloat::PPCDoubleDouble(), "10").compare(A));
 }
 
 TEST(APFloatTest, PPCDoubleDoubleRoundToIntegral) {
   {
     APFloat A(APFloat::PPCDoubleDouble(), "1.5");
     A.roundToIntegral(APFloat::rmNearestTiesToEven);
     EXPECT_EQ(APFloat::cmpEqual,
               APFloat(APFloat::PPCDoubleDouble(), "2").compare(A));