Skip to content

Conversation

@klausler
Copy link
Contributor

Excess hexadecimal digits were too significant for rounding purposes, leading to inappropriate rounding away from zero for some modes.

@klausler klausler requested a review from vdonaldson March 17, 2024 23:27
@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category labels Mar 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 17, 2024

@llvm/pr-subscribers-flang-runtime

Author: Peter Klausler (klausler)

Changes

Excess hexadecimal digits were too significant for rounding purposes, leading to inappropriate rounding away from zero for some modes.


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

2 Files Affected:

  • (modified) flang/runtime/edit-input.cpp (+41-29)
  • (modified) flang/unittests/Runtime/NumericalFormatTest.cpp (+8)
diff --git a/flang/runtime/edit-input.cpp b/flang/runtime/edit-input.cpp
index 85cce2f1a16623..b0636c9491fdd2 100644
--- a/flang/runtime/edit-input.cpp
+++ b/flang/runtime/edit-input.cpp
@@ -612,11 +612,22 @@ decimal::ConversionToBinaryResult<binaryPrecision> ConvertHexadecimal(
     } else {
       break;
     }
-    while (fraction >> binaryPrecision) {
-      guardBit |= roundingBit;
-      roundingBit = (int)fraction & 1;
-      fraction >>= 1;
-      ++expo;
+    if (fraction >> binaryPrecision) {
+      while (fraction >> binaryPrecision) {
+        guardBit |= roundingBit;
+        roundingBit = (int)fraction & 1;
+        fraction >>= 1;
+        ++expo;
+      }
+      // Consume excess digits
+      for (; *p; ++p) {
+        if ((*p >= '1' && *p <= '9') || (*p >= 'A' && *p <= 'F')) {
+          guardBit = 1;
+        } else if (*p != '0') {
+          break;
+        }
+      }
+      break;
     }
   }
   if (fraction) {
@@ -631,31 +642,32 @@ decimal::ConversionToBinaryResult<binaryPrecision> ConvertHexadecimal(
     while (expo > 1 && !(fraction >> (binaryPrecision - 1))) {
       fraction <<= 1;
       --expo;
+      guardBit = roundingBit = 0;
     }
-    // Rounding
-    bool increase{false};
-    switch (rounding) {
-    case decimal::RoundNearest: // RN & RP
-      increase = roundingBit && (guardBit | ((int)fraction & 1));
-      break;
-    case decimal::RoundUp: // RU
-      increase = !isNegative && (roundingBit | guardBit);
-      break;
-    case decimal::RoundDown: // RD
-      increase = isNegative && (roundingBit | guardBit);
-      break;
-    case decimal::RoundToZero: // RZ
-      break;
-    case decimal::RoundCompatible: // RC
-      increase = roundingBit != 0;
-      break;
-    }
-    if (increase) {
-      ++fraction;
-      if (fraction >> binaryPrecision) {
-        fraction >>= 1;
-        ++expo;
-      }
+  }
+  // Rounding
+  bool increase{false};
+  switch (rounding) {
+  case decimal::RoundNearest: // RN & RP
+    increase = roundingBit && (guardBit | ((int)fraction & 1));
+    break;
+  case decimal::RoundUp: // RU
+    increase = !isNegative && (roundingBit | guardBit);
+    break;
+  case decimal::RoundDown: // RD
+    increase = isNegative && (roundingBit | guardBit);
+    break;
+  case decimal::RoundToZero: // RZ
+    break;
+  case decimal::RoundCompatible: // RC
+    increase = roundingBit != 0;
+    break;
+  }
+  if (increase) {
+    ++fraction;
+    if (fraction >> binaryPrecision) {
+      fraction >>= 1;
+      ++expo;
     }
   }
   // Package & return result
diff --git a/flang/unittests/Runtime/NumericalFormatTest.cpp b/flang/unittests/Runtime/NumericalFormatTest.cpp
index 37eecd7708a1eb..dee4dda4a22869 100644
--- a/flang/unittests/Runtime/NumericalFormatTest.cpp
+++ b/flang/unittests/Runtime/NumericalFormatTest.cpp
@@ -902,6 +902,14 @@ TEST(IOApiTests, EditDoubleInputValues) {
           0}, // max finite
       {"(EX22.0)", "0X.8P1025             ", 0x7ff0000000000000, ovf}, // +Inf
       {"(EX22.0)", "-0X.8P1025            ", 0xfff0000000000000, ovf}, // -Inf
+      {"(RC,EX22.0)", "0X1.0000000000000P0   ", 0x3ff0000000000000, 0},
+      {"(RC,EX22.0)", "0X1.00000000000008P0  ", 0x3ff0000000000001, 0},
+      {"(RC,EX22.0)", "0X1.000000000000008P0 ", 0x3ff0000000000000, 0},
+      {"(RC,EX22.0)", "0X1.00000000000004P0  ", 0x3ff0000000000000, 0},
+      {"(RC,EX22.0)", "0X.80000000000000P1   ", 0x3ff0000000000000, 0},
+      {"(RC,EX22.0)", "0X.80000000000004P1   ", 0x3ff0000000000001, 0},
+      {"(RC,EX22.0)", "0X.800000000000004P1  ", 0x3ff0000000000000, 0},
+      {"(RC,EX22.0)", "0X.80000000000002P1   ", 0x3ff0000000000000, 0},
       {"(RZ,F7.0)", " 2.e308", 0x7fefffffffffffff, 0}, // +HUGE()
       {"(RD,F7.0)", " 2.e308", 0x7fefffffffffffff, 0}, // +HUGE()
       {"(RU,F7.0)", " 2.e308", 0x7ff0000000000000, ovf}, // +Inf

Excess hexadecimal digits were too significant for rounding purposes,
leading to inappropriate rounding away from zero for some modes.
@klausler klausler merged commit 7eb5d4f into llvm:main Mar 18, 2024
@klausler klausler deleted the bug1458 branch March 18, 2024 21:13
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
…lvm#85587)

Excess hexadecimal digits were too significant for rounding purposes,
leading to inappropriate rounding away from zero for some modes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:runtime flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants