[flang][semantics] fix issue with equality of min/max in module files (#145824)

Convert all binary calls of min/max to extremum operations, so that
extremums generated by the compiler compare equal, and user min/max
calls also compare equal.

Fixes #133646

Originally opened as #144162 but I accidentally pushed a merge in such a
way that a bunch of code owners got added to the review. This is just
rebasing the original work on main and fixing the failing tests.
This commit is contained in:
Andre Kuhlenschmidt
2025-06-26 12:15:57 -07:00
committed by GitHub
parent e25db2f6b3
commit 283c2e8d7c
10 changed files with 169 additions and 93 deletions

View File

@@ -1102,6 +1102,8 @@ template <typename T> Expr<T> Folder<T>::TRANSFER(FunctionRef<T> &&funcRef) {
}
}
// TODO: Once the backend supports character extremums we could support
// min/max with non-optional arguments to trees of extremum operations.
template <typename T>
Expr<T> FoldMINorMAX(
FoldingContext &context, FunctionRef<T> &&funcRef, Ordering order) {
@@ -1109,42 +1111,76 @@ Expr<T> FoldMINorMAX(
T::category == TypeCategory::Unsigned ||
T::category == TypeCategory::Real ||
T::category == TypeCategory::Character);
// Lots of constraints:
// - We want Extremum<T> generated by semantics to compare equal to
// Extremum<T> written out to module files as max or min calls.
// - Users can also write min/max calls that must also compare equal
// to min/max calls that wind up being written to module files.
// - Extremeum<T> is binary and can't currently handle processing
// optional arguments that may show up in 3rd + argument.
// - The code below only accepts more than 2 arguments if all the
// arguments are constant (and hence known to be present).
// - ConvertExprToHLFIR can't currently handle Extremum<Character>
// - Semantics doesn't currently generate Extremum<Character>
// The original code did the folding of arguments and the overall extremum
// operation in a single pass. This was shorter code-wise, but took me
// a while to tease out all the logic and was doing redundant work.
// So I split it into two passes:
// 1) fold the arguments and check if they are constant,
// 2) Decide if we:
// - can constant-fold the min/max operation, or
// - need to generate an extremum anyway,
// and do it if so.
// Otherwise, return the original call.
auto &args{funcRef.arguments()};
bool ok{true};
std::optional<Expr<T>> result;
Folder<T> folder{context};
for (std::optional<ActualArgument> &arg : args) {
// Call Folding on all arguments to make operand promotion explicit.
if (!folder.Folding(arg)) {
// TODO: Lowering can't handle having every FunctionRef for max and min
// being converted into Extremum<T>. That needs fixing. Until that
// is corrected, however, it is important that max and min references
// in module files be converted into Extremum<T> even when not constant;
// the Extremum<SubscriptInteger> operations created to normalize the
// values of array bounds are formatted as max operations in the
// declarations in modules, and need to be read back in as such in
// order for expression comparison to not produce false inequalities
// when checking function results for procedure interface compatibility.
if (!context.moduleFileName()) {
ok = false;
}
std::size_t nargs{args.size()};
bool allArgsConstant{true};
bool extremumAnyway{nargs == 2 && T::category != TypeCategory::Character};
// 1a)Fold the first two arguments.
{
Folder<T> folder{context, /*forOptionalArgument=*/false};
if (!folder.Folding(args[0])) {
allArgsConstant = false;
}
Expr<SomeType> *argExpr{arg ? arg->UnwrapExpr() : nullptr};
if (argExpr) {
*argExpr = Fold(context, std::move(*argExpr));
}
if (Expr<T> * tExpr{UnwrapExpr<Expr<T>>(argExpr)}) {
if (result) {
result = FoldOperation(
context, Extremum<T>{order, std::move(*result), Expr<T>{*tExpr}});
} else {
result = Expr<T>{*tExpr};
}
} else {
ok = false;
if (!folder.Folding(args[1])) {
allArgsConstant = false;
}
}
return ok && result ? std::move(*result) : Expr<T>{std::move(funcRef)};
// 1b) Fold any optional arguments.
if (nargs > 2) {
Folder<T> folder{context, /*forOptionalArgument=*/true};
for (std::size_t i{2}; i < nargs; ++i) {
if (args[i]) {
if (!folder.Folding(args[i])) {
allArgsConstant = false;
}
}
}
}
// 2) If we can fold the result or the call to min/max may compare equal to
// an extremum generated by semantics go ahead and convert to an extremum,
// and try to fold the result.
if (allArgsConstant || extremumAnyway) {
// Folding updates the argument expressions in place, no need to call
// Fold() on each argument again.
if (const auto *resultp{UnwrapExpr<Expr<T>>(args[0])}) {
Expr<T> result{*resultp};
for (std::size_t i{1}; i < nargs; ++i) {
if (const auto *tExpr{UnwrapExpr<Expr<T>>(args[i])}) {
result = FoldOperation(
context, Extremum<T>{order, std::move(result), *tExpr});
} else {
// This should never happen, but here is a value to return.
return Expr<T>{std::move(funcRef)};
}
}
return result;
}
}
// If we decided to not generate an extremum just return the original call,
// with the arguments folded.
return Expr<T>{std::move(funcRef)};
}
// For AMAX0, AMIN0, AMAX1, AMIN1, DMAX1, DMIN1, MAX0, MIN0, MAX1, and MIN1