[flang-commits] [flang] [flang][runtime] Handle Fw.0 case that needs to round up to 1.0 (PR #74384)
Peter Klausler via flang-commits
flang-commits at lists.llvm.org
Mon Dec 4 15:32:56 PST 2023
https://github.com/klausler created https://github.com/llvm/llvm-project/pull/74384
A tricky case in Fw.d output editing is when the value needs to be rounded either to a signed zero or away from zero to a power of ten (1.0, 0.1, &c.). A bug report for LLVM on GitHub (#74274) exposed a bug in this code in the case of Fw.0 editing where a value just over 0.5 was rounded incorrectly to 0 rather than 1 when no fractional digits were requested.
Rework that algorithm a little, ensuring that the initial binary->decimal conversion produces at least one digit, and coping correctly with the rounding of an exact 0.5 value for Fw.0 editing (rounding it to the nearest even decimal, namely 0, following near-universal compiler-dependent behavior in other Fortrans).
Fixes https://github.com/llvm/llvm-project/issues/74274.
>From 15f7c084047bbd9981c83a4be7b85ac4553d7dd5 Mon Sep 17 00:00:00 2001
From: Peter Klausler <pklausler at nvidia.com>
Date: Mon, 4 Dec 2023 15:21:58 -0800
Subject: [PATCH] [flang][runtime] Handle Fw.0 case that needs to round up to
1.0
A tricky case in Fw.d output editing is when the value needs to
be rounded either to a signed zero or away from zero to a power
of ten (1.0, 0.1, &c.). A bug report for LLVM on GitHub (#74274)
exposed a bug in this code in the case of Fw.0 editing where
a value just over 0.5 was rounded incorrectly to 0 rather than 1
when no fractional digits were requested.
Rework that algorithm a little, ensuring that the initial
binary->decimal conversion produces at least one digit, and
coping correctly with the rounding of an exact 0.5 value
for Fw.0 editing (rounding it to the nearest even decimal,
namely 0, following near-universal compiler-dependent behavior
in other Fortrans).
Fixes https://github.com/llvm/llvm-project/issues/74274.
---
flang/runtime/edit-output.cpp | 80 ++++++++++++-------
.../unittests/Runtime/NumericalFormatTest.cpp | 29 +++++--
2 files changed, 72 insertions(+), 37 deletions(-)
diff --git a/flang/runtime/edit-output.cpp b/flang/runtime/edit-output.cpp
index 18b209bc6798c..e49a78ca02e15 100644
--- a/flang/runtime/edit-output.cpp
+++ b/flang/runtime/edit-output.cpp
@@ -433,27 +433,28 @@ bool RealOutputEditing<KIND>::EditFOutput(const DataEdit &edit) {
}
// Multiple conversions may be needed to get the right number of
// effective rounded fractional digits.
- int extraDigits{0};
bool canIncrease{true};
- while (true) {
+ for (int extraDigits{fracDigits == 0 ? 1 : 0};;) {
decimal::ConversionToDecimalResult converted{
ConvertToDecimal(extraDigits + fracDigits, rounding, flags)};
- if (IsInfOrNaN(converted.str, static_cast<int>(converted.length))) {
+ const char *convertedStr{converted.str};
+ if (IsInfOrNaN(convertedStr, static_cast<int>(converted.length))) {
return editWidth > 0 &&
converted.length > static_cast<std::size_t>(editWidth)
? EmitRepeated(io_, '*', editWidth)
: EmitPrefix(edit, converted.length, editWidth) &&
- EmitAscii(io_, converted.str, converted.length) &&
+ EmitAscii(io_, convertedStr, converted.length) &&
EmitSuffix(edit);
}
int expo{converted.decimalExponent + edit.modes.scale /*kP*/};
- int signLength{*converted.str == '-' || *converted.str == '+' ? 1 : 0};
+ int signLength{*convertedStr == '-' || *convertedStr == '+' ? 1 : 0};
int convertedDigits{static_cast<int>(converted.length) - signLength};
if (IsZero()) { // don't treat converted "0" as significant digit
expo = 0;
convertedDigits = 0;
}
- int trailingOnes{0};
+ bool isNegative{*convertedStr == '-'};
+ char one[2];
if (expo > extraDigits && extraDigits >= 0 && canIncrease) {
extraDigits = expo;
if (!edit.digits.has_value()) { // F0
@@ -462,24 +463,45 @@ bool RealOutputEditing<KIND>::EditFOutput(const DataEdit &edit) {
canIncrease = false; // only once
continue;
} else if (expo == -fracDigits && convertedDigits > 0) {
- if ((rounding == decimal::FortranRounding::RoundUp &&
- *converted.str != '-') ||
- (rounding == decimal::FortranRounding::RoundDown &&
- *converted.str == '-') ||
- (rounding == decimal::FortranRounding::RoundToZero &&
- rounding != edit.modes.round && // it changed below
- converted.str[signLength] >= '5')) {
- // Round up/down to a scaled 1
+ // Result will be either a signed zero or power of ten, depending
+ // on rounding.
+ char leading{convertedStr[signLength]};
+ bool roundToPowerOfTen{false};
+ switch (edit.modes.round) {
+ case decimal::FortranRounding::RoundUp:
+ roundToPowerOfTen = !isNegative;
+ break;
+ case decimal::FortranRounding::RoundDown:
+ roundToPowerOfTen = isNegative;
+ break;
+ case decimal::FortranRounding::RoundToZero:
+ break;
+ case decimal::FortranRounding::RoundNearest:
+ if (leading == '5' &&
+ rounding == decimal::FortranRounding::RoundNearest) {
+ // Try again, rounding away from zero.
+ rounding = isNegative ? decimal::FortranRounding::RoundDown
+ : decimal::FortranRounding::RoundUp;
+ extraDigits = 1 - fracDigits; // just one digit needed
+ continue;
+ }
+ roundToPowerOfTen = leading > '5';
+ break;
+ case decimal::FortranRounding::RoundCompatible:
+ roundToPowerOfTen = leading >= '5';
+ break;
+ }
+ if (roundToPowerOfTen) {
++expo;
- convertedDigits = 0;
- trailingOnes = 1;
- } else if (rounding != decimal::FortranRounding::RoundToZero) {
- // Convert again with truncation so first digit can be checked
- // on the next iteration by the code above
- rounding = decimal::FortranRounding::RoundToZero;
- continue;
+ convertedDigits = 1;
+ if (signLength > 0) {
+ one[0] = *convertedStr;
+ one[1] = '1';
+ } else {
+ one[0] = '1';
+ }
+ convertedStr = one;
} else {
- // Value rounds down to zero
expo = 0;
convertedDigits = 0;
}
@@ -493,17 +515,14 @@ bool RealOutputEditing<KIND>::EditFOutput(const DataEdit &edit) {
int digitsAfterPoint{convertedDigits - digitsBeforePoint};
int trailingZeroes{flags & decimal::Minimize
? 0
- : std::max(0,
- fracDigits -
- (zeroesAfterPoint + digitsAfterPoint + trailingOnes))};
+ : std::max(0, fracDigits - (zeroesAfterPoint + digitsAfterPoint))};
if (digitsBeforePoint + zeroesBeforePoint + zeroesAfterPoint +
- digitsAfterPoint + trailingOnes + trailingZeroes ==
+ digitsAfterPoint + trailingZeroes ==
0) {
zeroesBeforePoint = 1; // "." -> "0."
}
int totalLength{signLength + digitsBeforePoint + zeroesBeforePoint +
- 1 /*'.'*/ + zeroesAfterPoint + digitsAfterPoint + trailingOnes +
- trailingZeroes};
+ 1 /*'.'*/ + zeroesAfterPoint + digitsAfterPoint + trailingZeroes};
int width{editWidth > 0 ? editWidth : totalLength};
if (totalLength > width) {
return EmitRepeated(io_, '*', width);
@@ -513,13 +532,12 @@ bool RealOutputEditing<KIND>::EditFOutput(const DataEdit &edit) {
++totalLength;
}
return EmitPrefix(edit, totalLength, width) &&
- EmitAscii(io_, converted.str, signLength + digitsBeforePoint) &&
+ EmitAscii(io_, convertedStr, signLength + digitsBeforePoint) &&
EmitRepeated(io_, '0', zeroesBeforePoint) &&
EmitAscii(io_, edit.modes.editingFlags & decimalComma ? "," : ".", 1) &&
EmitRepeated(io_, '0', zeroesAfterPoint) &&
- EmitAscii(io_, converted.str + signLength + digitsBeforePoint,
+ EmitAscii(io_, convertedStr + signLength + digitsBeforePoint,
digitsAfterPoint) &&
- EmitRepeated(io_, '1', trailingOnes) &&
EmitRepeated(io_, '0', trailingZeroes) &&
EmitRepeated(io_, ' ', trailingBlanks_) && EmitSuffix(edit);
}
diff --git a/flang/unittests/Runtime/NumericalFormatTest.cpp b/flang/unittests/Runtime/NumericalFormatTest.cpp
index 219947fe4fbbb..b5b8eb0594373 100644
--- a/flang/unittests/Runtime/NumericalFormatTest.cpp
+++ b/flang/unittests/Runtime/NumericalFormatTest.cpp
@@ -710,8 +710,12 @@ TEST(IOApiTests, FormatDoubleValues) {
{"(F5.3,';')", 0.099999, "0.100;"},
{"(F5.3,';')", 0.0099999, "0.010;"},
{"(F5.3,';')", 0.00099999, "0.001;"},
- {"(F5.3,';')", 0.0005, "0.001;"},
- {"(F5.3,';')", 0.00049999, "0.000;"},
+ {"(F5.3,';')",
+ 0.0005000000000000000104083408558608425664715468883514404296875,
+ "0.001;"},
+ {"(F5.3,';')",
+ 0.000499999999999999901988123607310399165726266801357269287109375,
+ "0.000;"},
{"(F5.3,';')", 0.000099999, "0.000;"},
{"(F5.3,';')", -99.999, "*****;"},
{"(F5.3,';')", -9.9999, "*****;"},
@@ -719,17 +723,30 @@ TEST(IOApiTests, FormatDoubleValues) {
{"(F5.3,';')", -0.099999, "-.100;"},
{"(F5.3,';')", -0.0099999, "-.010;"},
{"(F5.3,';')", -0.00099999, "-.001;"},
- {"(F5.3,';')", -0.0005, "-.001;"},
- {"(F5.3,';')", -0.00049999, "-.000;"},
+ {"(F5.3,';')",
+ -0.0005000000000000000104083408558608425664715468883514404296875,
+ "-.001;"},
+ {"(F5.3,';')",
+ -0.000499999999999999901988123607310399165726266801357269287109375,
+ "-.000;"},
{"(F5.3,';')", -0.000099999, "-.000;"},
{"(F0.1,';')", 0.0, ".0;"},
+ {"(F5.0,';')", -0.5000000000000001, " -1.;"},
+ {"(F5.0,';')", -0.5, " -0.;"},
+ {"(F5.0,';')", -0.49999999999999994, " -0.;"},
+ {"(F5.0,';')", 0.49999999999999994, " 0.;"},
+ {"(F5.0,';')", 0.5, " 0.;"},
+ {"(F5.0,';')", 0.5000000000000001, " 1.;"},
};
for (auto const &[format, value, expect] : individualTestCases) {
std::string got;
+ char hex[17];
+ std::snprintf(hex, sizeof hex, "%016llx",
+ *reinterpret_cast<const unsigned long long *>(&value));
ASSERT_TRUE(CompareFormatReal(format, value, expect, got))
- << "Failed to format " << format << ", expected '" << expect
- << "', got '" << got << "'";
+ << "Failed to format " << value << " 0x" << hex << " with format "
+ << format << ", expected '" << expect << "', got '" << got << "'";
}
// Problematic EN formatting edge cases with rounding
More information about the flang-commits
mailing list