[flang-commits] [flang] dfcccc6 - [flang][runtime] Fix edge case discrepancies with EN output editing

[Peter Klausler via flang-commits]

Author: Peter Klausler
Date: 2022-06-04T09:14:05-07:00
New Revision: dfcccc6dee90e08b9ae0f53c42c9c239fe4031a1

URL: https://github.com/llvm/llvm-project/commit/dfcccc6dee90e08b9ae0f53c42c9c239fe4031a1
DIFF: https://github.com/llvm/llvm-project/commit/dfcccc6dee90e08b9ae0f53c42c9c239fe4031a1.diff

LOG: [flang][runtime] Fix edge case discrepancies with EN output editing

The "engineering" ENw.d output editing descriptor has some difficult
edge case behavior for values that might format into a bunch of 9's
or round up to a 1 for a given scale factor.  Fix the algorithm,
and add tests to protect against regressions.

Differential Revision: https://reviews.llvm.org/D127028

Added: 
    

Modified: 
    flang/runtime/edit-output.cpp
    flang/unittests/Runtime/NumericalFormatTest.cpp

Removed: 
    


################################################################################
diff  --git a/flang/runtime/edit-output.cpp b/flang/runtime/edit-output.cpp
index f76be339cc7b6..b2e9c5a42b363 100644
--- a/flang/runtime/edit-output.cpp
+++ b/flang/runtime/edit-output.cpp
@@ -273,9 +273,15 @@ bool RealOutputEditing<binaryPrecision>::EditEorDOutput(const DataEdit &edit) {
   }
   bool isEN{edit.variation == 'N'};
   bool isES{edit.variation == 'S'};
-  int scale{isEN || isES ? 1 : edit.modes.scale}; // 'kP' value
+  int scale{edit.modes.scale}; // 'kP' value
   int zeroesAfterPoint{0};
-  if (scale < 0) {
+  if (isEN) {
+    scale = IsZero() ? 1 : 3;
+    significantDigits += scale;
+  } else if (isES) {
+    scale = 1;
+    ++significantDigits;
+  } else if (scale < 0) {
     if (scale <= -editDigits) {
       io_.GetIoErrorHandler().SignalError(IostatBadScaleFactor,
           "Scale factor (kP) %d cannot be less than -d (%d)", scale,
@@ -294,7 +300,7 @@ bool RealOutputEditing<binaryPrecision>::EditEorDOutput(const DataEdit &edit) {
     ++significantDigits;
     scale = std::min(scale, significantDigits + 1);
   }
-  // In EN editing, multiple attempts may be necessary, so it's in a loop.
+  // In EN editing, multiple attempts may be necessary, so this is a loop.
   while (true) {
     decimal::ConversionToDecimalResult converted{
         Convert(significantDigits, edit.modes.round, flags)};
@@ -305,12 +311,29 @@ bool RealOutputEditing<binaryPrecision>::EditEorDOutput(const DataEdit &edit) {
     if (!IsZero()) {
       converted.decimalExponent -= scale;
     }
-    if (isEN && scale < 3 && (converted.decimalExponent % 3) != 0) {
-      // EN mode: boost the scale and significant digits, try again; need
-      // an effective exponent field that's a multiple of three.
-      ++scale;
-      ++significantDigits;
-      continue;
+    if (isEN) {
+      // EN mode: we need an effective exponent field that is
+      // a multiple of three.
+      if (int modulus{converted.decimalExponent % 3}; modulus != 0) {
+        if (significantDigits > 1) {
+          --significantDigits;
+          --scale;
+          continue;
+        }
+        // Rounded nines up to a 1.
+        scale += modulus;
+        converted.decimalExponent -= modulus;
+      }
+      if (scale > 3) {
+        int adjust{3 * (scale / 3)};
+        scale -= adjust;
+        converted.decimalExponent += adjust;
+      } else if (scale < 1) {
+        int adjust{3 - 3 * (scale / 3)};
+        scale += adjust;
+        converted.decimalExponent -= adjust;
+      }
+      significantDigits = editDigits + scale;
     }
     // Format the exponent (see table 13.1 for all the cases)
     int expoLength{0};

diff  --git a/flang/unittests/Runtime/NumericalFormatTest.cpp b/flang/unittests/Runtime/NumericalFormatTest.cpp
index 0c8f7a0aad3ba..3a3a1c9862d69 100644
--- a/flang/unittests/Runtime/NumericalFormatTest.cpp
+++ b/flang/unittests/Runtime/NumericalFormatTest.cpp
@@ -672,6 +672,24 @@ TEST(IOApiTests, FormatDoubleValues) {
         << "Failed to format " << format << ", expected " << expect << ", got "
         << got;
   }
+
+  // Problematic EN formatting edge cases with rounding
+  using IndividualENTestCaseTy = std::tuple<std::uint64_t, const char *>;
+  static const std::vector<IndividualENTestCaseTy> individualENTestCases{
+      {0x3E11183197785F8C, " 995.0E-12"}, // 0.9950312500000000115852E-09
+      {0x3E11180E68455D30, " 995.0E-12"}, // 0.9949999999999999761502E-09
+      {0x3E112BD8F4F6B0D7, " 999.5E-12"}, // 0.9994999999999999089118E-09
+      {0x3E45794883CA8782, "  10.0E-09"}, // 0.9999499999999999642266E-08
+      {0x3F506218230C7482, " 999.9E-06"}, // 0.9999499999999998840761E-03
+      {0x3FB99652BD3C3612, " 100.0E-03"}, // 0.9999500000000000055067E+00
+      {0x4023E66666666667, "  10.0E+00"}, // 0.9950000000000001065814E+01
+  };
+
+  for (auto const &[value, expect] : individualENTestCases) {
+    std::string got;
+    ASSERT_TRUE(CompareFormatReal("(EN10.1)", value, expect, got))
+        << "Failed to format EN10.1, expected " << expect << ", got " << got;
+  }
 }
 
 //------------------------------------------------------------------------------