Skip to content

Commit ced631e

Browse files
authored
[flang][runtime] Handle Fw.0 case that needs to round up to 1.0 (#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 #74274.
1 parent 9458bae commit ced631e

File tree

2 files changed

+72
-37
lines changed

2 files changed

+72
-37
lines changed

flang/runtime/edit-output.cpp

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -433,27 +433,28 @@ bool RealOutputEditing<KIND>::EditFOutput(const DataEdit &edit) {
433433
}
434434
// Multiple conversions may be needed to get the right number of
435435
// effective rounded fractional digits.
436-
int extraDigits{0};
437436
bool canIncrease{true};
438-
while (true) {
437+
for (int extraDigits{fracDigits == 0 ? 1 : 0};;) {
439438
decimal::ConversionToDecimalResult converted{
440439
ConvertToDecimal(extraDigits + fracDigits, rounding, flags)};
441-
if (IsInfOrNaN(converted.str, static_cast<int>(converted.length))) {
440+
const char *convertedStr{converted.str};
441+
if (IsInfOrNaN(convertedStr, static_cast<int>(converted.length))) {
442442
return editWidth > 0 &&
443443
converted.length > static_cast<std::size_t>(editWidth)
444444
? EmitRepeated(io_, '*', editWidth)
445445
: EmitPrefix(edit, converted.length, editWidth) &&
446-
EmitAscii(io_, converted.str, converted.length) &&
446+
EmitAscii(io_, convertedStr, converted.length) &&
447447
EmitSuffix(edit);
448448
}
449449
int expo{converted.decimalExponent + edit.modes.scale /*kP*/};
450-
int signLength{*converted.str == '-' || *converted.str == '+' ? 1 : 0};
450+
int signLength{*convertedStr == '-' || *convertedStr == '+' ? 1 : 0};
451451
int convertedDigits{static_cast<int>(converted.length) - signLength};
452452
if (IsZero()) { // don't treat converted "0" as significant digit
453453
expo = 0;
454454
convertedDigits = 0;
455455
}
456-
int trailingOnes{0};
456+
bool isNegative{*convertedStr == '-'};
457+
char one[2];
457458
if (expo > extraDigits && extraDigits >= 0 && canIncrease) {
458459
extraDigits = expo;
459460
if (!edit.digits.has_value()) { // F0
@@ -462,24 +463,45 @@ bool RealOutputEditing<KIND>::EditFOutput(const DataEdit &edit) {
462463
canIncrease = false; // only once
463464
continue;
464465
} else if (expo == -fracDigits && convertedDigits > 0) {
465-
if ((rounding == decimal::FortranRounding::RoundUp &&
466-
*converted.str != '-') ||
467-
(rounding == decimal::FortranRounding::RoundDown &&
468-
*converted.str == '-') ||
469-
(rounding == decimal::FortranRounding::RoundToZero &&
470-
rounding != edit.modes.round && // it changed below
471-
converted.str[signLength] >= '5')) {
472-
// Round up/down to a scaled 1
466+
// Result will be either a signed zero or power of ten, depending
467+
// on rounding.
468+
char leading{convertedStr[signLength]};
469+
bool roundToPowerOfTen{false};
470+
switch (edit.modes.round) {
471+
case decimal::FortranRounding::RoundUp:
472+
roundToPowerOfTen = !isNegative;
473+
break;
474+
case decimal::FortranRounding::RoundDown:
475+
roundToPowerOfTen = isNegative;
476+
break;
477+
case decimal::FortranRounding::RoundToZero:
478+
break;
479+
case decimal::FortranRounding::RoundNearest:
480+
if (leading == '5' &&
481+
rounding == decimal::FortranRounding::RoundNearest) {
482+
// Try again, rounding away from zero.
483+
rounding = isNegative ? decimal::FortranRounding::RoundDown
484+
: decimal::FortranRounding::RoundUp;
485+
extraDigits = 1 - fracDigits; // just one digit needed
486+
continue;
487+
}
488+
roundToPowerOfTen = leading > '5';
489+
break;
490+
case decimal::FortranRounding::RoundCompatible:
491+
roundToPowerOfTen = leading >= '5';
492+
break;
493+
}
494+
if (roundToPowerOfTen) {
473495
++expo;
474-
convertedDigits = 0;
475-
trailingOnes = 1;
476-
} else if (rounding != decimal::FortranRounding::RoundToZero) {
477-
// Convert again with truncation so first digit can be checked
478-
// on the next iteration by the code above
479-
rounding = decimal::FortranRounding::RoundToZero;
480-
continue;
496+
convertedDigits = 1;
497+
if (signLength > 0) {
498+
one[0] = *convertedStr;
499+
one[1] = '1';
500+
} else {
501+
one[0] = '1';
502+
}
503+
convertedStr = one;
481504
} else {
482-
// Value rounds down to zero
483505
expo = 0;
484506
convertedDigits = 0;
485507
}
@@ -493,17 +515,14 @@ bool RealOutputEditing<KIND>::EditFOutput(const DataEdit &edit) {
493515
int digitsAfterPoint{convertedDigits - digitsBeforePoint};
494516
int trailingZeroes{flags & decimal::Minimize
495517
? 0
496-
: std::max(0,
497-
fracDigits -
498-
(zeroesAfterPoint + digitsAfterPoint + trailingOnes))};
518+
: std::max(0, fracDigits - (zeroesAfterPoint + digitsAfterPoint))};
499519
if (digitsBeforePoint + zeroesBeforePoint + zeroesAfterPoint +
500-
digitsAfterPoint + trailingOnes + trailingZeroes ==
520+
digitsAfterPoint + trailingZeroes ==
501521
0) {
502522
zeroesBeforePoint = 1; // "." -> "0."
503523
}
504524
int totalLength{signLength + digitsBeforePoint + zeroesBeforePoint +
505-
1 /*'.'*/ + zeroesAfterPoint + digitsAfterPoint + trailingOnes +
506-
trailingZeroes};
525+
1 /*'.'*/ + zeroesAfterPoint + digitsAfterPoint + trailingZeroes};
507526
int width{editWidth > 0 ? editWidth : totalLength};
508527
if (totalLength > width) {
509528
return EmitRepeated(io_, '*', width);
@@ -513,13 +532,12 @@ bool RealOutputEditing<KIND>::EditFOutput(const DataEdit &edit) {
513532
++totalLength;
514533
}
515534
return EmitPrefix(edit, totalLength, width) &&
516-
EmitAscii(io_, converted.str, signLength + digitsBeforePoint) &&
535+
EmitAscii(io_, convertedStr, signLength + digitsBeforePoint) &&
517536
EmitRepeated(io_, '0', zeroesBeforePoint) &&
518537
EmitAscii(io_, edit.modes.editingFlags & decimalComma ? "," : ".", 1) &&
519538
EmitRepeated(io_, '0', zeroesAfterPoint) &&
520-
EmitAscii(io_, converted.str + signLength + digitsBeforePoint,
539+
EmitAscii(io_, convertedStr + signLength + digitsBeforePoint,
521540
digitsAfterPoint) &&
522-
EmitRepeated(io_, '1', trailingOnes) &&
523541
EmitRepeated(io_, '0', trailingZeroes) &&
524542
EmitRepeated(io_, ' ', trailingBlanks_) && EmitSuffix(edit);
525543
}

flang/unittests/Runtime/NumericalFormatTest.cpp

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -710,26 +710,43 @@ TEST(IOApiTests, FormatDoubleValues) {
710710
{"(F5.3,';')", 0.099999, "0.100;"},
711711
{"(F5.3,';')", 0.0099999, "0.010;"},
712712
{"(F5.3,';')", 0.00099999, "0.001;"},
713-
{"(F5.3,';')", 0.0005, "0.001;"},
714-
{"(F5.3,';')", 0.00049999, "0.000;"},
713+
{"(F5.3,';')",
714+
0.0005000000000000000104083408558608425664715468883514404296875,
715+
"0.001;"},
716+
{"(F5.3,';')",
717+
0.000499999999999999901988123607310399165726266801357269287109375,
718+
"0.000;"},
715719
{"(F5.3,';')", 0.000099999, "0.000;"},
716720
{"(F5.3,';')", -99.999, "*****;"},
717721
{"(F5.3,';')", -9.9999, "*****;"},
718722
{"(F5.3,';')", -0.99999, "*****;"},
719723
{"(F5.3,';')", -0.099999, "-.100;"},
720724
{"(F5.3,';')", -0.0099999, "-.010;"},
721725
{"(F5.3,';')", -0.00099999, "-.001;"},
722-
{"(F5.3,';')", -0.0005, "-.001;"},
723-
{"(F5.3,';')", -0.00049999, "-.000;"},
726+
{"(F5.3,';')",
727+
-0.0005000000000000000104083408558608425664715468883514404296875,
728+
"-.001;"},
729+
{"(F5.3,';')",
730+
-0.000499999999999999901988123607310399165726266801357269287109375,
731+
"-.000;"},
724732
{"(F5.3,';')", -0.000099999, "-.000;"},
725733
{"(F0.1,';')", 0.0, ".0;"},
734+
{"(F5.0,';')", -0.5000000000000001, " -1.;"},
735+
{"(F5.0,';')", -0.5, " -0.;"},
736+
{"(F5.0,';')", -0.49999999999999994, " -0.;"},
737+
{"(F5.0,';')", 0.49999999999999994, " 0.;"},
738+
{"(F5.0,';')", 0.5, " 0.;"},
739+
{"(F5.0,';')", 0.5000000000000001, " 1.;"},
726740
};
727741

728742
for (auto const &[format, value, expect] : individualTestCases) {
729743
std::string got;
744+
char hex[17];
745+
std::snprintf(hex, sizeof hex, "%016llx",
746+
*reinterpret_cast<const unsigned long long *>(&value));
730747
ASSERT_TRUE(CompareFormatReal(format, value, expect, got))
731-
<< "Failed to format " << format << ", expected '" << expect
732-
<< "', got '" << got << "'";
748+
<< "Failed to format " << value << " 0x" << hex << " with format "
749+
<< format << ", expected '" << expect << "', got '" << got << "'";
733750
}
734751

735752
// Problematic EN formatting edge cases with rounding

0 commit comments

Comments
 (0)