[llvm] [APFloat] Correct upper boundary distances for directed decimal rounding (PR #230774)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: khan (aikhanjum)

<details>
<summary>Changes</summary>

Partially addresses #<!-- -->228928.

An exactly representable decimal can convert to the adjacent floating point value under directed rounding. For example, parsing `0.100000001490116119384765625` as IEEEsingle with rmTowardZero produces `0x3DCCCCCC` instead of the exact `0x3DCCCCCD` on upstream commit `88e3d6b0a846996d5efab5ce16fa4ed5152b30f8`.

Under directed rounding, the boundary has all truncated bits zero. An approximation can sit next to that boundary from below, with an all-ones tail, as well as from above.

For a single-word remainder narrower than 64 bits, the old path computed `min(part - 0, 0 - part)` at the full word width. An all-ones tail therefore appeared far from the boundary. For a multiword remainder whose high word has fewer than 64 relevant bits, the old comparison `part == boundary - 1` compared the masked high word against `2^64 - 1`, so the upper-boundary branch could never match.

Using `std::min(part, -part & mask)` for the single-word distance and masking `boundary - 1` for the multiword comparison closes both holes. The existing precision retry then handles ambiguous approximations instead of accepting an incorrectly rounded value.

The regression covers exact decimal inputs for normal single and double precision values and a subnormal half precision value. It checks both signs and all three directed rounding modes, including the exact result bits and opOK status.

To reproduce, apply only the regression test to the upstream commit above, configure an assertion-enabled Release build, build ADTTests, and run the following command. The regression fails with unchanged production code and passes with this patch.

```sh
build/unittests/ADT/ADTTests --gtest_filter=APFloatTest.ExactDecimalStringsDirectedRounding
```

Validation on macOS arm64 with AppleClang 17 and assertions enabled.

- All 2253 ADT tests passed. Three existing tests are disabled upstream.
- An additional local check exercised 214458 exact decimal conversions, covering every positive finite half precision value and 4000 sampled single and double precision values, both signs and all three directed modes. No failures.
- An additional local check exercised 952290 inexact decimal conversions at quarter, half, and three-quarter points between adjacent finite half precision values, both signs and all five rounding modes. No failures.
- An independent check compared the compiled boundary helper against arbitrary precision distances in 66886 cases across bit widths 1 through 512. No unsafe or small-distance mismatches, with undefined behavior sanitizer enabled.
- Independent exact conversion checks passed 195840 BFloat, 12066 single, and 12066 double cases.
- Changed lines pass clang-format and git diff --check.

The full LLVM regression suite and other host architectures have not been tested. This patch does not address the separate inexact status problem under nearest rounding reported in the same issue. An additional quad precision check hit the existing powerOfFivePartsForExponent assertion on the exact decimal spelling of the minimum positive subnormal, 2^-16494. The same input aborts on unchanged upstream before reaching the modified helper. That separate limitation remains unresolved, and the quad batch did not pass. The additional conversion checks are local evidence, not added test infrastructure.

OpenAI Codex assisted with investigation, implementation, regression tests, additional validation, and this description. GPT-6 Astra was used for an independent technical review. The mechanical explanation above is based on my review of the patch and was copy-edited with AI assistance.


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


2 Files Affected:

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


``````````diff
diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 3d1932454761f..27212ea5886a5 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -741,9 +741,9 @@ ulpsFromBoundary(const APFloatBase::integerPart *parts, unsigned int bits,
   unsigned count = bits / APFloatBase::integerPartWidth;
   unsigned partBits = bits % APFloatBase::integerPartWidth + 1;
 
-  APFloatBase::integerPart part =
-      parts[count] & (~(APFloatBase::integerPart)0 >>
-                      (APFloatBase::integerPartWidth - partBits));
+  APFloatBase::integerPart mask = ~(APFloatBase::integerPart)0 >>
+                                  (APFloatBase::integerPartWidth - partBits);
+  APFloatBase::integerPart part = parts[count] & mask;
 
   APFloatBase::integerPart boundary;
   if (isNearest)
@@ -752,6 +752,9 @@ ulpsFromBoundary(const APFloatBase::integerPart *parts, unsigned int bits,
     boundary = 0;
 
   if (count == 0) {
+    // Directed rounding has boundaries at both ends of the truncated bits.
+    if (!isNearest)
+      return std::min(part, -part & mask);
     if (part - boundary <= boundary - part)
       return part - boundary;
     else
@@ -764,7 +767,7 @@ ulpsFromBoundary(const APFloatBase::integerPart *parts, unsigned int bits,
         return ~(APFloatBase::integerPart) 0; /* A lot.  */
 
     return parts[0];
-  } else if (part == boundary - 1) {
+  } else if (part == ((boundary - 1) & mask)) {
     while (--count)
       if (~parts[count])
         return ~(APFloatBase::integerPart) 0; /* A lot.  */
diff --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp
index 40a121232fd29..8d94201ecf1c3 100644
--- a/llvm/unittests/ADT/APFloatTest.cpp
+++ b/llvm/unittests/ADT/APFloatTest.cpp
@@ -1393,6 +1393,42 @@ TEST(APFloatTest, fromZeroDecimalLargeExponentString) {
   EXPECT_EQ(0.0,  APFloat(APFloat::IEEEdouble(), StringRef("0e1234" "\0" "2", 6)).convertToDouble());
 }
 
+TEST(APFloatTest, ExactDecimalStringsDirectedRounding) {
+  struct TestCase {
+    const fltSemantics &Sem;
+    const char *Decimal;
+    uint64_t Bits;
+  };
+  const TestCase Tests[] = {
+      {APFloat::IEEEsingle(), "0.100000001490116119384765625", 0x3DCCCCCD},
+      {APFloat::IEEEhalf(), "0.000017225742340087890625", 0x0121},
+      {APFloat::IEEEdouble(),
+       "0.1000000000000000055511151231257827021181583404541015625",
+       0x3FB999999999999A},
+  };
+  // These decimals are exact binary values. An approximation just below the
+  // upper truncation boundary must not be mistaken for a safe rounding result.
+  for (const TestCase &Test : Tests) {
+    for (bool Negative : {false, true}) {
+      APFloat Expected(
+          Test.Sem, APInt(APFloat::semanticsSizeInBits(Test.Sem), Test.Bits));
+      if (Negative)
+        Expected.changeSign();
+      std::string Input = std::string(Negative ? "-" : "") + Test.Decimal;
+      for (auto RM : {APFloat::rmTowardZero, APFloat::rmTowardPositive,
+                      APFloat::rmTowardNegative}) {
+        SCOPED_TRACE(Input);
+        SCOPED_TRACE(static_cast<unsigned>(RM));
+        APFloat Value(Test.Sem);
+        auto Status = Value.convertFromString(Input, RM);
+        ASSERT_TRUE(!!Status);
+        EXPECT_EQ(*Status, APFloat::opOK);
+        EXPECT_TRUE(Value.bitwiseIsEqual(Expected));
+      }
+    }
+  }
+}
+
 TEST(APFloatTest, DecimalStringsUseLargePowersOfFive) {
   // These values are just below one, so they cannot take the early obvious
   // underflow path. The number of fractional digits is the power passed to

``````````

</details>


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