Skip to content

Commit

Permalink
[flang][runtime] Handle Fw.0 case that needs to round up to 1.0 (#74384)
Browse files Browse the repository at this point in the history 
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 #74274.
  • Loading branch information
@klausler
klausler committed Dec 11, 2023
1 parent 9458bae commit ced631e
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 37 deletions.
80 changes: 49 additions & 31 deletions flang/runtime/edit-output.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down
29 changes: 23 additions & 6 deletions flang/unittests/Runtime/NumericalFormatTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,26 +710,43 @@ 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, "*****;"},
{"(F5.3,';')", -0.99999, "*****;"},
{"(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
Expand Down

0 comments on commit ced631e

Please sign in to comment.