[llvm] [ADT] Fix a bug in DoubleAPFloat::frexp (PR #161625)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Kazu Hirata (kazutakahirata)

<details>
<summary>Changes</summary>

Without this patch, we call APFloat::makeQuiet() in frexp like so:

  Quiet.getFirst().makeQuiet();

The problem is that makeQuiet returns a new value instead of modifying
"*this" in place, so we end up discarding the newly returned value.

This patch fixes the problem by assigning the result back to
Quiet.getFirst().

We should put [[nodiscard]] on APFloat::makeQuiet, but I'll do that in
another patch.


---
Full diff: https://github.com/llvm/llvm-project/pull/161625.diff


2 Files Affected:

- (modified) llvm/lib/Support/APFloat.cpp (+1-1) 
- (modified) llvm/unittests/ADT/APFloatTest.cpp (+7) 


``````````diff
diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index d14abb4bd05b5..8623c06597f5c 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -5857,7 +5857,7 @@ DoubleAPFloat frexp(const DoubleAPFloat &Arg, int &Exp,
   // practice.
   if (Exp == APFloat::IEK_NaN) {
     DoubleAPFloat Quiet{Arg};
-    Quiet.getFirst().makeQuiet();
+    Quiet.getFirst() = Quiet.getFirst().makeQuiet();
     return Quiet;
   }
 
diff --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp
index 141282ea254b4..30f0a8e5089ef 100644
--- a/llvm/unittests/ADT/APFloatTest.cpp
+++ b/llvm/unittests/ADT/APFloatTest.cpp
@@ -10176,4 +10176,11 @@ TEST(APFloatTest, hasSignBitInMSB) {
   EXPECT_FALSE(APFloat::hasSignBitInMSB(APFloat::Float8E8M0FNU()));
 }
 
+TEST(APFloatTest, FrexpQuietSNaN) {
+  APFloat SNaN = APFloat::getSNaN(APFloat::PPCDoubleDouble());
+  int Exp;
+  APFloat Result = frexp(SNaN, Exp, APFloat::rmNearestTiesToEven);
+  EXPECT_FALSE(Result.isSignaling());
+}
+
 } // namespace

``````````

</details>


https://github.com/llvm/llvm-project/pull/161625