Skip to content

[flang][fold] fix bug with folding min/max #144162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[flang][fold] fix bug with folding min/max #144162

wants to merge 6 commits into from

Conversation

akuhlens
Copy link
Contributor

@akuhlens akuhlens commented Jun 13, 2025 •
edited
Loading

Convert all binary calls of min/max to extremum operations.

Fixes #133646

@akuhlens akuhlens changed the title [flang][fold] fix bug with folding mix/max [flang][fold] fix bug with folding min/max Jun 13, 2025
@akuhlens akuhlens marked this pull request as ready for review June 16, 2025 19:21
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:semantics labels Jun 16, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 16, 2025 •
edited
Loading

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-openmp

Author: Andre Kuhlenschmidt (akuhlens)

Changes

Convert all binary calls of min/max to extremum operations.

Fixes #133646

Patch is 27.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144162.diff

9 Files Affected:

  • (modified) flang/lib/Evaluate/fold-implementation.h (+66-30)
  • (modified) flang/test/Lower/HLFIR/custom-intrinsic.f90 (+8-8)
  • (modified) flang/test/Lower/OpenMP/reduction-array-intrinsic.f90 (+4-4)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90 (+29-29)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90 (+5-5)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max.f90 (+5-5)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min-byref.f90 (+5-5)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min.f90 (+5-5)
  • (added) flang/test/Semantics/function-result-extent-max.f90 (+32)
diff --git a/flang/lib/Evaluate/fold-implementation.h b/flang/lib/Evaluate/fold-implementation.h
index b0f39e63d0941..346d615ab12ea 100644
--- a/flang/lib/Evaluate/fold-implementation.h
+++ b/flang/lib/Evaluate/fold-implementation.h
@@ -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 source 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;
+  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, false};
+    if (!folder.Folding(args[0])) {
+      allArgsConstant = false;
+    }
+    if (!folder.Folding(args[1])) {
+      allArgsConstant = false;
+    }
+  }
+  // 1b) Fold any optional arguments.
+  if (nargs > 2) {
+    Folder<T> folder{context, true};
+    for (size_t i{2}; i < nargs; ++i) {
+      if (args[i]) {
+        if (!folder.Folding(args[i])) {
+          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};
+  }
+  // 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 (auto *resultp{UnwrapExpr<Expr<T>>(args[0])}) {
+      Expr<T> result{*resultp};
+      for (size_t i{1}; i < nargs; ++i) {
+        if (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)};
+        }
       }
-    } else {
-      ok = false;
+      return result;
     }
   }
-  return ok && result ? std::move(*result) : Expr<T>{std::move(funcRef)};
+  // 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
diff --git a/flang/test/Lower/HLFIR/custom-intrinsic.f90 b/flang/test/Lower/HLFIR/custom-intrinsic.f90
index 161a2ab75b7c8..5ec6e0a17e9ac 100644
--- a/flang/test/Lower/HLFIR/custom-intrinsic.f90
+++ b/flang/test/Lower/HLFIR/custom-intrinsic.f90
@@ -115,10 +115,10 @@ function max_array(a, b)
 ! CHECK:           %[[VAL_11:.*]]:2 = hlfir.declare %[[VAL_9]](%[[VAL_10]]) {uniq_name = "_QFmax_arrayEmax_array"} : (!fir.ref<!fir.array<42xi32>>, !fir.shape<1>) -> (!fir.ref<!fir.array<42xi32>>, !fir.ref<!fir.array<42xi32>>)
 ! CHECK:           %[[VAL_12:.*]] = hlfir.elemental %[[VAL_3]] unordered : (!fir.shape<1>) -> !hlfir.expr<42xi32> {
 ! CHECK:           ^bb0(%[[VAL_13:.*]]: index):
-! CHECK:             %[[VAL_14:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
-! CHECK:             %[[VAL_15:.*]] = fir.load %[[VAL_14]] : !fir.ref<i32>
-! CHECK:             %[[VAL_16:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
-! CHECK:             %[[VAL_17:.*]] = fir.load %[[VAL_16]] : !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_14:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_15:.*]] = fir.load %[[VAL_14]] : !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_16:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_17:.*]] = fir.load %[[VAL_16]] : !fir.ref<i32>
 ! CHECK:             %[[VAL_18:.*]] = arith.cmpi sgt, %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             %[[VAL_19:.*]] = arith.select %[[VAL_18]], %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             hlfir.yield_element %[[VAL_19]] : i32
@@ -288,10 +288,10 @@ function min_array(a, b)
 ! CHECK:           %[[VAL_11:.*]]:2 = hlfir.declare %[[VAL_9]](%[[VAL_10]]) {uniq_name = "_QFmin_arrayEmin_array"} : (!fir.ref<!fir.array<42xi32>>, !fir.shape<1>) -> (!fir.ref<!fir.array<42xi32>>, !fir.ref<!fir.array<42xi32>>)
 ! CHECK:           %[[VAL_12:.*]] = hlfir.elemental %[[VAL_3]] unordered : (!fir.shape<1>) -> !hlfir.expr<42xi32> {
 ! CHECK:           ^bb0(%[[VAL_13:.*]]: index):
-! CHECK:             %[[VAL_14:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
-! CHECK:             %[[VAL_15:.*]] = fir.load %[[VAL_14]] : !fir.ref<i32>
-! CHECK:             %[[VAL_16:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
-! CHECK:             %[[VAL_17:.*]] = fir.load %[[VAL_16]] : !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_14:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_15:.*]] = fir.load %[[VAL_14]] : !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_16:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_17:.*]] = fir.load %[[VAL_16]] : !fir.ref<i32>
 ! CHECK:             %[[VAL_18:.*]] = arith.cmpi slt, %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             %[[VAL_19:.*]] = arith.select %[[VAL_18]], %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             hlfir.yield_element %[[VAL_19]] : i32
diff --git a/flang/test/Lower/OpenMP/reduction-array-intrinsic.f90 b/flang/test/Lower/OpenMP/reduction-array-intrinsic.f90
index 8b4f37278185e..0cf88cf889868 100644
--- a/flang/test/Lower/OpenMP/reduction-array-intrinsic.f90
+++ b/flang/test/Lower/OpenMP/reduction-array-intrinsic.f90
@@ -82,10 +82,10 @@ subroutine max_array_reduction(l, r)
 ! CHECK:               %[[VAL_16:.*]] = arith.constant 1 : index
 ! CHECK:               %[[VAL_17:.*]] = arith.subi %[[VAL_15]]#0, %[[VAL_16]] : index
 ! CHECK:               %[[VAL_18:.*]] = arith.addi %[[VAL_13]], %[[VAL_17]] : index
-! CHECK:               %[[VAL_19:.*]] = hlfir.designate %[[VAL_8]] (%[[VAL_18]])  : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
-! CHECK:               %[[VAL_20:.*]] = fir.load %[[VAL_19]] : !fir.ref<i32>
-! CHECK:               %[[VAL_21:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
-! CHECK:               %[[VAL_22:.*]] = fir.load %[[VAL_21]] : !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_19:.*]] = hlfir.designate %[[VAL_8]] (%[[VAL_18]])  : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_20:.*]] = fir.load %[[VAL_19]] : !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_21:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_22:.*]] = fir.load %[[VAL_21]] : !fir.ref<i32>
 ! CHECK:               %[[VAL_23:.*]] = arith.cmpi sgt, %[[VAL_20]], %[[VAL_22]] : i32
 ! CHECK:               %[[VAL_24:.*]] = arith.select %[[VAL_23]], %[[VAL_20]], %[[VAL_22]] : i32
 ! CHECK:               hlfir.yield_element %[[VAL_24]] : i32
diff --git a/flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90 b/flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90
index 5b4c5e65ffccc..58b68e5ec4cfd 100644
--- a/flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90
+++ b/flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90
@@ -228,11 +228,11 @@ program reduce15
 ! CHECK:                 %[[VAL_56:.*]]:2 = hlfir.declare %[[VAL_55]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 %[[VAL_62:.*]]:2 = hlfir.declare %[[VAL_60]] {fortran_attrs = {{.*}}<allocatable>, uniq_name = "_QFEmaxes"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
 ! CHECK:                 hlfir.assign %[[VAL_61]] to %[[VAL_56]]#0 : i32, !fir.ref<i32>
-! CHECK:                 %[[VAL_63:.*]] = fir.load %[[VAL_1]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
-! CHECK:                 %[[VAL_64:.*]] = arith.constant 0 : index
-! CHECK:                 %[[VAL_65:.*]]:3 = fir.box_dims %[[VAL_63]], %[[VAL_64]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
-! CHECK:                 %[[VAL_66:.*]] = fir.shape %[[VAL_65]]#1 : (index) -> !fir.shape<1>
-! CHECK:                 %[[VAL_67:.*]] = fir.load %[[VAL_62]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK-DAG:             %[[VAL_63:.*]] = fir.load %[[VAL_1]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK-DAG:             %[[VAL_64:.*]] = arith.constant 0 : index
+! CHECK-DAG:             %[[VAL_65:.*]]:3 = fir.box_dims %[[VAL_63]], %[[VAL_64]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
+! CHECK-DAG:             %[[VAL_66:.*]] = fir.shape %[[VAL_65]]#1 : (index) -> !fir.shape<1>
+! CHECK-DAG:             %[[VAL_67:.*]] = fir.load %[[VAL_62]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
 ! CHECK:                 %[[VAL_68:.*]] = hlfir.elemental %[[VAL_66]] unordered : (!fir.shape<1>) -> !hlfir.expr<?xi32> {
 ! CHECK:                 ^bb0(%[[VAL_69:.*]]: index):
 ! CHECK:                   %[[VAL_70:.*]] = arith.constant 0 : index
@@ -240,15 +240,15 @@ program reduce15
 ! CHECK:                   %[[VAL_72:.*]] = arith.constant 1 : index
 ! CHECK:                   %[[VAL_73:.*]] = arith.subi %[[VAL_71]]#0, %[[VAL_72]] : index
 ! CHECK:                   %[[VAL_74:.*]] = arith.addi %[[VAL_69]], %[[VAL_73]] : index
-! CHECK:                   %[[VAL_75:.*]] = hlfir.designate %[[VAL_63]] (%[[VAL_74]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
-! CHECK:                   %[[VAL_76:.*]] = fir.load %[[VAL_75]] : !fir.ref<i32>
-! CHECK:                   %[[VAL_77:.*]] = arith.constant 0 : index
-! CHECK:                   %[[VAL_78:.*]]:3 = fir.box_dims %[[VAL_67]], %[[VAL_77]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
-! CHECK:                   %[[VAL_79:.*]] = arith.constant 1 : index
-! CHECK:                   %[[VAL_80:.*]] = arith.subi %[[VAL_78]]#0, %[[VAL_79]] : index
-! CHECK:                   %[[VAL_81:.*]] = arith.addi %[[VAL_69]], %[[VAL_80]] : index
-! CHECK:                   %[[VAL_82:.*]] = hlfir.designate %[[VAL_67]] (%[[VAL_81]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
-! CHECK:                   %[[VAL_83:.*]] = fir.load %[[VAL_82]] : !fir.ref<i32>
+! CHECK-DAG:                   %[[VAL_75:.*]] = hlfir.designate %[[VAL_63]] (%[[VAL_74]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
+! CHECK-DAG:                   %[[VAL_76:.*]] = fir.load %[[VAL_75]] : !fir.ref<i32>
+! CHECK-DAG:                   %[[VAL_77:.*]] = arith.constant 0 : index
+! CHECK-DAG:                   %[[VAL_78:.*]]:3 = fir.box_dims %[[VAL_67]], %[[VAL_77]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
+! CHECK-DAG:                   %[[VAL_79:.*]] = arith.constant 1 : index
+! CHECK-DAG:                   %[[VAL_80:.*]] = arith.subi %[[VAL_78]]#0, %[[VAL_79]] : index
+! CHECK-DAG:                   %[[VAL_81:.*]] = arith.addi %[[VAL_69]], %[[VAL_80]] : index
+! CHECK-DAG:                   %[[VAL_82:.*]] = hlfir.designate %[[VAL_67]] (%[[VAL_81]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
+! CHECK-DAG:                   %[[VAL_83:.*]] = fir.load %[[VAL_82]] : !fir.ref<i32>
 ! CHECK:                   %[[VAL_84:.*]] = arith.cmpi sgt, %[[VAL_76]], %[[VAL_83]] : i32
 ! CHECK:                   %[[VAL_85:.*]] = arith.select %[[VAL_84]], %[[VAL_76]], %[[VAL_83]] : i32
 ! CHECK:                   hlfir.yield_element %[[VAL_85]] : i32
@@ -269,27 +269,27 @@ program reduce15
 ! CHECK:                 %[[VAL_88:.*]]:2 = hlfir.declare %[[VAL_87]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 %[[VAL_94:.*]]:2 = hlfir.declare %[[VAL_92]] {fortran_attrs = {{.*}}<allocatable>, uniq_name = "_QFEmins"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
 ! CHECK:                 hlfir.assign %[[VAL_93]] to %[[VAL_88]]#0 : i32, !fir.ref<i32>
-! CHECK:                 %[[VAL_95:.*]] = fir.load %[[VAL_1]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
-! CHECK:                 %[[VAL_96:.*]] = arith.constant 0 : index
-! CHECK:                 %[[VAL_97:.*]]:3 = fir.box_dims %[[VAL_95]], %[[VAL_96]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
-! CHECK:                 %[[VAL_98:.*]] = fir.shape %[[VAL_97]]#1 : (index) -> !fir.shape<1>
-! CHECK:                 %[[VAL_99:.*]] = fir.load %[[VAL_94]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
-! CHECK:                 %[[VAL_100:.*]] = hlfir.elemental %[[VAL_98]] unordered : (!fir.shape<1>) -> !hlfir.expr<?xi32> {
+! CHECK-DAG:                 %[[VAL_95:.*]] = fir.load %[[VAL_1]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK-DAG:                 %[[VAL_96:.*]] = arith.constant 0 : index
+! CHECK-DAG:                 %[[VAL_97:.*]]:3 = fir.box_dims %[[VAL_95]], %[[VAL_96]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
+! CHECK-DAG:                 %[[VAL_98:.*]] = fir.shape %[[VAL_97]]#1 : (index) -> !fir.shape<1>
+! CHECK-DAG:                 %[[VAL_99:.*]] = fir.load %[[VAL_94]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK-DAG:                 %[[VAL_100:.*]] = hlfir.elemental %[[VAL_98]] unordered : (!fir.shape<1>) -> !hlfir.expr<?xi32> {
 ! CHECK:                 ^bb0(%[[VAL_101:.*]]: index):
 ! CHECK:                   %[[VAL_102:.*]] = arith.constant 0 : index
 ! CHECK:                   %[[VAL_103:.*]]:3 = fir.box_dims %[[VAL_95]], %[[VAL_102]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
 ! CHECK:                   %[[VAL_104:.*]] = arith.constant 1 : index
 ! CHECK:                   %[[VAL_105:.*]] = arith.subi %[[VAL_103]]#0, %[[VAL_104]] : index
 ! CHECK:                   %[[VAL_106:.*]] = arith.addi %[[VAL_101]], %[[VAL_105]] : index
-! CHECK:                   %[[VAL_107:.*]] = hlfir.designate %[[VAL_95]] (%[[VAL_106]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
-! CHECK:                   %[[VAL_108:.*]] = fir.load %[[VAL_107]] : !fir.ref<i32>
-! CHECK:                   %[[VAL_109:.*]] = arith.constant 0 : index
-! CHECK:                   %[[VAL_110:.*]]:3 = fir.box_dims %[[VAL_99]], %[[VAL_109]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
-! CHECK:                   %[[VAL_111:.*]] = arith.constant 1 : index
-! CHECK:                   %[[VAL_112:.*]] = arith.subi %[[VAL_110]]#0, %[[VAL_111]] : index
-! CHECK:                   %[[VAL_113:.*]] = arith.addi %[[VAL_101]], %[[VAL_112]] : index
-! CHECK:                   %[[VAL_114:.*]] = hlfir.designate %[[VAL_99]] (%[[VAL_113]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
-! CHECK:                   %[[VAL_115:.*]] = fir.load %[[VAL_114]] : !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_107:.*]] = hlfir.designate %[[VAL_95]] (%[[VAL_106]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_108:.*]] = fir.load %[[VAL_107]] : !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_109:.*]] = arith.constant 0 : index
+! CHECK-DAG:               %[[VAL_110:.*]]:3 = fir.box_dims %[[VAL_99]], %[[VAL_109]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
+! CHECK-DAG:               %[[VAL_111:.*]] = arith.constant 1 : index
+! CHECK-DAG:               %[[VAL_112:.*]] = arith.subi %[[VAL_110]]#0, %[[VAL_111]] : index
+! CHECK-DAG:               %[[VAL_113:.*]] = arith.addi %[[VAL_101]], %[[VAL_112]] : index
+! CHECK-DAG:               %[[VAL_114:.*]] = hlfir.designate %[[VAL_99]] (%[[VAL_113]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_115:.*]] = fir.load %[[VAL_114]] : !fir.ref<i32>
 ! CHECK:                   %[[VAL_116:.*]] = arith.cmpi slt, %[[VAL_108]], %[[VAL_115]] : i32
 ! CHECK:                   %[[VAL_117:.*]] = arith.select %[[VAL_116]], %[[VAL_108]], %[[VAL_115]] : i32
 ! CHECK:                   hlfir.yield_element %[[VAL_117]] : i32
diff --git a/flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90 b/flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90
index d27804fb5606e..69219331ab3ab 100644
--- a/flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90
+++ b/flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90
@@ -53,11 +53,11 @@
 ! CHECK:                 %[[VAL_8:.*]]:2 = hlfir.declare %[[VAL_7]] {uniq_name = "_QFreduction_max_intEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 %[[VAL_14:.*]]:2 = hlfir.declare %[[VAL_12]] {uniq_name = "_QFreduction_max_intEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 hlfir.assi...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jun 16, 2025

@llvm/pr-subscribers-flang-semantics

Author: Andre Kuhlenschmidt (akuhlens)

Changes

Convert all binary calls of min/max to extremum operations.

Fixes #133646

Patch is 27.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144162.diff

9 Files Affected:

  • (modified) flang/lib/Evaluate/fold-implementation.h (+66-30)
  • (modified) flang/test/Lower/HLFIR/custom-intrinsic.f90 (+8-8)
  • (modified) flang/test/Lower/OpenMP/reduction-array-intrinsic.f90 (+4-4)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90 (+29-29)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90 (+5-5)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max.f90 (+5-5)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min-byref.f90 (+5-5)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min.f90 (+5-5)
  • (added) flang/test/Semantics/function-result-extent-max.f90 (+32)
diff --git a/flang/lib/Evaluate/fold-implementation.h b/flang/lib/Evaluate/fold-implementation.h
index b0f39e63d0941..346d615ab12ea 100644
--- a/flang/lib/Evaluate/fold-implementation.h
+++ b/flang/lib/Evaluate/fold-implementation.h
@@ -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 source 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;
+  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, false};
+    if (!folder.Folding(args[0])) {
+      allArgsConstant = false;
+    }
+    if (!folder.Folding(args[1])) {
+      allArgsConstant = false;
+    }
+  }
+  // 1b) Fold any optional arguments.
+  if (nargs > 2) {
+    Folder<T> folder{context, true};
+    for (size_t i{2}; i < nargs; ++i) {
+      if (args[i]) {
+        if (!folder.Folding(args[i])) {
+          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};
+  }
+  // 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 (auto *resultp{UnwrapExpr<Expr<T>>(args[0])}) {
+      Expr<T> result{*resultp};
+      for (size_t i{1}; i < nargs; ++i) {
+        if (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)};
+        }
       }
-    } else {
-      ok = false;
+      return result;
     }
   }
-  return ok && result ? std::move(*result) : Expr<T>{std::move(funcRef)};
+  // 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
diff --git a/flang/test/Lower/HLFIR/custom-intrinsic.f90 b/flang/test/Lower/HLFIR/custom-intrinsic.f90
index 161a2ab75b7c8..5ec6e0a17e9ac 100644
--- a/flang/test/Lower/HLFIR/custom-intrinsic.f90
+++ b/flang/test/Lower/HLFIR/custom-intrinsic.f90
@@ -115,10 +115,10 @@ function max_array(a, b)
 ! CHECK:           %[[VAL_11:.*]]:2 = hlfir.declare %[[VAL_9]](%[[VAL_10]]) {uniq_name = "_QFmax_arrayEmax_array"} : (!fir.ref<!fir.array<42xi32>>, !fir.shape<1>) -> (!fir.ref<!fir.array<42xi32>>, !fir.ref<!fir.array<42xi32>>)
 ! CHECK:           %[[VAL_12:.*]] = hlfir.elemental %[[VAL_3]] unordered : (!fir.shape<1>) -> !hlfir.expr<42xi32> {
 ! CHECK:           ^bb0(%[[VAL_13:.*]]: index):
-! CHECK:             %[[VAL_14:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
-! CHECK:             %[[VAL_15:.*]] = fir.load %[[VAL_14]] : !fir.ref<i32>
-! CHECK:             %[[VAL_16:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
-! CHECK:             %[[VAL_17:.*]] = fir.load %[[VAL_16]] : !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_14:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_15:.*]] = fir.load %[[VAL_14]] : !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_16:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_17:.*]] = fir.load %[[VAL_16]] : !fir.ref<i32>
 ! CHECK:             %[[VAL_18:.*]] = arith.cmpi sgt, %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             %[[VAL_19:.*]] = arith.select %[[VAL_18]], %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             hlfir.yield_element %[[VAL_19]] : i32
@@ -288,10 +288,10 @@ function min_array(a, b)
 ! CHECK:           %[[VAL_11:.*]]:2 = hlfir.declare %[[VAL_9]](%[[VAL_10]]) {uniq_name = "_QFmin_arrayEmin_array"} : (!fir.ref<!fir.array<42xi32>>, !fir.shape<1>) -> (!fir.ref<!fir.array<42xi32>>, !fir.ref<!fir.array<42xi32>>)
 ! CHECK:           %[[VAL_12:.*]] = hlfir.elemental %[[VAL_3]] unordered : (!fir.shape<1>) -> !hlfir.expr<42xi32> {
 ! CHECK:           ^bb0(%[[VAL_13:.*]]: index):
-! CHECK:             %[[VAL_14:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
-! CHECK:             %[[VAL_15:.*]] = fir.load %[[VAL_14]] : !fir.ref<i32>
-! CHECK:             %[[VAL_16:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
-! CHECK:             %[[VAL_17:.*]] = fir.load %[[VAL_16]] : !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_14:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_15:.*]] = fir.load %[[VAL_14]] : !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_16:.*]] = hlfir.designate %[[VAL_7]]#0 (%[[VAL_13]])  : (!fir.ref<!fir.array<42xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:             %[[VAL_17:.*]] = fir.load %[[VAL_16]] : !fir.ref<i32>
 ! CHECK:             %[[VAL_18:.*]] = arith.cmpi slt, %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             %[[VAL_19:.*]] = arith.select %[[VAL_18]], %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             hlfir.yield_element %[[VAL_19]] : i32
diff --git a/flang/test/Lower/OpenMP/reduction-array-intrinsic.f90 b/flang/test/Lower/OpenMP/reduction-array-intrinsic.f90
index 8b4f37278185e..0cf88cf889868 100644
--- a/flang/test/Lower/OpenMP/reduction-array-intrinsic.f90
+++ b/flang/test/Lower/OpenMP/reduction-array-intrinsic.f90
@@ -82,10 +82,10 @@ subroutine max_array_reduction(l, r)
 ! CHECK:               %[[VAL_16:.*]] = arith.constant 1 : index
 ! CHECK:               %[[VAL_17:.*]] = arith.subi %[[VAL_15]]#0, %[[VAL_16]] : index
 ! CHECK:               %[[VAL_18:.*]] = arith.addi %[[VAL_13]], %[[VAL_17]] : index
-! CHECK:               %[[VAL_19:.*]] = hlfir.designate %[[VAL_8]] (%[[VAL_18]])  : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
-! CHECK:               %[[VAL_20:.*]] = fir.load %[[VAL_19]] : !fir.ref<i32>
-! CHECK:               %[[VAL_21:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
-! CHECK:               %[[VAL_22:.*]] = fir.load %[[VAL_21]] : !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_19:.*]] = hlfir.designate %[[VAL_8]] (%[[VAL_18]])  : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_20:.*]] = fir.load %[[VAL_19]] : !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_21:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_13]])  : (!fir.box<!fir.array<?xi32>>, index) -> !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_22:.*]] = fir.load %[[VAL_21]] : !fir.ref<i32>
 ! CHECK:               %[[VAL_23:.*]] = arith.cmpi sgt, %[[VAL_20]], %[[VAL_22]] : i32
 ! CHECK:               %[[VAL_24:.*]] = arith.select %[[VAL_23]], %[[VAL_20]], %[[VAL_22]] : i32
 ! CHECK:               hlfir.yield_element %[[VAL_24]] : i32
diff --git a/flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90 b/flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90
index 5b4c5e65ffccc..58b68e5ec4cfd 100644
--- a/flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90
+++ b/flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90
@@ -228,11 +228,11 @@ program reduce15
 ! CHECK:                 %[[VAL_56:.*]]:2 = hlfir.declare %[[VAL_55]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 %[[VAL_62:.*]]:2 = hlfir.declare %[[VAL_60]] {fortran_attrs = {{.*}}<allocatable>, uniq_name = "_QFEmaxes"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
 ! CHECK:                 hlfir.assign %[[VAL_61]] to %[[VAL_56]]#0 : i32, !fir.ref<i32>
-! CHECK:                 %[[VAL_63:.*]] = fir.load %[[VAL_1]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
-! CHECK:                 %[[VAL_64:.*]] = arith.constant 0 : index
-! CHECK:                 %[[VAL_65:.*]]:3 = fir.box_dims %[[VAL_63]], %[[VAL_64]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
-! CHECK:                 %[[VAL_66:.*]] = fir.shape %[[VAL_65]]#1 : (index) -> !fir.shape<1>
-! CHECK:                 %[[VAL_67:.*]] = fir.load %[[VAL_62]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK-DAG:             %[[VAL_63:.*]] = fir.load %[[VAL_1]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK-DAG:             %[[VAL_64:.*]] = arith.constant 0 : index
+! CHECK-DAG:             %[[VAL_65:.*]]:3 = fir.box_dims %[[VAL_63]], %[[VAL_64]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
+! CHECK-DAG:             %[[VAL_66:.*]] = fir.shape %[[VAL_65]]#1 : (index) -> !fir.shape<1>
+! CHECK-DAG:             %[[VAL_67:.*]] = fir.load %[[VAL_62]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
 ! CHECK:                 %[[VAL_68:.*]] = hlfir.elemental %[[VAL_66]] unordered : (!fir.shape<1>) -> !hlfir.expr<?xi32> {
 ! CHECK:                 ^bb0(%[[VAL_69:.*]]: index):
 ! CHECK:                   %[[VAL_70:.*]] = arith.constant 0 : index
@@ -240,15 +240,15 @@ program reduce15
 ! CHECK:                   %[[VAL_72:.*]] = arith.constant 1 : index
 ! CHECK:                   %[[VAL_73:.*]] = arith.subi %[[VAL_71]]#0, %[[VAL_72]] : index
 ! CHECK:                   %[[VAL_74:.*]] = arith.addi %[[VAL_69]], %[[VAL_73]] : index
-! CHECK:                   %[[VAL_75:.*]] = hlfir.designate %[[VAL_63]] (%[[VAL_74]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
-! CHECK:                   %[[VAL_76:.*]] = fir.load %[[VAL_75]] : !fir.ref<i32>
-! CHECK:                   %[[VAL_77:.*]] = arith.constant 0 : index
-! CHECK:                   %[[VAL_78:.*]]:3 = fir.box_dims %[[VAL_67]], %[[VAL_77]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
-! CHECK:                   %[[VAL_79:.*]] = arith.constant 1 : index
-! CHECK:                   %[[VAL_80:.*]] = arith.subi %[[VAL_78]]#0, %[[VAL_79]] : index
-! CHECK:                   %[[VAL_81:.*]] = arith.addi %[[VAL_69]], %[[VAL_80]] : index
-! CHECK:                   %[[VAL_82:.*]] = hlfir.designate %[[VAL_67]] (%[[VAL_81]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
-! CHECK:                   %[[VAL_83:.*]] = fir.load %[[VAL_82]] : !fir.ref<i32>
+! CHECK-DAG:                   %[[VAL_75:.*]] = hlfir.designate %[[VAL_63]] (%[[VAL_74]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
+! CHECK-DAG:                   %[[VAL_76:.*]] = fir.load %[[VAL_75]] : !fir.ref<i32>
+! CHECK-DAG:                   %[[VAL_77:.*]] = arith.constant 0 : index
+! CHECK-DAG:                   %[[VAL_78:.*]]:3 = fir.box_dims %[[VAL_67]], %[[VAL_77]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
+! CHECK-DAG:                   %[[VAL_79:.*]] = arith.constant 1 : index
+! CHECK-DAG:                   %[[VAL_80:.*]] = arith.subi %[[VAL_78]]#0, %[[VAL_79]] : index
+! CHECK-DAG:                   %[[VAL_81:.*]] = arith.addi %[[VAL_69]], %[[VAL_80]] : index
+! CHECK-DAG:                   %[[VAL_82:.*]] = hlfir.designate %[[VAL_67]] (%[[VAL_81]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
+! CHECK-DAG:                   %[[VAL_83:.*]] = fir.load %[[VAL_82]] : !fir.ref<i32>
 ! CHECK:                   %[[VAL_84:.*]] = arith.cmpi sgt, %[[VAL_76]], %[[VAL_83]] : i32
 ! CHECK:                   %[[VAL_85:.*]] = arith.select %[[VAL_84]], %[[VAL_76]], %[[VAL_83]] : i32
 ! CHECK:                   hlfir.yield_element %[[VAL_85]] : i32
@@ -269,27 +269,27 @@ program reduce15
 ! CHECK:                 %[[VAL_88:.*]]:2 = hlfir.declare %[[VAL_87]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 %[[VAL_94:.*]]:2 = hlfir.declare %[[VAL_92]] {fortran_attrs = {{.*}}<allocatable>, uniq_name = "_QFEmins"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
 ! CHECK:                 hlfir.assign %[[VAL_93]] to %[[VAL_88]]#0 : i32, !fir.ref<i32>
-! CHECK:                 %[[VAL_95:.*]] = fir.load %[[VAL_1]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
-! CHECK:                 %[[VAL_96:.*]] = arith.constant 0 : index
-! CHECK:                 %[[VAL_97:.*]]:3 = fir.box_dims %[[VAL_95]], %[[VAL_96]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
-! CHECK:                 %[[VAL_98:.*]] = fir.shape %[[VAL_97]]#1 : (index) -> !fir.shape<1>
-! CHECK:                 %[[VAL_99:.*]] = fir.load %[[VAL_94]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
-! CHECK:                 %[[VAL_100:.*]] = hlfir.elemental %[[VAL_98]] unordered : (!fir.shape<1>) -> !hlfir.expr<?xi32> {
+! CHECK-DAG:                 %[[VAL_95:.*]] = fir.load %[[VAL_1]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK-DAG:                 %[[VAL_96:.*]] = arith.constant 0 : index
+! CHECK-DAG:                 %[[VAL_97:.*]]:3 = fir.box_dims %[[VAL_95]], %[[VAL_96]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
+! CHECK-DAG:                 %[[VAL_98:.*]] = fir.shape %[[VAL_97]]#1 : (index) -> !fir.shape<1>
+! CHECK-DAG:                 %[[VAL_99:.*]] = fir.load %[[VAL_94]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK-DAG:                 %[[VAL_100:.*]] = hlfir.elemental %[[VAL_98]] unordered : (!fir.shape<1>) -> !hlfir.expr<?xi32> {
 ! CHECK:                 ^bb0(%[[VAL_101:.*]]: index):
 ! CHECK:                   %[[VAL_102:.*]] = arith.constant 0 : index
 ! CHECK:                   %[[VAL_103:.*]]:3 = fir.box_dims %[[VAL_95]], %[[VAL_102]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
 ! CHECK:                   %[[VAL_104:.*]] = arith.constant 1 : index
 ! CHECK:                   %[[VAL_105:.*]] = arith.subi %[[VAL_103]]#0, %[[VAL_104]] : index
 ! CHECK:                   %[[VAL_106:.*]] = arith.addi %[[VAL_101]], %[[VAL_105]] : index
-! CHECK:                   %[[VAL_107:.*]] = hlfir.designate %[[VAL_95]] (%[[VAL_106]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
-! CHECK:                   %[[VAL_108:.*]] = fir.load %[[VAL_107]] : !fir.ref<i32>
-! CHECK:                   %[[VAL_109:.*]] = arith.constant 0 : index
-! CHECK:                   %[[VAL_110:.*]]:3 = fir.box_dims %[[VAL_99]], %[[VAL_109]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
-! CHECK:                   %[[VAL_111:.*]] = arith.constant 1 : index
-! CHECK:                   %[[VAL_112:.*]] = arith.subi %[[VAL_110]]#0, %[[VAL_111]] : index
-! CHECK:                   %[[VAL_113:.*]] = arith.addi %[[VAL_101]], %[[VAL_112]] : index
-! CHECK:                   %[[VAL_114:.*]] = hlfir.designate %[[VAL_99]] (%[[VAL_113]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
-! CHECK:                   %[[VAL_115:.*]] = fir.load %[[VAL_114]] : !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_107:.*]] = hlfir.designate %[[VAL_95]] (%[[VAL_106]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_108:.*]] = fir.load %[[VAL_107]] : !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_109:.*]] = arith.constant 0 : index
+! CHECK-DAG:               %[[VAL_110:.*]]:3 = fir.box_dims %[[VAL_99]], %[[VAL_109]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)
+! CHECK-DAG:               %[[VAL_111:.*]] = arith.constant 1 : index
+! CHECK-DAG:               %[[VAL_112:.*]] = arith.subi %[[VAL_110]]#0, %[[VAL_111]] : index
+! CHECK-DAG:               %[[VAL_113:.*]] = arith.addi %[[VAL_101]], %[[VAL_112]] : index
+! CHECK-DAG:               %[[VAL_114:.*]] = hlfir.designate %[[VAL_99]] (%[[VAL_113]])  : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> !fir.ref<i32>
+! CHECK-DAG:               %[[VAL_115:.*]] = fir.load %[[VAL_114]] : !fir.ref<i32>
 ! CHECK:                   %[[VAL_116:.*]] = arith.cmpi slt, %[[VAL_108]], %[[VAL_115]] : i32
 ! CHECK:                   %[[VAL_117:.*]] = arith.select %[[VAL_116]], %[[VAL_108]], %[[VAL_115]] : i32
 ! CHECK:                   hlfir.yield_element %[[VAL_117]] : i32
diff --git a/flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90 b/flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90
index d27804fb5606e..69219331ab3ab 100644
--- a/flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90
+++ b/flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90
@@ -53,11 +53,11 @@
 ! CHECK:                 %[[VAL_8:.*]]:2 = hlfir.declare %[[VAL_7]] {uniq_name = "_QFreduction_max_intEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 %[[VAL_14:.*]]:2 = hlfir.declare %[[VAL_12]] {uniq_name = "_QFreduction_max_intEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 hlfir.assi...
[truncated]

klausler
klausler reviewed Jun 16, 2025
View reviewed changes
jeanPerier
jeanPerier approved these changes Jun 17, 2025
View reviewed changes
Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jeanPerier
jeanPerier requested changes Jun 17, 2025
View reviewed changes
flang/test/Semantics/function-result-extent-max.f90
submodule (m) function_with_max_result_extent_submodule
implicit none
contains
pure module function function_with_max_result_extent(n) result(res)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this test is failing.

error: Semantic errors in /home/gha/actions-runner/_work/llvm-project/llvm-project/flang/test/Semantics/function-result-extent-max.f90
/home/gha/actions-runner/_work/llvm-project/llvm-project/flang/test/Semantics/function-result-extent-max.f90:26:24: error: 'function_with_max_result_extent' was not declared a separate module procedure
    pure module function function_with_max_result_extent(n) result(res)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeanPerier jeanPerier jeanPerier requested changes

@klausler klausler klausler approved these changes

@eugeneepshteyn eugeneepshteyn Awaiting requested review from eugeneepshteyn

Assignees
No one assigned
Labels
flang:fir-hlfir flang:openmp flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[flang] The function result res(max(n, 0)) has distinct extents when split across a module and its submodule
4 participants
@akuhlens @llvmbot @jeanPerier @klausler