[flang-commits] [PATCH] D127028: [flang][runtime] Fix edge case discrepancies with EN output editing

[Peter Klausler via Phabricator via flang-commits]

klausler created this revision.
klausler added a reviewer: vdonaldson.
klausler added a project: Flang.
Herald added a subscriber: jdoerfert.
Herald added a project: All.
klausler requested review of this revision.

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.


https://reviews.llvm.org/D127028

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


Index: flang/unittests/Runtime/NumericalFormatTest.cpp
===================================================================
--- flang/unittests/Runtime/NumericalFormatTest.cpp
+++ flang/unittests/Runtime/NumericalFormatTest.cpp
@@ -657,6 +657,23 @@
     ASSERT_TRUE(CompareFormatReal(format, value, expect))
         << "Failed to format " << format << ", expected " << expect;
   }
+
+  // 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) {
+    ASSERT_TRUE(CompareFormatReal("(EN10.1)", value, expect))
+        << "Failed to format EN10.1, expected " << expect;
+  }
 }
 
 //------------------------------------------------------------------------------
Index: flang/runtime/edit-output.cpp
===================================================================
--- flang/runtime/edit-output.cpp
+++ flang/runtime/edit-output.cpp
@@ -265,9 +265,15 @@
   }
   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,
@@ -286,7 +292,7 @@
     ++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)};
@@ -297,12 +303,29 @@
     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};


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D127028.434187.patch
Type: text/x-patch
Size: 3779 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20220603/a499c851/attachment-0001.bin>