Add minnum / maxnum to APFloat

[chandlerc]

include/llvm/ADT/APFloat.h

339–341

I think these should be free functions. I know that breaks with historical patterns here, but we should start moving the API in that direction IMO (and others I've chatted about this API with seemed to agree).

As part of that, this needs to return by-value. As-is, this can return a dangling reference to a temporary argument.

[arsenm]

Move to free function, return by value

[chandlerc]

Feel free to submit once suggestions are addressed.

include/llvm/ADT/APFloat.h

621–625

Please provide nice documenting comments for the semantics. Especially important to call out that these model IEEE's functionality and are not std::max.

Finally, why not inline? These seem reasonable inlining candidates....

[arsenm]

Moved inline + comments in r219475

include/llvm/ADT/APFloat.h

public:
   /// \brief Operator/ overload which provides the default
   /// \c nmNearestTiesToEven rounding mode and *no* error checking.
   APFloat operator/(const APFloat &RHS) const {
     APFloat Result = *this;
     Result.divide(RHS, rmNearestTiesToEven);
     return Result;
   }
 
   /// @}
 
   /// \name Sign operations.
   /// @{
 
   void changeSign();
   void clearSign();
   void copySign(const APFloat &);
 
   /// \brief A static helper to produce a copy of an APFloat value with its sign
   /// copied from some other APFloat.
private:
   unsigned int sign : 1;
 };
 
 /// See friend declaration above.
 ///
 /// This additional declaration is required in order to compile LLVM with IBM
 /// xlC compiler.
 hash_code hash_value(const APFloat &Arg);
+
+LLVM_READONLY
+APFloat minnum(const APFloat &, const APFloat &);
+
+LLVM_READONLY
+APFloat maxnum(const APFloat &, const APFloat &);
 } // namespace llvm
 
 #endif // LLVM_ADT_APFLOAT_H

lib/Support/APFloat.cpp

 
 void
 APFloat::makeZero(bool Negative) {
   category = fcZero;
   sign = Negative;
   exponent = semantics->minExponent-1;
   APInt::tcSet(significandParts(), 0, partCount());  
 }
+
+APFloat llvm::minnum(const APFloat &A, const APFloat &B) {
+  if (A.isNaN())
+    return B;
+  if (B.isNaN())
+    return A;
+  return (B.compare(A) == APFloat::cmpLessThan) ? B : A;
+}
+
+APFloat llvm::maxnum(const APFloat &A, const APFloat &B) {
+  if (A.isNaN())
+    return B;
+  if (B.isNaN())
+    return A;
+  return (A.compare(B) == APFloat::cmpLessThan) ? B : A;
+}

unittests/ADT/APFloatTest.cpp

APFloat::roundingMode rdmd = APFloat::rmNearestTiesToEven;
     f1.divide(Val2, rdmd);
     f2.divide(Val2, rdmd);
     APFloat f3(12.0f);
     f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
     EXPECT_EQ(12.0f, f1.convertToFloat());
   }
 }
 
+TEST(APFloatTest, MinNum) {
+  APFloat f1(1.0);
+  APFloat f2(2.0);
+  APFloat nan = APFloat::getNaN(APFloat::IEEEdouble);
+
+  EXPECT_EQ(1.0, minnum(f1, f2).convertToDouble());
+  EXPECT_EQ(1.0, minnum(f2, f1).convertToDouble());
+  EXPECT_EQ(1.0, minnum(f1, nan).convertToDouble());
+  EXPECT_EQ(1.0, minnum(nan, f1).convertToDouble());
+}
+
+TEST(APFloatTest, MaxNum) {
+  APFloat f1(1.0);
+  APFloat f2(2.0);
+  APFloat nan = APFloat::getNaN(APFloat::IEEEdouble);
+
+  EXPECT_EQ(2.0, maxnum(f1, f2).convertToDouble());
+  EXPECT_EQ(2.0, maxnum(f2, f1).convertToDouble());
+  EXPECT_EQ(1.0, maxnum(f1, nan).convertToDouble());
+  EXPECT_EQ(1.0, minnum(nan, f1).convertToDouble());
+}
+
 TEST(APFloatTest, Denormal) {
   APFloat::roundingMode rdmd = APFloat::rmNearestTiesToEven;
 
   // Test single precision
   {
     const char *MinNormalStr = "1.17549435082228750797e-38";
     EXPECT_FALSE(APFloat(APFloat::IEEEsingle, MinNormalStr).isDenormal());
     EXPECT_FALSE(APFloat(APFloat::IEEEsingle, 0.0).isDenormal());