[libc-commits] [libc] [libc][math] Fix incorrect logic in fputil::generic::add_or_sub (PR #116129)

[via libc-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: OverMighty (overmighty)

<details>
<summary>Changes</summary>

Fixes incorrect logic that went unnoticed until the function was tested
with output and input types having the same underlying floating-point
format. The resulting DyadicFloat's exponent was off by one when adding
two subnormal numbers, and the minimum operand's mantissa was misaligned
by one bit when adding a normal number with a subnormal number.


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


2 Files Affected:

- (modified) libc/src/__support/FPUtil/generic/add_sub.h (+8-7) 
- (modified) libc/test/src/math/smoke/AddTest.h (+12-1) 


``````````diff
diff --git a/libc/src/__support/FPUtil/generic/add_sub.h b/libc/src/__support/FPUtil/generic/add_sub.h
index 6bc9dcd23bafad..5c503e752c8ecb 100644
--- a/libc/src/__support/FPUtil/generic/add_sub.h
+++ b/libc/src/__support/FPUtil/generic/add_sub.h
@@ -160,20 +160,21 @@ add_or_sub(InType x, InType y) {
   } else {
     InStorageType max_mant = max_bits.get_explicit_mantissa() << GUARD_BITS_LEN;
     InStorageType min_mant = min_bits.get_explicit_mantissa() << GUARD_BITS_LEN;
-    int alignment =
-        max_bits.get_biased_exponent() - min_bits.get_biased_exponent();
+
+    int alignment = (max_bits.get_biased_exponent() - max_bits.is_normal()) -
+                    (min_bits.get_biased_exponent() - min_bits.is_normal());
 
     InStorageType aligned_min_mant =
         min_mant >> cpp::min(alignment, RESULT_MANTISSA_LEN);
     bool aligned_min_mant_sticky;
 
-    if (alignment <= 3)
+    if (alignment <= GUARD_BITS_LEN)
       aligned_min_mant_sticky = false;
-    else if (alignment <= InFPBits::FRACTION_LEN + 3)
+    else if (alignment > InFPBits::FRACTION_LEN + GUARD_BITS_LEN)
+      aligned_min_mant_sticky = true;
+    else
       aligned_min_mant_sticky =
           (min_mant << (InFPBits::STORAGE_LEN - alignment)) != 0;
-    else
-      aligned_min_mant_sticky = true;
 
     InStorageType min_mant_sticky(static_cast<int>(aligned_min_mant_sticky));
 
@@ -183,7 +184,7 @@ add_or_sub(InType x, InType y) {
       result_mant = max_mant - (aligned_min_mant | min_mant_sticky);
   }
 
-  int result_exp = max_bits.get_exponent() - RESULT_FRACTION_LEN;
+  int result_exp = max_bits.get_explicit_exponent() - RESULT_FRACTION_LEN;
   DyadicFloat result(result_sign, result_exp, result_mant);
   return result.template as<OutType, /*ShouldSignalExceptions=*/true>();
 }
diff --git a/libc/test/src/math/smoke/AddTest.h b/libc/test/src/math/smoke/AddTest.h
index 66b188f4fa7b31..26efff4efaf948 100644
--- a/libc/test/src/math/smoke/AddTest.h
+++ b/libc/test/src/math/smoke/AddTest.h
@@ -136,6 +136,16 @@ class AddTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
     func(InType(1.0), in.min_denormal);
     EXPECT_FP_EXCEPTION(FE_INEXACT);
   }
+
+  void test_mixed_normality(AddFunc func) {
+    if (LIBC_NAMESPACE::fputil::get_fp_type<OutType>() !=
+        LIBC_NAMESPACE::fputil::get_fp_type<InType>())
+      return;
+
+    EXPECT_FP_EQ(FPBits::create_value(Sign::POS, 2U, 0b1U).get_val(),
+                 func(InFPBits::create_value(Sign::POS, 2U, 0U).get_val(),
+                      InFPBits::create_value(Sign::POS, 2U, 0b10U).get_val()));
+  }
 };
 
 #define LIST_ADD_TESTS(OutType, InType, func)                                  \
@@ -145,6 +155,7 @@ class AddTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
     test_invalid_operations(&func);                                            \
   }                                                                            \
   TEST_F(LlvmLibcAddTest, RangeErrors) { test_range_errors(&func); }           \
-  TEST_F(LlvmLibcAddTest, InexactResults) { test_inexact_results(&func); }
+  TEST_F(LlvmLibcAddTest, InexactResults) { test_inexact_results(&func); }     \
+  TEST_F(LlvmLibcAddTest, MixedNormality) { test_mixed_normality(&func); }
 
 #endif // LLVM_LIBC_TEST_SRC_MATH_SMOKE_ADDTEST_H

``````````

</details>


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