Skip to content

[flang] Use integer arith.max/min operations for max/min lowering. - #186466

Merged
vzakhari merged 5 commits into
llvm:mainfrom
vzakhari:flang_int_maxmin
Mar 18, 2026
Merged

[flang] Use integer arith.max/min operations for max/min lowering.#186466
vzakhari merged 5 commits into
llvm:mainfrom
vzakhari:flang_int_maxmin

Conversation

@vzakhari

Copy link
Copy Markdown
Contributor

arith.maxsi/maxui/minsi/minui are more concise than cmp+select
and probably allow more folding, so we should use it in Flang lowering.

arith.maxsi/maxui/minsi/minui are more concise than cmp+select
and probably allow more folding, so we should use it in Flang lowering.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp openacc labels Mar 13, 2026
@llvmbot

llvmbot commented Mar 13, 2026

Copy link
Copy Markdown
Member

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-openacc

Author: Slava Zakharin (vzakhari)

Changes

arith.maxsi/maxui/minsi/minui are more concise than cmp+select
and probably allow more folding, so we should use it in Flang lowering.


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

21 Files Affected:

  • (modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+17-18)
  • (modified) flang/test/Lower/HLFIR/array-ctor-as-elemental.f90 (+1-2)
  • (modified) flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90 (+4-8)
  • (modified) flang/test/Lower/HLFIR/binary-ops.f90 (+1-2)
  • (modified) flang/test/Lower/HLFIR/custom-intrinsic.f90 (+20-40)
  • (modified) flang/test/Lower/Intrinsics/max.f90 (+10-20)
  • (modified) flang/test/Lower/OpenACC/acc-atomic-update-hlfir.f90 (+3-6)
  • (modified) flang/test/Lower/OpenACC/acc-reduction.f90 (+4-8)
  • (modified) flang/test/Lower/OpenMP/atomic-update.f90 (+2-4)
  • (modified) flang/test/Lower/OpenMP/minmax-optional-parameters.f90 (+3-6)
  • (modified) flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/reduction-array-intrinsic.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90 (+2-4)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-2-byref.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-2.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min-byref.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-multi.f90 (+1-1)
  • (modified) flang/test/Lower/array-elemental-calls-2.f90 (+1-2)
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index d67eebdd8c93c..f421bddebea8a 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -8736,12 +8736,6 @@ static mlir::Value genExtremumResult(mlir::Location loc,
                                      fir::FirOpBuilder &builder,
                                      mlir::Value left, mlir::Value right) {
   mlir::Type type = left.getType();
-  mlir::arith::CmpIPredicate integerPredicate =
-      type.isUnsignedInteger() ? isMax ? mlir::arith::CmpIPredicate::ugt
-                                       : mlir::arith::CmpIPredicate::ult
-      : isMax                  ? mlir::arith::CmpIPredicate::sgt
-                               : mlir::arith::CmpIPredicate::slt;
-  mlir::Value pred;
   if (fir::isa_real(type)) {
     switch (builder.getFPMaxminBehavior()) {
     case Fortran::common::FPMaxminBehavior::Portable:
@@ -8782,26 +8776,31 @@ static mlir::Value genExtremumResult(mlir::Location loc,
 
     llvm_unreachable("unsupported FPMaxminBehavior");
   } else if (fir::isa_integer(type)) {
-    mlir::Value cmpLeft = left;
-    mlir::Value cmpRight = right;
+    // It is probably okay to use signed index.maxs/mins, but
+    // maybe the caller needs to specify signedness.
+    // There are currently no callers that pass values of index
+    // type, so just emit a TODO.
+    if (mlir::isa<mlir::IndexType>(type))
+      TODO(loc, "extremum for index type");
+
     if (type.isUnsignedInteger()) {
-      mlir::Type signlessType = mlir::IntegerType::get(
-          builder.getContext(), type.getIntOrFloatBitWidth(),
-          mlir::IntegerType::SignednessSemantics::Signless);
-      cmpLeft = builder.createConvert(loc, signlessType, left);
-      cmpRight = builder.createConvert(loc, signlessType, right);
+      if constexpr (isMax)
+        return mlir::arith::MaxUIOp::create(builder, loc, left, right);
+      else
+        return mlir::arith::MinUIOp::create(builder, loc, left, right);
+    } else {
+      if constexpr (isMax)
+        return mlir::arith::MaxSIOp::create(builder, loc, left, right);
+      else
+        return mlir::arith::MinSIOp::create(builder, loc, left, right);
     }
-    pred = mlir::arith::CmpIOp::create(builder, loc, integerPredicate, cmpLeft,
-                                       cmpRight);
   } else if (fir::isa_char(type) || fir::isa_char(fir::unwrapRefType(type))) {
     // TODO: ! character min and max is tricky because the result
     // length is the length of the longest argument!
     // So we may need a temp.
     TODO(loc, "intrinsic: min and max for CHARACTER");
   }
-  assert(pred && "pred must be defined");
-
-  return mlir::arith::SelectOp::create(builder, loc, pred, left, right);
+  llvm_unreachable("unsupported extremum");
 }
 
 // UNLINK
diff --git a/flang/test/Lower/HLFIR/array-ctor-as-elemental.f90 b/flang/test/Lower/HLFIR/array-ctor-as-elemental.f90
index 5c4f079c86e20..f52216d2f558d 100644
--- a/flang/test/Lower/HLFIR/array-ctor-as-elemental.f90
+++ b/flang/test/Lower/HLFIR/array-ctor-as-elemental.f90
@@ -54,8 +54,7 @@ subroutine test_as_strided_elemental(lb, ub, stride)
 ! CHECK:           %[[VAL_12:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i64>
 ! CHECK:           %[[VAL_13:.*]] = arith.divsi %[[VAL_11]], %[[VAL_12]] : i64
 ! CHECK:           %[[VAL_14:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_15:.*]] = arith.cmpi sgt, %[[VAL_13]], %[[VAL_14]] : i64
-! CHECK:           %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_13]], %[[VAL_14]] : i64
+! CHECK:           %[[VAL_16:.*]] = arith.maxsi %[[VAL_13]], %[[VAL_14]] : i64
 ! CHECK:           %[[VAL_17:.*]] = arith.addi %[[VAL_6]], %[[VAL_16]] : i64
 ! CHECK:           %[[VAL_18:.*]] = fir.convert %[[VAL_17]] : (i64) -> index
 ! CHECK:           %[[VAL_19:.*]] = fir.shape %[[VAL_18]] : (index) -> !fir.shape<1>
diff --git a/flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90 b/flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90
index 415a9df5bf838..6fcd942f4a1e5 100644
--- a/flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90
+++ b/flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90
@@ -127,8 +127,7 @@ subroutine test_implied_do(n)
 ! CHECK:           %[[VAL_10:.*]] = arith.constant 1 : i64
 ! CHECK:           %[[VAL_11:.*]] = arith.divsi %[[VAL_9]], %[[VAL_10]] : i64
 ! CHECK:           %[[VAL_12:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_13:.*]] = arith.cmpi sgt, %[[VAL_11]], %[[VAL_12]] : i64
-! CHECK:           %[[VAL_14:.*]] = arith.select %[[VAL_13]], %[[VAL_11]], %[[VAL_12]] : i64
+! CHECK:           %[[VAL_14:.*]] = arith.maxsi %[[VAL_11]], %[[VAL_12]] : i64
 ! CHECK:           %[[VAL_15:.*]] = arith.muli %[[VAL_4]], %[[VAL_14]] : i64
 ! CHECK:           %[[VAL_16:.*]] = arith.addi %[[VAL_3]], %[[VAL_15]] : i64
 ! CHECK:           %[[VAL_17:.*]] = fir.convert %[[VAL_16]] : (i64) -> index
@@ -191,8 +190,7 @@ subroutine test_strided_implied_do(lb, ub, stride)
 ! CHECK:           %[[VAL_14:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i64>
 ! CHECK:           %[[VAL_15:.*]] = arith.divsi %[[VAL_13]], %[[VAL_14]] : i64
 ! CHECK:           %[[VAL_16:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_17:.*]] = arith.cmpi sgt, %[[VAL_15]], %[[VAL_16]] : i64
-! CHECK:           %[[VAL_18:.*]] = arith.select %[[VAL_17]], %[[VAL_15]], %[[VAL_16]] : i64
+! CHECK:           %[[VAL_18:.*]] = arith.maxsi %[[VAL_15]], %[[VAL_16]] : i64
 ! CHECK:           %[[VAL_19:.*]] = arith.muli %[[VAL_8]], %[[VAL_18]] : i64
 ! CHECK:           %[[VAL_20:.*]] = arith.addi %[[VAL_7]], %[[VAL_19]] : i64
 ! CHECK:           %[[VAL_21:.*]] = fir.convert %[[VAL_20]] : (i64) -> index
@@ -253,8 +251,7 @@ subroutine test_nested_implied_do(n, m)
 ! CHECK:           %[[VAL_12:.*]] = arith.constant 1 : i64
 ! CHECK:           %[[VAL_13:.*]] = arith.divsi %[[VAL_11]], %[[VAL_12]] : i64
 ! CHECK:           %[[VAL_14:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_15:.*]] = arith.cmpi sgt, %[[VAL_13]], %[[VAL_14]] : i64
-! CHECK:           %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_13]], %[[VAL_14]] : i64
+! CHECK:           %[[VAL_16:.*]] = arith.maxsi %[[VAL_13]], %[[VAL_14]] : i64
 ! CHECK:           %[[VAL_17:.*]] = arith.addi %[[VAL_6]], %[[VAL_16]] : i64
 ! CHECK:           %[[VAL_18:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i64>
 ! CHECK:           %[[VAL_19:.*]] = arith.constant 1 : i64
@@ -264,8 +261,7 @@ subroutine test_nested_implied_do(n, m)
 ! CHECK:           %[[VAL_23:.*]] = arith.constant 1 : i64
 ! CHECK:           %[[VAL_24:.*]] = arith.divsi %[[VAL_22]], %[[VAL_23]] : i64
 ! CHECK:           %[[VAL_25:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_26:.*]] = arith.cmpi sgt, %[[VAL_24]], %[[VAL_25]] : i64
-! CHECK:           %[[VAL_27:.*]] = arith.select %[[VAL_26]], %[[VAL_24]], %[[VAL_25]] : i64
+! CHECK:           %[[VAL_27:.*]] = arith.maxsi %[[VAL_24]], %[[VAL_25]] : i64
 ! CHECK:           %[[VAL_28:.*]] = arith.muli %[[VAL_17]], %[[VAL_27]] : i64
 ! CHECK:           %[[VAL_29:.*]] = arith.addi %[[VAL_5]], %[[VAL_28]] : i64
 ! CHECK:           %[[VAL_30:.*]] = fir.convert %[[VAL_29]] : (i64) -> index
diff --git a/flang/test/Lower/HLFIR/binary-ops.f90 b/flang/test/Lower/HLFIR/binary-ops.f90
index f4e1643dd78b6..3a83cd44d3c2e 100644
--- a/flang/test/Lower/HLFIR/binary-ops.f90
+++ b/flang/test/Lower/HLFIR/binary-ops.f90
@@ -206,8 +206,7 @@ subroutine extremum(c, n, l)
 ! CHECK:  hlfir.declare {{.*}}c"}
 ! CHECK:  %[[VAL_11:.*]] = arith.constant 0 : i64
 ! CHECK:  %[[VAL_12:.*]] = fir.load %{{.*}} : !fir.ref<i64>
-! CHECK:  %[[VAL_13:.*]] = arith.cmpi sgt, %[[VAL_11]], %[[VAL_12]] : i64
-! CHECK:  arith.select %[[VAL_13]], %[[VAL_11]], %[[VAL_12]] : i64
+! CHECK:  arith.maxsi %[[VAL_11]], %[[VAL_12]] : i64
 
 subroutine cmp_int(l, x, y)
   logical :: l
diff --git a/flang/test/Lower/HLFIR/custom-intrinsic.f90 b/flang/test/Lower/HLFIR/custom-intrinsic.f90
index 4999eebf376e7..294077105ed7b 100644
--- a/flang/test/Lower/HLFIR/custom-intrinsic.f90
+++ b/flang/test/Lower/HLFIR/custom-intrinsic.f90
@@ -14,8 +14,7 @@ function max_simple(a, b)
 ! CHECK-NEXT:    %[[RES_DECL:.*]]:2 = hlfir.declare %[[RES_ALLOC]] {uniq_name = "_QFmax_simpleEmax_simple"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK-NEXT:    %[[A_LD:.*]] = fir.load %[[A_DECL]]#0 : !fir.ref<i32>
 ! CHECK-NEXT:    %[[B_LD:.*]] = fir.load %[[B_DECL]]#0 : !fir.ref<i32>
-! CHECK-NEXT:    %[[A_GT_B:.*]] = arith.cmpi sgt, %[[A_LD]], %[[B_LD]] : i32
-! CHECK-NEXT:    %[[SELECT:.*]] = arith.select %[[A_GT_B]], %[[A_LD]], %[[B_LD]] : i32
+! CHECK-NEXT:    %[[SELECT:.*]] = arith.maxsi %[[A_LD]], %[[B_LD]] : i32
 ! CHECK-NEXT:    hlfir.assign %[[SELECT]] to %[[RES_DECL]]#0 : i32, !fir.ref<i32>
 ! CHECK-NEXT:    %[[RES_LD:.*]] = fir.load %[[RES_DECL]]#0 : !fir.ref<i32>
 ! CHECK-NEXT:    return %[[RES_LD]] : i32
@@ -38,12 +37,10 @@ function max_dynamic_optional_scalar(a, b, c)
 ! CHECK:           %[[VAL_8:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_9:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_10:.*]] = fir.is_present %[[VAL_5]]#0 : (!fir.ref<i32>) -> i1
-! CHECK:           %[[VAL_11:.*]] = arith.cmpi sgt, %[[VAL_8]], %[[VAL_9]] : i32
-! CHECK:           %[[VAL_12:.*]] = arith.select %[[VAL_11]], %[[VAL_8]], %[[VAL_9]] : i32
+! CHECK:           %[[VAL_12:.*]] = arith.maxsi %[[VAL_8]], %[[VAL_9]] : i32
 ! CHECK:           %[[VAL_13:.*]] = fir.if %[[VAL_10]] -> (i32) {
 ! CHECK:             %[[VAL_14:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
-! CHECK:             %[[VAL_15:.*]] = arith.cmpi sgt, %[[VAL_12]], %[[VAL_14]] : i32
-! CHECK:             %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_12]], %[[VAL_14]] : i32
+! CHECK:             %[[VAL_16:.*]] = arith.maxsi %[[VAL_12]], %[[VAL_14]] : i32
 ! CHECK:             fir.result %[[VAL_16]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_12]] : i32
@@ -73,20 +70,17 @@ function max_dynamic_optional_scalar2(a, b, c, d)
 ! CHECK:           %[[VAL_11:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_12:.*]] = fir.is_present %[[VAL_6]]#0 : (!fir.ref<i32>) -> i1
 ! CHECK:           %[[VAL_13:.*]] = fir.is_present %[[VAL_7]]#0 : (!fir.ref<i32>) -> i1
-! CHECK:           %[[VAL_14:.*]] = arith.cmpi sgt, %[[VAL_10]], %[[VAL_11]] : i32
-! CHECK:           %[[VAL_15:.*]] = arith.select %[[VAL_14]], %[[VAL_10]], %[[VAL_11]] : i32
+! CHECK:           %[[VAL_15:.*]] = arith.maxsi %[[VAL_10]], %[[VAL_11]] : i32
 ! CHECK:           %[[VAL_16:.*]] = fir.if %[[VAL_12]] -> (i32) {
 ! CHECK:             %[[VAL_17:.*]] = fir.load %[[VAL_6]]#0 : !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:             %[[VAL_19:.*]] = arith.maxsi %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             fir.result %[[VAL_19]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_15]] : i32
 ! CHECK:           }
 ! CHECK:           %[[VAL_20:.*]] = fir.if %[[VAL_13]] -> (i32) {
 ! CHECK:             %[[VAL_21:.*]] = fir.load %[[VAL_7]]#0 : !fir.ref<i32>
-! CHECK:             %[[VAL_22:.*]] = arith.cmpi sgt, %[[VAL_23:.*]], %[[VAL_21]] : i32
-! CHECK:             %[[VAL_24:.*]] = arith.select %[[VAL_22]], %[[VAL_23]], %[[VAL_21]] : i32
+! CHECK:             %[[VAL_24:.*]] = arith.maxsi %[[VAL_23:.*]], %[[VAL_21]] : i32
 ! CHECK:             fir.result %[[VAL_24]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_25:.*]] : i32
@@ -119,8 +113,7 @@ function max_array(a, b)
 ! 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:             %[[VAL_19:.*]] = arith.maxsi %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             hlfir.yield_element %[[VAL_19]] : i32
 ! CHECK:           }
 ! CHECK:           hlfir.assign %[[VAL_20:.*]] to %[[VAL_11]]#0 : !hlfir.expr<42xi32>, !fir.ref<!fir.array<42xi32>>
@@ -155,13 +148,11 @@ function max_dynamic_optional_array(a, b, c)
 ! CHECK:           ^bb0(%[[VAL_17:.*]]: index):
 ! CHECK:             %[[VAL_18:.*]] = hlfir.designate %[[VAL_6]]#0 (%[[VAL_17]])  : (!fir.ref<!fir.array<10xi32>>, index) -> !fir.ref<i32>
 ! CHECK:             %[[VAL_19:.*]] = fir.load %[[VAL_18]] : !fir.ref<i32>
-! CHECK:             %[[VAL_20:.*]] = arith.cmpi sgt, %[[VAL_15]], %[[VAL_19]] : i32
-! CHECK:             %[[VAL_21:.*]] = arith.select %[[VAL_20]], %[[VAL_15]], %[[VAL_19]] : i32
+! CHECK:             %[[VAL_21:.*]] = arith.maxsi %[[VAL_15]], %[[VAL_19]] : i32
 ! CHECK:             %[[VAL_22:.*]] = fir.if %[[VAL_14]] -> (i32) {
 ! CHECK:               %[[VAL_23:.*]] = hlfir.designate %[[VAL_9]]#0 (%[[VAL_17]])  : (!fir.ref<!fir.array<10xi32>>, index) -> !fir.ref<i32>
 ! CHECK:               %[[VAL_24:.*]] = fir.load %[[VAL_23]] : !fir.ref<i32>
-! CHECK:               %[[VAL_25:.*]] = arith.cmpi sgt, %[[VAL_21]], %[[VAL_24]] : i32
-! CHECK:               %[[VAL_26:.*]] = arith.select %[[VAL_25]], %[[VAL_21]], %[[VAL_24]] : i32
+! CHECK:               %[[VAL_26:.*]] = arith.maxsi %[[VAL_21]], %[[VAL_24]] : i32
 ! CHECK:               fir.result %[[VAL_26]] : i32
 ! CHECK:             } else {
 ! CHECK:               fir.result %[[VAL_21]] : i32
@@ -187,8 +178,7 @@ function min_simple(a, b)
 ! CHECK:           %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_4]] {uniq_name = "_QFmin_simpleEmin_simple"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:           %[[VAL_6:.*]] = fir.load %[[VAL_2]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_7:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
-! CHECK:           %[[VAL_8:.*]] = arith.cmpi slt, %[[VAL_6]], %[[VAL_7]] : i32
-! CHECK:           %[[VAL_9:.*]] = arith.select %[[VAL_8]], %[[VAL_6]], %[[VAL_7]] : i32
+! CHECK:           %[[VAL_9:.*]] = arith.minsi %[[VAL_6]], %[[VAL_7]] : i32
 ! CHECK:           hlfir.assign %[[VAL_9]] to %[[VAL_5]]#0 : i32, !fir.ref<i32>
 ! CHECK:           %[[VAL_10:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
 ! CHECK:           return %[[VAL_10]] : i32
@@ -211,12 +201,10 @@ function min_dynamic_optional_scalar(a, b, c)
 ! CHECK:           %[[VAL_8:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_9:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_10:.*]] = fir.is_present %[[VAL_5]]#0 : (!fir.ref<i32>) -> i1
-! CHECK:           %[[VAL_11:.*]] = arith.cmpi slt, %[[VAL_8]], %[[VAL_9]] : i32
-! CHECK:           %[[VAL_12:.*]] = arith.select %[[VAL_11]], %[[VAL_8]], %[[VAL_9]] : i32
+! CHECK:           %[[VAL_12:.*]] = arith.minsi %[[VAL_8]], %[[VAL_9]] : i32
 ! CHECK:           %[[VAL_13:.*]] = fir.if %[[VAL_10]] -> (i32) {
 ! CHECK:             %[[VAL_14:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
-! CHECK:             %[[VAL_15:.*]] = arith.cmpi slt, %[[VAL_12]], %[[VAL_14]] : i32
-! CHECK:             %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_12]], %[[VAL_14]] : i32
+! CHECK:             %[[VAL_16:.*]] = arith.minsi %[[VAL_12]], %[[VAL_14]] : i32
 ! CHECK:             fir.result %[[VAL_16]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_12]] : i32
@@ -246,20 +234,17 @@ function min_dynamic_optional_scalar2(a, b, c, d)
 ! CHECK:           %[[VAL_11:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_12:.*]] = fir.is_present %[[VAL_6]]#0 : (!fir.ref<i32>) -> i1
 ! CHECK:           %[[VAL_13:.*]] = fir.is_present %[[VAL_7]]#0 : (!fir.ref<i32>) -> i1
-! CHECK:           %[[VAL_14:.*]] = arith.cmpi slt, %[[VAL_10]], %[[VAL_11]] : i32
-! CHECK:           %[[VAL_15:.*]] = arith.select %[[VAL_14]], %[[VAL_10]], %[[VAL_11]] : i32
+! CHECK:           %[[VAL_15:.*]] = arith.minsi %[[VAL_10]], %[[VAL_11]] : i32
 ! CHECK:           %[[VAL_16:.*]] = fir.if %[[VAL_12]] -> (i32) {
 ! CHECK:             %[[VAL_17:.*]] = fir.load %[[VAL_6]]#0 : !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:             %[[VAL_19:.*]] = arith.minsi %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             fir.result %[[VAL_19]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_15]] : i32
 ! CHECK:           }
 ! CHECK:           %[[VAL_20:.*]] = fir.if %[[VAL_13]] -> (i32) {
 ! CHECK:             %[[VAL_21:.*]] = fir.load %[[VAL_7]]#0 : !fir.ref<i32>
-! CHECK:             %[[VAL_22:.*]] = arith.cmpi slt, %[[VAL_23:.*]], %[[VAL_21]] : i32
-! CHECK:             %[[VAL_24:.*]] = arith.select %[[VAL_22]], %[[VAL_23]], %[[VAL_21]] : i32
+! CHECK:             %[[VAL_24:.*]] = arith.minsi %[[VAL_23:.*]], %[[VAL_21]] : i32
 ! CHECK:             fir.result %[[VAL_24]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_25:.*]] : i32
@@ -292,8 +277,7 @@ function min_array(a, b)
 ! 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:             %[[VAL_19:.*]] = arith.minsi %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             hlfir.yield_element %[[VAL_19]] : i32
 ! CHECK:           }
 ! CHECK:           hlfir.assign %[[VAL_20:.*]] to %[[VAL_11]]#0 : !hlfir.expr<42xi32>, !fir.ref<!fir.array<42xi32>>
@@ -328,13 +312,11 @@ function min_dynamic_optional_array(a, b, c)
 ! CHECK:           ^bb0(%[[VAL_17:.*]]: index):
 ! CHECK:             %[[VAL_18:.*]] = hlfir.designate %[[VAL_6]]#0 (%[[VAL_17]])  : (!fir.ref<!fir.array<10xi32>>, index) -> !fir.ref<i32>
 ! CHECK:             %[[VAL_19:.*]] = fir.load %[[VAL_18]] : !fir.ref<i32>
-! CHECK:             %[[VAL_20:.*]] = arith.cmpi slt, %[[VAL_15]], %[[VAL_19]] : i32
-! CHECK:             %[[VAL_21:.*]] = arith.select %[[VAL_20]], %[[VAL_15]], %[[VAL_19]] : i32
+! CHECK:             %[[VAL_21:.*]] = arith.minsi %[[VAL_15]], %[[VAL_19]] : i32
 ! CHECK:             %[[VAL_22:.*]] = fir.if %[[VAL_14]] -> (i32) {
 ! CHECK:               %[[VAL_23:.*]] = hlfir.designate %[[VAL_9]]#0 (%[[VAL_17]])  : (!fir.ref<!fir.array<10xi32>>, index) -> !fir.ref<i32>
 ! CHECK:               %[[VAL_24:.*]] = fir.load %[[VAL_23]] : !fir.ref<i32>
-! CHECK:               %[[VAL_25:.*]] = arith.cmpi slt, %[[VAL_21]], %[[VAL_24]] : i32
-! CHECK:               %[[VAL_26:.*]] = arith.select %[[VAL_25]], %[[VAL_21]], %[[VAL_24]] : i32
+! CHECK:               %[[VAL_26:.*]] = arith.minsi %[[VAL_21]], %[[VAL_24]] : i32
 ! CHECK:               fir.result %[[VAL_26]] : i32
 ! CHECK:             } else {
 ! CHECK:               fir.result %[[VAL_21]] : i32
@@ -812,8 +794,7 @@ subroutine allocatables_test(a, b, c)
 ! CHECK:             %[[VAL_112:.*]] = arith.addi %[[VAL_84]], %[[VAL_111]] : index
 ! CHECK:             %[[VAL_113:.*]] = hlfir.designate %[[VAL_79]] (%[[VAL_108]], %[[VAL_110]], %[[VAL_112]])  : (!fir.box<!fir.heap<!fir.array<?x?x?xi32>>>, index, index, index) -> !fir.ref<i32>
 ! CHECK:             %[[VAL_114:.*]] = fir.load %[[VAL_113]] : !fir.ref<i32>
-! CHECK:             %[[VAL_115:.*]] = arith.cmpi slt, %[[VAL_99]], %[[VAL_114]] : i32
-! CHECK:             %[[VAL_116:.*]] = arith.select %[[VAL_115]], %[[VAL_99]], %[[VAL_114]] : i32
+! CHECK:             %[[VAL_116:.*]] = arith.minsi %[[VAL_99]], %[[VAL_114]] : i32
 ! CHECK:             %[[VAL_117:.*]] = fir.if %[[VAL_70]] -> (i32) {
 ! CHECK:               %[[VAL_118:.*]] = arith.constant 0 : index
 ! CHECK:               %[[VAL_119:.*]]:3 = fir.box_dims %[[VAL_80]], %[[VAL_118]] : (!fir.box<!fir.heap<!fir.array<?x?x?xi32>>>, inde...
[truncated]

@llvmbot

llvmbot commented Mar 13, 2026

Copy link
Copy Markdown
Member

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

Author: Slava Zakharin (vzakhari)

Changes

arith.maxsi/maxui/minsi/minui are more concise than cmp+select
and probably allow more folding, so we should use it in Flang lowering.


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

21 Files Affected:

  • (modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+17-18)
  • (modified) flang/test/Lower/HLFIR/array-ctor-as-elemental.f90 (+1-2)
  • (modified) flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90 (+4-8)
  • (modified) flang/test/Lower/HLFIR/binary-ops.f90 (+1-2)
  • (modified) flang/test/Lower/HLFIR/custom-intrinsic.f90 (+20-40)
  • (modified) flang/test/Lower/Intrinsics/max.f90 (+10-20)
  • (modified) flang/test/Lower/OpenACC/acc-atomic-update-hlfir.f90 (+3-6)
  • (modified) flang/test/Lower/OpenACC/acc-reduction.f90 (+4-8)
  • (modified) flang/test/Lower/OpenMP/atomic-update.f90 (+2-4)
  • (modified) flang/test/Lower/OpenMP/minmax-optional-parameters.f90 (+3-6)
  • (modified) flang/test/Lower/OpenMP/omp-declare-reduction-derivedtype.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/reduction-array-intrinsic.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90 (+2-4)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-2-byref.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-2.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min-byref.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min.f90 (+1-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-multi.f90 (+1-1)
  • (modified) flang/test/Lower/array-elemental-calls-2.f90 (+1-2)
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index d67eebdd8c93c..f421bddebea8a 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -8736,12 +8736,6 @@ static mlir::Value genExtremumResult(mlir::Location loc,
                                      fir::FirOpBuilder &builder,
                                      mlir::Value left, mlir::Value right) {
   mlir::Type type = left.getType();
-  mlir::arith::CmpIPredicate integerPredicate =
-      type.isUnsignedInteger() ? isMax ? mlir::arith::CmpIPredicate::ugt
-                                       : mlir::arith::CmpIPredicate::ult
-      : isMax                  ? mlir::arith::CmpIPredicate::sgt
-                               : mlir::arith::CmpIPredicate::slt;
-  mlir::Value pred;
   if (fir::isa_real(type)) {
     switch (builder.getFPMaxminBehavior()) {
     case Fortran::common::FPMaxminBehavior::Portable:
@@ -8782,26 +8776,31 @@ static mlir::Value genExtremumResult(mlir::Location loc,
 
     llvm_unreachable("unsupported FPMaxminBehavior");
   } else if (fir::isa_integer(type)) {
-    mlir::Value cmpLeft = left;
-    mlir::Value cmpRight = right;
+    // It is probably okay to use signed index.maxs/mins, but
+    // maybe the caller needs to specify signedness.
+    // There are currently no callers that pass values of index
+    // type, so just emit a TODO.
+    if (mlir::isa<mlir::IndexType>(type))
+      TODO(loc, "extremum for index type");
+
     if (type.isUnsignedInteger()) {
-      mlir::Type signlessType = mlir::IntegerType::get(
-          builder.getContext(), type.getIntOrFloatBitWidth(),
-          mlir::IntegerType::SignednessSemantics::Signless);
-      cmpLeft = builder.createConvert(loc, signlessType, left);
-      cmpRight = builder.createConvert(loc, signlessType, right);
+      if constexpr (isMax)
+        return mlir::arith::MaxUIOp::create(builder, loc, left, right);
+      else
+        return mlir::arith::MinUIOp::create(builder, loc, left, right);
+    } else {
+      if constexpr (isMax)
+        return mlir::arith::MaxSIOp::create(builder, loc, left, right);
+      else
+        return mlir::arith::MinSIOp::create(builder, loc, left, right);
     }
-    pred = mlir::arith::CmpIOp::create(builder, loc, integerPredicate, cmpLeft,
-                                       cmpRight);
   } else if (fir::isa_char(type) || fir::isa_char(fir::unwrapRefType(type))) {
     // TODO: ! character min and max is tricky because the result
     // length is the length of the longest argument!
     // So we may need a temp.
     TODO(loc, "intrinsic: min and max for CHARACTER");
   }
-  assert(pred && "pred must be defined");
-
-  return mlir::arith::SelectOp::create(builder, loc, pred, left, right);
+  llvm_unreachable("unsupported extremum");
 }
 
 // UNLINK
diff --git a/flang/test/Lower/HLFIR/array-ctor-as-elemental.f90 b/flang/test/Lower/HLFIR/array-ctor-as-elemental.f90
index 5c4f079c86e20..f52216d2f558d 100644
--- a/flang/test/Lower/HLFIR/array-ctor-as-elemental.f90
+++ b/flang/test/Lower/HLFIR/array-ctor-as-elemental.f90
@@ -54,8 +54,7 @@ subroutine test_as_strided_elemental(lb, ub, stride)
 ! CHECK:           %[[VAL_12:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i64>
 ! CHECK:           %[[VAL_13:.*]] = arith.divsi %[[VAL_11]], %[[VAL_12]] : i64
 ! CHECK:           %[[VAL_14:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_15:.*]] = arith.cmpi sgt, %[[VAL_13]], %[[VAL_14]] : i64
-! CHECK:           %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_13]], %[[VAL_14]] : i64
+! CHECK:           %[[VAL_16:.*]] = arith.maxsi %[[VAL_13]], %[[VAL_14]] : i64
 ! CHECK:           %[[VAL_17:.*]] = arith.addi %[[VAL_6]], %[[VAL_16]] : i64
 ! CHECK:           %[[VAL_18:.*]] = fir.convert %[[VAL_17]] : (i64) -> index
 ! CHECK:           %[[VAL_19:.*]] = fir.shape %[[VAL_18]] : (index) -> !fir.shape<1>
diff --git a/flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90 b/flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90
index 415a9df5bf838..6fcd942f4a1e5 100644
--- a/flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90
+++ b/flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90
@@ -127,8 +127,7 @@ subroutine test_implied_do(n)
 ! CHECK:           %[[VAL_10:.*]] = arith.constant 1 : i64
 ! CHECK:           %[[VAL_11:.*]] = arith.divsi %[[VAL_9]], %[[VAL_10]] : i64
 ! CHECK:           %[[VAL_12:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_13:.*]] = arith.cmpi sgt, %[[VAL_11]], %[[VAL_12]] : i64
-! CHECK:           %[[VAL_14:.*]] = arith.select %[[VAL_13]], %[[VAL_11]], %[[VAL_12]] : i64
+! CHECK:           %[[VAL_14:.*]] = arith.maxsi %[[VAL_11]], %[[VAL_12]] : i64
 ! CHECK:           %[[VAL_15:.*]] = arith.muli %[[VAL_4]], %[[VAL_14]] : i64
 ! CHECK:           %[[VAL_16:.*]] = arith.addi %[[VAL_3]], %[[VAL_15]] : i64
 ! CHECK:           %[[VAL_17:.*]] = fir.convert %[[VAL_16]] : (i64) -> index
@@ -191,8 +190,7 @@ subroutine test_strided_implied_do(lb, ub, stride)
 ! CHECK:           %[[VAL_14:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i64>
 ! CHECK:           %[[VAL_15:.*]] = arith.divsi %[[VAL_13]], %[[VAL_14]] : i64
 ! CHECK:           %[[VAL_16:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_17:.*]] = arith.cmpi sgt, %[[VAL_15]], %[[VAL_16]] : i64
-! CHECK:           %[[VAL_18:.*]] = arith.select %[[VAL_17]], %[[VAL_15]], %[[VAL_16]] : i64
+! CHECK:           %[[VAL_18:.*]] = arith.maxsi %[[VAL_15]], %[[VAL_16]] : i64
 ! CHECK:           %[[VAL_19:.*]] = arith.muli %[[VAL_8]], %[[VAL_18]] : i64
 ! CHECK:           %[[VAL_20:.*]] = arith.addi %[[VAL_7]], %[[VAL_19]] : i64
 ! CHECK:           %[[VAL_21:.*]] = fir.convert %[[VAL_20]] : (i64) -> index
@@ -253,8 +251,7 @@ subroutine test_nested_implied_do(n, m)
 ! CHECK:           %[[VAL_12:.*]] = arith.constant 1 : i64
 ! CHECK:           %[[VAL_13:.*]] = arith.divsi %[[VAL_11]], %[[VAL_12]] : i64
 ! CHECK:           %[[VAL_14:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_15:.*]] = arith.cmpi sgt, %[[VAL_13]], %[[VAL_14]] : i64
-! CHECK:           %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_13]], %[[VAL_14]] : i64
+! CHECK:           %[[VAL_16:.*]] = arith.maxsi %[[VAL_13]], %[[VAL_14]] : i64
 ! CHECK:           %[[VAL_17:.*]] = arith.addi %[[VAL_6]], %[[VAL_16]] : i64
 ! CHECK:           %[[VAL_18:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i64>
 ! CHECK:           %[[VAL_19:.*]] = arith.constant 1 : i64
@@ -264,8 +261,7 @@ subroutine test_nested_implied_do(n, m)
 ! CHECK:           %[[VAL_23:.*]] = arith.constant 1 : i64
 ! CHECK:           %[[VAL_24:.*]] = arith.divsi %[[VAL_22]], %[[VAL_23]] : i64
 ! CHECK:           %[[VAL_25:.*]] = arith.constant 0 : i64
-! CHECK:           %[[VAL_26:.*]] = arith.cmpi sgt, %[[VAL_24]], %[[VAL_25]] : i64
-! CHECK:           %[[VAL_27:.*]] = arith.select %[[VAL_26]], %[[VAL_24]], %[[VAL_25]] : i64
+! CHECK:           %[[VAL_27:.*]] = arith.maxsi %[[VAL_24]], %[[VAL_25]] : i64
 ! CHECK:           %[[VAL_28:.*]] = arith.muli %[[VAL_17]], %[[VAL_27]] : i64
 ! CHECK:           %[[VAL_29:.*]] = arith.addi %[[VAL_5]], %[[VAL_28]] : i64
 ! CHECK:           %[[VAL_30:.*]] = fir.convert %[[VAL_29]] : (i64) -> index
diff --git a/flang/test/Lower/HLFIR/binary-ops.f90 b/flang/test/Lower/HLFIR/binary-ops.f90
index f4e1643dd78b6..3a83cd44d3c2e 100644
--- a/flang/test/Lower/HLFIR/binary-ops.f90
+++ b/flang/test/Lower/HLFIR/binary-ops.f90
@@ -206,8 +206,7 @@ subroutine extremum(c, n, l)
 ! CHECK:  hlfir.declare {{.*}}c"}
 ! CHECK:  %[[VAL_11:.*]] = arith.constant 0 : i64
 ! CHECK:  %[[VAL_12:.*]] = fir.load %{{.*}} : !fir.ref<i64>
-! CHECK:  %[[VAL_13:.*]] = arith.cmpi sgt, %[[VAL_11]], %[[VAL_12]] : i64
-! CHECK:  arith.select %[[VAL_13]], %[[VAL_11]], %[[VAL_12]] : i64
+! CHECK:  arith.maxsi %[[VAL_11]], %[[VAL_12]] : i64
 
 subroutine cmp_int(l, x, y)
   logical :: l
diff --git a/flang/test/Lower/HLFIR/custom-intrinsic.f90 b/flang/test/Lower/HLFIR/custom-intrinsic.f90
index 4999eebf376e7..294077105ed7b 100644
--- a/flang/test/Lower/HLFIR/custom-intrinsic.f90
+++ b/flang/test/Lower/HLFIR/custom-intrinsic.f90
@@ -14,8 +14,7 @@ function max_simple(a, b)
 ! CHECK-NEXT:    %[[RES_DECL:.*]]:2 = hlfir.declare %[[RES_ALLOC]] {uniq_name = "_QFmax_simpleEmax_simple"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK-NEXT:    %[[A_LD:.*]] = fir.load %[[A_DECL]]#0 : !fir.ref<i32>
 ! CHECK-NEXT:    %[[B_LD:.*]] = fir.load %[[B_DECL]]#0 : !fir.ref<i32>
-! CHECK-NEXT:    %[[A_GT_B:.*]] = arith.cmpi sgt, %[[A_LD]], %[[B_LD]] : i32
-! CHECK-NEXT:    %[[SELECT:.*]] = arith.select %[[A_GT_B]], %[[A_LD]], %[[B_LD]] : i32
+! CHECK-NEXT:    %[[SELECT:.*]] = arith.maxsi %[[A_LD]], %[[B_LD]] : i32
 ! CHECK-NEXT:    hlfir.assign %[[SELECT]] to %[[RES_DECL]]#0 : i32, !fir.ref<i32>
 ! CHECK-NEXT:    %[[RES_LD:.*]] = fir.load %[[RES_DECL]]#0 : !fir.ref<i32>
 ! CHECK-NEXT:    return %[[RES_LD]] : i32
@@ -38,12 +37,10 @@ function max_dynamic_optional_scalar(a, b, c)
 ! CHECK:           %[[VAL_8:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_9:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_10:.*]] = fir.is_present %[[VAL_5]]#0 : (!fir.ref<i32>) -> i1
-! CHECK:           %[[VAL_11:.*]] = arith.cmpi sgt, %[[VAL_8]], %[[VAL_9]] : i32
-! CHECK:           %[[VAL_12:.*]] = arith.select %[[VAL_11]], %[[VAL_8]], %[[VAL_9]] : i32
+! CHECK:           %[[VAL_12:.*]] = arith.maxsi %[[VAL_8]], %[[VAL_9]] : i32
 ! CHECK:           %[[VAL_13:.*]] = fir.if %[[VAL_10]] -> (i32) {
 ! CHECK:             %[[VAL_14:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
-! CHECK:             %[[VAL_15:.*]] = arith.cmpi sgt, %[[VAL_12]], %[[VAL_14]] : i32
-! CHECK:             %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_12]], %[[VAL_14]] : i32
+! CHECK:             %[[VAL_16:.*]] = arith.maxsi %[[VAL_12]], %[[VAL_14]] : i32
 ! CHECK:             fir.result %[[VAL_16]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_12]] : i32
@@ -73,20 +70,17 @@ function max_dynamic_optional_scalar2(a, b, c, d)
 ! CHECK:           %[[VAL_11:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_12:.*]] = fir.is_present %[[VAL_6]]#0 : (!fir.ref<i32>) -> i1
 ! CHECK:           %[[VAL_13:.*]] = fir.is_present %[[VAL_7]]#0 : (!fir.ref<i32>) -> i1
-! CHECK:           %[[VAL_14:.*]] = arith.cmpi sgt, %[[VAL_10]], %[[VAL_11]] : i32
-! CHECK:           %[[VAL_15:.*]] = arith.select %[[VAL_14]], %[[VAL_10]], %[[VAL_11]] : i32
+! CHECK:           %[[VAL_15:.*]] = arith.maxsi %[[VAL_10]], %[[VAL_11]] : i32
 ! CHECK:           %[[VAL_16:.*]] = fir.if %[[VAL_12]] -> (i32) {
 ! CHECK:             %[[VAL_17:.*]] = fir.load %[[VAL_6]]#0 : !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:             %[[VAL_19:.*]] = arith.maxsi %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             fir.result %[[VAL_19]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_15]] : i32
 ! CHECK:           }
 ! CHECK:           %[[VAL_20:.*]] = fir.if %[[VAL_13]] -> (i32) {
 ! CHECK:             %[[VAL_21:.*]] = fir.load %[[VAL_7]]#0 : !fir.ref<i32>
-! CHECK:             %[[VAL_22:.*]] = arith.cmpi sgt, %[[VAL_23:.*]], %[[VAL_21]] : i32
-! CHECK:             %[[VAL_24:.*]] = arith.select %[[VAL_22]], %[[VAL_23]], %[[VAL_21]] : i32
+! CHECK:             %[[VAL_24:.*]] = arith.maxsi %[[VAL_23:.*]], %[[VAL_21]] : i32
 ! CHECK:             fir.result %[[VAL_24]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_25:.*]] : i32
@@ -119,8 +113,7 @@ function max_array(a, b)
 ! 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:             %[[VAL_19:.*]] = arith.maxsi %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             hlfir.yield_element %[[VAL_19]] : i32
 ! CHECK:           }
 ! CHECK:           hlfir.assign %[[VAL_20:.*]] to %[[VAL_11]]#0 : !hlfir.expr<42xi32>, !fir.ref<!fir.array<42xi32>>
@@ -155,13 +148,11 @@ function max_dynamic_optional_array(a, b, c)
 ! CHECK:           ^bb0(%[[VAL_17:.*]]: index):
 ! CHECK:             %[[VAL_18:.*]] = hlfir.designate %[[VAL_6]]#0 (%[[VAL_17]])  : (!fir.ref<!fir.array<10xi32>>, index) -> !fir.ref<i32>
 ! CHECK:             %[[VAL_19:.*]] = fir.load %[[VAL_18]] : !fir.ref<i32>
-! CHECK:             %[[VAL_20:.*]] = arith.cmpi sgt, %[[VAL_15]], %[[VAL_19]] : i32
-! CHECK:             %[[VAL_21:.*]] = arith.select %[[VAL_20]], %[[VAL_15]], %[[VAL_19]] : i32
+! CHECK:             %[[VAL_21:.*]] = arith.maxsi %[[VAL_15]], %[[VAL_19]] : i32
 ! CHECK:             %[[VAL_22:.*]] = fir.if %[[VAL_14]] -> (i32) {
 ! CHECK:               %[[VAL_23:.*]] = hlfir.designate %[[VAL_9]]#0 (%[[VAL_17]])  : (!fir.ref<!fir.array<10xi32>>, index) -> !fir.ref<i32>
 ! CHECK:               %[[VAL_24:.*]] = fir.load %[[VAL_23]] : !fir.ref<i32>
-! CHECK:               %[[VAL_25:.*]] = arith.cmpi sgt, %[[VAL_21]], %[[VAL_24]] : i32
-! CHECK:               %[[VAL_26:.*]] = arith.select %[[VAL_25]], %[[VAL_21]], %[[VAL_24]] : i32
+! CHECK:               %[[VAL_26:.*]] = arith.maxsi %[[VAL_21]], %[[VAL_24]] : i32
 ! CHECK:               fir.result %[[VAL_26]] : i32
 ! CHECK:             } else {
 ! CHECK:               fir.result %[[VAL_21]] : i32
@@ -187,8 +178,7 @@ function min_simple(a, b)
 ! CHECK:           %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_4]] {uniq_name = "_QFmin_simpleEmin_simple"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:           %[[VAL_6:.*]] = fir.load %[[VAL_2]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_7:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
-! CHECK:           %[[VAL_8:.*]] = arith.cmpi slt, %[[VAL_6]], %[[VAL_7]] : i32
-! CHECK:           %[[VAL_9:.*]] = arith.select %[[VAL_8]], %[[VAL_6]], %[[VAL_7]] : i32
+! CHECK:           %[[VAL_9:.*]] = arith.minsi %[[VAL_6]], %[[VAL_7]] : i32
 ! CHECK:           hlfir.assign %[[VAL_9]] to %[[VAL_5]]#0 : i32, !fir.ref<i32>
 ! CHECK:           %[[VAL_10:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
 ! CHECK:           return %[[VAL_10]] : i32
@@ -211,12 +201,10 @@ function min_dynamic_optional_scalar(a, b, c)
 ! CHECK:           %[[VAL_8:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_9:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_10:.*]] = fir.is_present %[[VAL_5]]#0 : (!fir.ref<i32>) -> i1
-! CHECK:           %[[VAL_11:.*]] = arith.cmpi slt, %[[VAL_8]], %[[VAL_9]] : i32
-! CHECK:           %[[VAL_12:.*]] = arith.select %[[VAL_11]], %[[VAL_8]], %[[VAL_9]] : i32
+! CHECK:           %[[VAL_12:.*]] = arith.minsi %[[VAL_8]], %[[VAL_9]] : i32
 ! CHECK:           %[[VAL_13:.*]] = fir.if %[[VAL_10]] -> (i32) {
 ! CHECK:             %[[VAL_14:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
-! CHECK:             %[[VAL_15:.*]] = arith.cmpi slt, %[[VAL_12]], %[[VAL_14]] : i32
-! CHECK:             %[[VAL_16:.*]] = arith.select %[[VAL_15]], %[[VAL_12]], %[[VAL_14]] : i32
+! CHECK:             %[[VAL_16:.*]] = arith.minsi %[[VAL_12]], %[[VAL_14]] : i32
 ! CHECK:             fir.result %[[VAL_16]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_12]] : i32
@@ -246,20 +234,17 @@ function min_dynamic_optional_scalar2(a, b, c, d)
 ! CHECK:           %[[VAL_11:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
 ! CHECK:           %[[VAL_12:.*]] = fir.is_present %[[VAL_6]]#0 : (!fir.ref<i32>) -> i1
 ! CHECK:           %[[VAL_13:.*]] = fir.is_present %[[VAL_7]]#0 : (!fir.ref<i32>) -> i1
-! CHECK:           %[[VAL_14:.*]] = arith.cmpi slt, %[[VAL_10]], %[[VAL_11]] : i32
-! CHECK:           %[[VAL_15:.*]] = arith.select %[[VAL_14]], %[[VAL_10]], %[[VAL_11]] : i32
+! CHECK:           %[[VAL_15:.*]] = arith.minsi %[[VAL_10]], %[[VAL_11]] : i32
 ! CHECK:           %[[VAL_16:.*]] = fir.if %[[VAL_12]] -> (i32) {
 ! CHECK:             %[[VAL_17:.*]] = fir.load %[[VAL_6]]#0 : !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:             %[[VAL_19:.*]] = arith.minsi %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             fir.result %[[VAL_19]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_15]] : i32
 ! CHECK:           }
 ! CHECK:           %[[VAL_20:.*]] = fir.if %[[VAL_13]] -> (i32) {
 ! CHECK:             %[[VAL_21:.*]] = fir.load %[[VAL_7]]#0 : !fir.ref<i32>
-! CHECK:             %[[VAL_22:.*]] = arith.cmpi slt, %[[VAL_23:.*]], %[[VAL_21]] : i32
-! CHECK:             %[[VAL_24:.*]] = arith.select %[[VAL_22]], %[[VAL_23]], %[[VAL_21]] : i32
+! CHECK:             %[[VAL_24:.*]] = arith.minsi %[[VAL_23:.*]], %[[VAL_21]] : i32
 ! CHECK:             fir.result %[[VAL_24]] : i32
 ! CHECK:           } else {
 ! CHECK:             fir.result %[[VAL_25:.*]] : i32
@@ -292,8 +277,7 @@ function min_array(a, b)
 ! 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:             %[[VAL_19:.*]] = arith.minsi %[[VAL_15]], %[[VAL_17]] : i32
 ! CHECK:             hlfir.yield_element %[[VAL_19]] : i32
 ! CHECK:           }
 ! CHECK:           hlfir.assign %[[VAL_20:.*]] to %[[VAL_11]]#0 : !hlfir.expr<42xi32>, !fir.ref<!fir.array<42xi32>>
@@ -328,13 +312,11 @@ function min_dynamic_optional_array(a, b, c)
 ! CHECK:           ^bb0(%[[VAL_17:.*]]: index):
 ! CHECK:             %[[VAL_18:.*]] = hlfir.designate %[[VAL_6]]#0 (%[[VAL_17]])  : (!fir.ref<!fir.array<10xi32>>, index) -> !fir.ref<i32>
 ! CHECK:             %[[VAL_19:.*]] = fir.load %[[VAL_18]] : !fir.ref<i32>
-! CHECK:             %[[VAL_20:.*]] = arith.cmpi slt, %[[VAL_15]], %[[VAL_19]] : i32
-! CHECK:             %[[VAL_21:.*]] = arith.select %[[VAL_20]], %[[VAL_15]], %[[VAL_19]] : i32
+! CHECK:             %[[VAL_21:.*]] = arith.minsi %[[VAL_15]], %[[VAL_19]] : i32
 ! CHECK:             %[[VAL_22:.*]] = fir.if %[[VAL_14]] -> (i32) {
 ! CHECK:               %[[VAL_23:.*]] = hlfir.designate %[[VAL_9]]#0 (%[[VAL_17]])  : (!fir.ref<!fir.array<10xi32>>, index) -> !fir.ref<i32>
 ! CHECK:               %[[VAL_24:.*]] = fir.load %[[VAL_23]] : !fir.ref<i32>
-! CHECK:               %[[VAL_25:.*]] = arith.cmpi slt, %[[VAL_21]], %[[VAL_24]] : i32
-! CHECK:               %[[VAL_26:.*]] = arith.select %[[VAL_25]], %[[VAL_21]], %[[VAL_24]] : i32
+! CHECK:               %[[VAL_26:.*]] = arith.minsi %[[VAL_21]], %[[VAL_24]] : i32
 ! CHECK:               fir.result %[[VAL_26]] : i32
 ! CHECK:             } else {
 ! CHECK:               fir.result %[[VAL_21]] : i32
@@ -812,8 +794,7 @@ subroutine allocatables_test(a, b, c)
 ! CHECK:             %[[VAL_112:.*]] = arith.addi %[[VAL_84]], %[[VAL_111]] : index
 ! CHECK:             %[[VAL_113:.*]] = hlfir.designate %[[VAL_79]] (%[[VAL_108]], %[[VAL_110]], %[[VAL_112]])  : (!fir.box<!fir.heap<!fir.array<?x?x?xi32>>>, index, index, index) -> !fir.ref<i32>
 ! CHECK:             %[[VAL_114:.*]] = fir.load %[[VAL_113]] : !fir.ref<i32>
-! CHECK:             %[[VAL_115:.*]] = arith.cmpi slt, %[[VAL_99]], %[[VAL_114]] : i32
-! CHECK:             %[[VAL_116:.*]] = arith.select %[[VAL_115]], %[[VAL_99]], %[[VAL_114]] : i32
+! CHECK:             %[[VAL_116:.*]] = arith.minsi %[[VAL_99]], %[[VAL_114]] : i32
 ! CHECK:             %[[VAL_117:.*]] = fir.if %[[VAL_70]] -> (i32) {
 ! CHECK:               %[[VAL_118:.*]] = arith.constant 0 : index
 ! CHECK:               %[[VAL_119:.*]]:3 = fir.box_dims %[[VAL_80]], %[[VAL_118]] : (!fir.box<!fir.heap<!fir.array<?x?x?xi32>>>, inde...
[truncated]

cmpLeft = builder.createConvert(loc, signlessType, left);
cmpRight = builder.createConvert(loc, signlessType, right);
if constexpr (isMax)
return mlir::arith::MaxUIOp::create(builder, loc, left, right);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

According to https://mlir.llvm.org/docs/Dialects/ArithOps/#arithmaxui-arithmaxuiop , maxUIOp requires signless integers. It seems that the original code was creating signless integers for left and right, but the new code doesn't.

I quickly checked the tests modifications and it seems all existing tests work with signed args. Perhaps add some unsigned tests to see if not having explicitly signless integers is a problem?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, Eugene! I need to bring back those converts. Unfortunately, I do not know a way to wrtie such a test, because unsigned argument of max/min is rejected by semantics.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unfortunately, I do not know a way to write such a test, because unsigned argument of max/min is rejected by semantics.

I'll look into this. Maybe I'll at least give you a temporary patch to get past the error, so the unsigned code could be tested.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe you could test it via some FIR to FIR tests starting with hlfir.maxval on unsigned (if we allow it).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vzakhari , the following worked for me with upstream flang:

implicit none
unsigned a, b, c
a = unsigned(1)
b = unsigned(2)
c = unsigned(3)
print *, "min =", min(a, b, c), ", max =", max(a, b, c)
end

Compile with -funsigned.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed, it works. Thank you, Eugene. I did see some semantics errors, but I must have done some mistake in the code. I will add the tests!

@vzakhari
vzakhari requested a review from eugeneepshteyn March 13, 2026 18:39
@github-actions

github-actions Bot commented Mar 13, 2026

Copy link
Copy Markdown

🐧 Linux x64 Test Results

  • 4369 tests passed
  • 201 tests skipped

✅ The build succeeded and all tests passed.

@jeanPerier jeanPerier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks!

Unified code between max/min and maxval/minval.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about here? Should this code be also updated?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This would require a bit more restructuring, because this function generates just the comparison for the select, but not the whole max/min. I would prefer to keep it as-is for the time being.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good. Maybe add a short comment note for the future developer?

Comment on lines 785 to 790

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this check still relevant? (I mean, you start pulling on threads, and now you have to unravel the whole yarn :-) )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should be relevant as I was not planning to enable inlining UNSIGNED maxval/minval in this PR. That would be an unrelated change. So we will continue keeping UNSIGNED maxval/minval as the hlfir operation in this pass.

@eugeneepshteyn eugeneepshteyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks, Slava!

@jeanPerier jeanPerier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@vzakhari
vzakhari merged commit 227ced1 into llvm:main Mar 18, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category openacc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants