[APFloat] Make functions that produce APFloaat objects use correct semantics.

[timshen]

Summary

Fixes PR30869.

In D25977 I meant to change all functions that care about lifetime. I changed constructors, factory functions, but I missed member/free functions that return new instances. This patch changes them.

[hfinkel]

Can you please explain the problem/solution?

llvm/include/llvm/ADT/APFloat.h

645 ↗

(On Diff #76810)

Why is S unused? If we're assuming that semantics is initialized elsewhere, a comment would be useful explaining why we have the argument.

[timshen]

Actually use the Semantics that is passed into APFloat::Storage::Storage.

[timshen]

In D26269#586853, @hfinkel wrote:

Can you please explain the problem/solution?

Added. Sorry for the oversight!

[timshen]

In D26269#586853, @hfinkel wrote:

Can you please explain the problem/solution?

Added. Sorry for the oversight!

llvm/include/llvm/ADT/APFloat.h

645 ↗

(On Diff #76810)

It should be propagated into DoubleAPFloat, but currently DoubleAPFloat doesn't need to support conversion between different semantics (it only has one!). Put an assert there, and someday DoubleAPFloat may change to actually consume this fltSemantic.

[hfinkel]

LGTM

llvm/trunk/include/llvm/ADT/APFloat.h

public:
   /// @}
 
   /// \brief Returns whether this instance allocated memory.
   bool needsCleanup() const { return partCount() > 1; }
 
   /// \name Convenience "constructors"
   /// @{
 
-  /// Returns a float which is bitcasted from an all one value int.
-  ///
-  /// \param BitWidth - Select float type
-  static IEEEFloat getAllOnesValue(unsigned BitWidth);
-
   /// @}
 
   /// Used to insert APFloat objects, or objects that contain APFloat objects,
   /// into FoldingSets.
   void Profile(FoldingSetNodeID &NID) const;
 
   /// \name Arithmetic
   /// @{
class APFloat : public APFloatBase {
 
   static_assert(std::is_standard_layout<IEEEFloat>::value, "");
 
   union Storage {
     const fltSemantics *semantics;
     IEEEFloat IEEE;
     DoubleAPFloat Double;
 
-    explicit Storage(IEEEFloat F) : IEEE(std::move(F)) {}
-    explicit Storage(DoubleAPFloat F) : Double(std::move(F)) {}
+    explicit Storage(IEEEFloat F, const fltSemantics &S);
+    explicit Storage(DoubleAPFloat F, const fltSemantics &S)
+        : Double(std::move(F)) {
+      assert(&S == &PPCDoubleDouble);
+    }
 
     template <typename... ArgTypes>
     Storage(const fltSemantics &Semantics, ArgTypes &&... Args) {
       if (usesLayout<IEEEFloat>(Semantics)) {
         new (&IEEE) IEEEFloat(Semantics, std::forward<ArgTypes>(Args)...);
       } else if (usesLayout<DoubleAPFloat>(Semantics)) {
         new (&Double) DoubleAPFloat(Semantics, std::forward<ArgTypes>(Args)...);
       } else {
class APFloat : public APFloatBase {
 
   // FIXME: This is due to clang 3.3 (or older version) always checks for the
   // default constructor in an array aggregate initialization, even if no
   // elements in the array is default initialized.
   APFloat() : U(IEEEdouble) {
     llvm_unreachable("This is a workaround for old clang.");
   }
 
-  explicit APFloat(IEEEFloat F) : U(std::move(F)) {}
-  explicit APFloat(DoubleAPFloat F) : U(std::move(F)) {}
+  explicit APFloat(IEEEFloat F, const fltSemantics &S) : U(std::move(F), S) {}
+  explicit APFloat(DoubleAPFloat F, const fltSemantics &S)
+      : U(std::move(F), S) {}
 
 public:
   APFloat(const fltSemantics &Semantics) : U(Semantics) {}
   APFloat(const fltSemantics &Semantics, StringRef S);
   APFloat(const fltSemantics &Semantics, integerPart I) : U(Semantics, I) {}
   // TODO: Remove this constructor. This isn't faster than the first one.
   APFloat(const fltSemantics &Semantics, uninitializedTag)
       : U(Semantics, uninitialized) {}
   APFloat(const fltSemantics &Semantics, const APInt &I) : U(Semantics, I) {}
-  explicit APFloat(double d) : U(IEEEFloat(d)) {}
-  explicit APFloat(float f) : U(IEEEFloat(f)) {}
+  explicit APFloat(double d) : U(IEEEFloat(d), IEEEdouble) {}
+  explicit APFloat(float f) : U(IEEEFloat(f), IEEEsingle) {}
   APFloat(const APFloat &RHS) = default;
   APFloat(APFloat &&RHS) = default;
 
   ~APFloat() = default;
 
   bool needsCleanup() const {
     if (usesLayout<IEEEFloat>(getSemantics())) {
       return U.IEEE.needsCleanup();
static APFloat getSmallestNormalized(const fltSemantics &Sem,
     Val.makeSmallestNormalized(Negative);
     return Val;
   }
 
   /// Returns a float which is bitcasted from an all one value int.
   ///
   /// \param BitWidth - Select float type
   /// \param isIEEE   - If 128 bit number, select between PPC and IEEE
-  static APFloat getAllOnesValue(unsigned BitWidth, bool isIEEE = false) {
-    if (isIEEE) {
-      return APFloat(IEEEFloat::getAllOnesValue(BitWidth));
-    } else {
-      assert(BitWidth == 128);
-      return APFloat(PPCDoubleDouble, APInt::getAllOnesValue(BitWidth));
-    }
-  }
+  static APFloat getAllOnesValue(unsigned BitWidth, bool isIEEE = false);
 
   void Profile(FoldingSetNodeID &NID) const { getIEEE().Profile(NID); }
 
   opStatus add(const APFloat &RHS, roundingMode RM) {
     return getIEEE().add(RHS.getIEEE(), RM);
   }
   opStatus subtract(const APFloat &RHS, roundingMode RM) {
     return getIEEE().subtract(RHS.getIEEE(), RM);
return getIEEE().fusedMultiplyAdd(Multiplicand.getIEEE(), Addend.getIEEE(),
                                       RM);
   }
   opStatus roundToIntegral(roundingMode RM) {
     return getIEEE().roundToIntegral(RM);
   }
   opStatus next(bool nextDown) { return getIEEE().next(nextDown); }
 
   APFloat operator+(const APFloat &RHS) const {
-    return APFloat(getIEEE() + RHS.getIEEE());
+    return APFloat(getIEEE() + RHS.getIEEE(), getSemantics());
   }
 
   APFloat operator-(const APFloat &RHS) const {
-    return APFloat(getIEEE() - RHS.getIEEE());
+    return APFloat(getIEEE() - RHS.getIEEE(), getSemantics());
   }
 
   APFloat operator*(const APFloat &RHS) const {
-    return APFloat(getIEEE() * RHS.getIEEE());
+    return APFloat(getIEEE() * RHS.getIEEE(), getSemantics());
   }
 
   APFloat operator/(const APFloat &RHS) const {
-    return APFloat(getIEEE() / RHS.getIEEE());
+    return APFloat(getIEEE() / RHS.getIEEE(), getSemantics());
   }
 
   void changeSign() { getIEEE().changeSign(); }
   void clearSign() { getIEEE().clearSign(); }
   void copySign(const APFloat &RHS) { getIEEE().copySign(RHS.getIEEE()); }
 
   static APFloat copySign(APFloat Value, const APFloat &Sign) {
-    return APFloat(IEEEFloat::copySign(Value.getIEEE(), Sign.getIEEE()));
+    return APFloat(IEEEFloat::copySign(Value.getIEEE(), Sign.getIEEE()),
+                   Value.getSemantics());
   }
 
   opStatus convert(const fltSemantics &ToSemantics, roundingMode RM,
                    bool *losesInfo);
   opStatus convertToInteger(integerPart *Input, unsigned int Width,
                             bool IsSigned, roundingMode RM,
                             bool *IsExact) const {
     return getIEEE().convertToInteger(Input, Width, IsSigned, RM, IsExact);

 };
 
 /// See friend declarations above.
 ///
 /// These additional declarations are required in order to compile LLVM with IBM
 /// xlC compiler.
 hash_code hash_value(const APFloat &Arg);
 inline APFloat scalbn(APFloat X, int Exp, APFloat::roundingMode RM) {
-  return APFloat(scalbn(X.getIEEE(), Exp, RM));
+  return APFloat(scalbn(X.getIEEE(), Exp, RM), X.getSemantics());
 }
 
 /// \brief Equivalent of C standard library function.
 ///
 /// While the C standard says Exp is an unspecified value for infinity and nan,
 /// this returns INT_MAX for infinities, and INT_MIN for NaNs.
 inline APFloat frexp(const APFloat &X, int &Exp, APFloat::roundingMode RM) {
-  return APFloat(frexp(X.getIEEE(), Exp, RM));
+  return APFloat(frexp(X.getIEEE(), Exp, RM), X.getSemantics());
 }
 /// \brief Returns the absolute value of the argument.
 inline APFloat abs(APFloat X) {
   X.clearSign();
   return X;
 }
 
 /// Implements IEEE minNum semantics. Returns the smaller of the 2 arguments if

llvm/trunk/lib/Support/APFloat.cpp

void IEEEFloat::initFromAPInt(const fltSemantics *Sem, const APInt &api) {
   if (Sem == &IEEEquad)
     return initFromQuadrupleAPInt(api);
   if (Sem == &PPCDoubleDoubleImpl)
     return initFromPPCDoubleDoubleAPInt(api);
 
   llvm_unreachable(nullptr);
 }
 
-IEEEFloat IEEEFloat::getAllOnesValue(unsigned BitWidth) {
-  switch (BitWidth) {
-  case 16:
-    return IEEEFloat(IEEEhalf, APInt::getAllOnesValue(BitWidth));
-  case 32:
-    return IEEEFloat(IEEEsingle, APInt::getAllOnesValue(BitWidth));
-  case 64:
-    return IEEEFloat(IEEEdouble, APInt::getAllOnesValue(BitWidth));
-  case 80:
-    return IEEEFloat(x87DoubleExtended, APInt::getAllOnesValue(BitWidth));
-  case 128:
-    return IEEEFloat(IEEEquad, APInt::getAllOnesValue(BitWidth));
-  default:
-    llvm_unreachable("Unknown floating bit width");
-  }
-}
-
 /// Make this number the largest magnitude normal number in the given
 /// semantics.
 void IEEEFloat::makeLargest(bool Negative) {
   // We want (in interchange format):
   //   sign = {Negative}
   //   exponent = 1..10
   //   significand = 1..1
   category = fcNormal;
if (Semantics == RHS.Semantics && RHS.Floats) {
     this->~DoubleAPFloat();
     new (this) DoubleAPFloat(RHS);
   }
   return *this;
 }
 
 } // End detail namespace
 
+APFloat::Storage::Storage(IEEEFloat F, const fltSemantics &Semantics) {
+  if (usesLayout<IEEEFloat>(Semantics)) {
+    new (&IEEE) IEEEFloat(std::move(F));
+  } else if (usesLayout<DoubleAPFloat>(Semantics)) {
+    new (&Double)
+        DoubleAPFloat(Semantics, APFloat(std::move(F), F.getSemantics()),
+                      APFloat(IEEEdouble));
+  } else {
+    llvm_unreachable("Unexpected semantics");
+  }
+}
+
 APFloat::opStatus APFloat::convertFromString(StringRef Str, roundingMode RM) {
   return getIEEE().convertFromString(Str, RM);
 }
 
 hash_code hash_value(const APFloat &Arg) { return hash_value(Arg.getIEEE()); }
 
 APFloat::APFloat(const fltSemantics &Semantics, StringRef S)
     : APFloat(Semantics) {
   convertFromString(S, rmNearestTiesToEven);
 }
 
 APFloat::opStatus APFloat::convert(const fltSemantics &ToSemantics,
                                    roundingMode RM, bool *losesInfo) {
   if (&getSemantics() == &ToSemantics)
     return opOK;
   if (usesLayout<IEEEFloat>(getSemantics()) &&
       usesLayout<IEEEFloat>(ToSemantics)) {
     return U.IEEE.convert(ToSemantics, RM, losesInfo);
   } else if (usesLayout<IEEEFloat>(getSemantics()) &&
              usesLayout<DoubleAPFloat>(ToSemantics)) {
     assert(&ToSemantics == &PPCDoubleDouble);
     auto Ret = U.IEEE.convert(PPCDoubleDoubleImpl, RM, losesInfo);
     *this = APFloat(
-        DoubleAPFloat(PPCDoubleDouble, std::move(*this), APFloat(IEEEdouble)));
+        DoubleAPFloat(PPCDoubleDouble, std::move(*this), APFloat(IEEEdouble)),
+        ToSemantics);
     return Ret;
   } else if (usesLayout<DoubleAPFloat>(getSemantics()) &&
              usesLayout<IEEEFloat>(ToSemantics)) {
     auto Ret = getIEEE().convert(ToSemantics, RM, losesInfo);
-    *this = APFloat(std::move(getIEEE()));
+    *this = APFloat(std::move(getIEEE()), ToSemantics);
     return Ret;
   } else {
     llvm_unreachable("Unexpected semantics");
   }
 }
 
+APFloat APFloat::getAllOnesValue(unsigned BitWidth, bool isIEEE) {
+  if (isIEEE) {
+    switch (BitWidth) {
+    case 16:
+      return APFloat(IEEEhalf, APInt::getAllOnesValue(BitWidth));
+    case 32:
+      return APFloat(IEEEsingle, APInt::getAllOnesValue(BitWidth));
+    case 64:
+      return APFloat(IEEEdouble, APInt::getAllOnesValue(BitWidth));
+    case 80:
+      return APFloat(x87DoubleExtended, APInt::getAllOnesValue(BitWidth));
+    case 128:
+      return APFloat(IEEEquad, APInt::getAllOnesValue(BitWidth));
+    default:
+      llvm_unreachable("Unknown floating bit width");
+    }
+  } else {
+    assert(BitWidth == 128);
+    return APFloat(PPCDoubleDouble, APInt::getAllOnesValue(BitWidth));
+  }
+}
+
 } // End llvm namespace

llvm/trunk/unittests/ADT/APFloatTest.cpp

TEST(APFloatTest, PPCDoubleDouble) {
   EXPECT_EQ(0x3ff0000000000000ull, test.bitcastToAPInt().getRawData()[0]);
 #if 0 // XFAIL
   // This is what we would expect with a true double-double implementation
   EXPECT_EQ(0x3950000000000000ull, test.bitcastToAPInt().getRawData()[1]);
 #else
   // This is what we get with our 106-bit mantissa approximation
   EXPECT_EQ(0x0000000000000000ull, test.bitcastToAPInt().getRawData()[1]);
 #endif
+
+  // PR30869
+  {
+    auto Result = APFloat(APFloat::PPCDoubleDouble, "1.0") +
+                  APFloat(APFloat::PPCDoubleDouble, "1.0");
+    EXPECT_EQ(&APFloat::PPCDoubleDouble, &Result.getSemantics());
+
+    Result = APFloat(APFloat::PPCDoubleDouble, "1.0") -
+             APFloat(APFloat::PPCDoubleDouble, "1.0");
+    EXPECT_EQ(&APFloat::PPCDoubleDouble, &Result.getSemantics());
+
+    Result = APFloat(APFloat::PPCDoubleDouble, "1.0") *
+             APFloat(APFloat::PPCDoubleDouble, "1.0");
+    EXPECT_EQ(&APFloat::PPCDoubleDouble, &Result.getSemantics());
+
+    Result = APFloat(APFloat::PPCDoubleDouble, "1.0") /
+             APFloat(APFloat::PPCDoubleDouble, "1.0");
+    EXPECT_EQ(&APFloat::PPCDoubleDouble, &Result.getSemantics());
+
+    int Exp;
+    Result = frexp(APFloat(APFloat::PPCDoubleDouble, "1.0"), Exp,
+                   APFloat::rmNearestTiesToEven);
+    EXPECT_EQ(&APFloat::PPCDoubleDouble, &Result.getSemantics());
+
+    Result = scalbn(APFloat(APFloat::PPCDoubleDouble, "1.0"), 1,
+                    APFloat::rmNearestTiesToEven);
+    EXPECT_EQ(&APFloat::PPCDoubleDouble, &Result.getSemantics());
+  }
 }
 
 TEST(APFloatTest, isNegative) {
   APFloat t(APFloat::IEEEsingle, "0x1p+0");
   EXPECT_FALSE(t.isNegative());
   t = APFloat(APFloat::IEEEsingle, "-0x1p+0");
   EXPECT_TRUE(t.isNegative());