Make IEEEFloat::roundToIntegral more standard conformant

[sepavloff]

Summary

Behavior of IEEEFloat::roundToIntegral is aligned with IEEE-754
operation roundToIntegralExact. In partucular this function now:

• produces opInvalid for signaling NaNs,
• returns opInexact if the result of rounding differs from argument.

[sepavloff]

Ping.

[scanon]

llvm/lib/Support/APFloat.cpp

2011

What is this check intended to do? AFAICT from a quick glance, this will never happen.

2020

fs will never be anything other than opOK here, right? By Sterbenz' lemma this subtraction is always exact.

[sepavloff]

Updated patch according to reviewer's feedback.

llvm/lib/Support/APFloat.cpp

2011

For signaling NaN fs may be opInvalid.
Reorganized code so that the respective check is made explicitly.

2020

Sterbenz' lemma! Exactly! Thank you very much!

[scanon]

llvm/lib/Support/APFloat.cpp

1984

Correct as written, but can we get some brackets since we have the comment here?

llvm/lib/Support/APFloat.cpp

/* FS can only be opOK or opInvalidOp.  There is no more work
        precision.  */
     if (fs == opOK)
       fs = addOrSubtract(addend, rounding_mode, false);
   }
 
   return fs;
 }
 
-/* Rounding-mode corrrect round to integral value.  */
+/* Rounding-mode correct round to integral value.  */
 IEEEFloat::opStatus IEEEFloat::roundToIntegral(roundingMode rounding_mode) {
   opStatus fs;
 
   // If the exponent is large enough, we know that this value is already
   // integral, and the arithmetic below would potentially cause it to saturate
   // to +/-Inf.  Bail out early instead.
   if (isFiniteNonZero() && exponent+1 >= (int)semanticsPrecision(*semantics))
     return opOK;
 
   // The algorithm here is quite simple: we add 2^(p-1), where p is the
   // precision of our format, and then subtract it back off again.  The choice
   // of rounding modes for the addition/subtraction determines the rounding mode
IEEEFloat::opStatus IEEEFloat::roundToIntegral(roundingMode rounding_mode) {
 
   if (fs != opOK)
     return fs;
 
   // Preserve the input sign so that we can handle 0.0/-0.0 cases correctly.
   bool inputSign = isNegative();
 
   fs = add(MagicConstant, rounding_mode);
-  if (fs != opOK && fs != opInexact)
+  opStatus IsInexact = static_cast<opStatus>(fs & opInexact);
+  if (fs != opOK && !IsInexact)
     return fs;
 
   fs = subtract(MagicConstant, rounding_mode);
 
   // Restore the input sign.
   if (inputSign != isNegative())
     changeSign();
 
-  return fs;
+  return fs == opOK ? IsInexact : fs;
 }
 
 
 /* Comparison requires normalized numbers.  */
 IEEEFloat::cmpResult IEEEFloat::compare(const IEEEFloat &rhs) const {
   cmpResult result;
 
   assert(semantics == rhs.semantics);

llvm/unittests/ADT/APFloatTest.cpp

TEST(APFloatTest, roundToIntegral) {
   P.roundToIntegral(APFloat::rmTowardZero);
   EXPECT_TRUE(std::isnan(P.convertToDouble()));
   P = APFloat::getInf(APFloat::IEEEdouble());
   P.roundToIntegral(APFloat::rmTowardZero);
   EXPECT_TRUE(std::isinf(P.convertToDouble()) && P.convertToDouble() > 0.0);
   P = APFloat::getInf(APFloat::IEEEdouble(), true);
   P.roundToIntegral(APFloat::rmTowardZero);
   EXPECT_TRUE(std::isinf(P.convertToDouble()) && P.convertToDouble() < 0.0);
+
+  APFloat::opStatus St;
+
+  P = APFloat(10.0);
+  St = P.roundToIntegral(APFloat::rmTowardZero);
+  EXPECT_EQ(10.0, P.convertToDouble());
+  EXPECT_EQ(APFloat::opOK, St);
+
+  P = APFloat::getNaN(APFloat::IEEEdouble());
+  St = P.roundToIntegral(APFloat::rmTowardZero);
+  EXPECT_TRUE(P.isNaN());
+  EXPECT_FALSE(P.isNegative());
+  EXPECT_EQ(APFloat::opOK, St);
+
+  P = APFloat::getNaN(APFloat::IEEEdouble(), true);
+  St = P.roundToIntegral(APFloat::rmTowardZero);
+  EXPECT_TRUE(P.isNaN());
+  EXPECT_TRUE(P.isNegative());
+  EXPECT_EQ(APFloat::opOK, St);
+
+  P = APFloat::getInf(APFloat::IEEEdouble());
+  St = P.roundToIntegral(APFloat::rmTowardZero);
+  EXPECT_TRUE(P.isInfinity());
+  EXPECT_FALSE(P.isNegative());
+  EXPECT_EQ(APFloat::opOK, St);
+
+  P = APFloat::getInf(APFloat::IEEEdouble(), true);
+  St = P.roundToIntegral(APFloat::rmTowardZero);
+  EXPECT_TRUE(P.isInfinity());
+  EXPECT_TRUE(P.isNegative());
+  EXPECT_EQ(APFloat::opOK, St);
+
+  P = APFloat(10.5);
+  St = P.roundToIntegral(APFloat::rmTowardZero);
+  EXPECT_EQ(10.0, P.convertToDouble());
+  EXPECT_EQ(APFloat::opInexact, St);
+
+  P = APFloat(10.5);
+  St = P.roundToIntegral(APFloat::rmTowardPositive);
+  EXPECT_EQ(11.0, P.convertToDouble());
+  EXPECT_EQ(APFloat::opInexact, St);
+
+  P = APFloat(10.5);
+  St = P.roundToIntegral(APFloat::rmTowardNegative);
+  EXPECT_EQ(10.0, P.convertToDouble());
+  EXPECT_EQ(APFloat::opInexact, St);
+
+  P = APFloat(10.5);
+  St = P.roundToIntegral(APFloat::rmNearestTiesToAway);
+  EXPECT_EQ(11.0, P.convertToDouble());
+  EXPECT_EQ(APFloat::opInexact, St);
+
+  P = APFloat(10.5);
+  St = P.roundToIntegral(APFloat::rmNearestTiesToEven);
+  EXPECT_EQ(10.0, P.convertToDouble());
+  EXPECT_EQ(APFloat::opInexact, St);
 }
 
 TEST(APFloatTest, isInteger) {
   APFloat T(-0.0);
   EXPECT_TRUE(T.isInteger());
   T = APFloat(3.14159);
   EXPECT_FALSE(T.isInteger());
   T = APFloat::getNaN(APFloat::IEEEdouble());