[flang-commits] [flang] 0f86e23 - [flang] Avoid optimizing min and max if not valid type (#134972)

[via flang-commits]

Author: Miguel Saldivar
Date: 2025-04-15T10:14:58+01:00
New Revision: 0f86e2395e4f22bf3cb05e38be05b90575066829

URL: https://github.com/llvm/llvm-project/commit/0f86e2395e4f22bf3cb05e38be05b90575066829
DIFF: https://github.com/llvm/llvm-project/commit/0f86e2395e4f22bf3cb05e38be05b90575066829.diff

LOG: [flang] Avoid optimizing min and max if not valid type (#134972)

In `makeMinMaxInitValGenerator` it explicitly checks for only
`FloatType` and `IntegerType`, so we shouldn't match if we don't have
either of those types.

Fix for #134308

Added: 
    flang/test/HLFIR/minval-maxval-issue-134308.fir

Modified: 
    flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp

Removed: 
    


################################################################################
diff  --git a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
index 96a3622f4afee..c489450384a35 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
@@ -988,8 +988,15 @@ class ReductionConversion : public mlir::OpRewritePattern<Op> {
           op, "Currently minloc/maxloc is not handled");
     } else if constexpr (std::is_same_v<Op, hlfir::MaxvalOp> ||
                          std::is_same_v<Op, hlfir::MinvalOp>) {
+      mlir::Type ty = op.getType();
+      if (!(mlir::isa<mlir::FloatType>(ty) ||
+            mlir::isa<mlir::IntegerType>(ty))) {
+        return rewriter.notifyMatchFailure(
+            op, "Type is not supported for Maxval or Minval yet");
+      }
+
       bool isMax = std::is_same_v<Op, hlfir::MaxvalOp>;
-      init = makeMinMaxInitValGenerator(isMax)(builder, loc, op.getType());
+      init = makeMinMaxInitValGenerator(isMax)(builder, loc, ty);
       genBodyFn = [inlineSource, isMax](
                       fir::FirOpBuilder builder, mlir::Location loc,
                       mlir::Value reduction,

diff  --git a/flang/test/HLFIR/minval-maxval-issue-134308.fir b/flang/test/HLFIR/minval-maxval-issue-134308.fir
new file mode 100644
index 0000000000000..9108a70be0fe7
--- /dev/null
+++ b/flang/test/HLFIR/minval-maxval-issue-134308.fir
@@ -0,0 +1,54 @@
+// Test to assure we don't hit assertion when passing characters to maxval
+// RUN: fir-opt %s -opt-bufferization | FileCheck %s
+
+
+// This simplified `fir` is derived from this test program:
+//  program FlangOptimizerBug
+//     character(len=*), parameter :: values(*) = &
+//              [ "To be   ","or not  " &
+//              , "to      ","be.     " &
+//              , "that    ","is      " &
+//              , "the     ","question"]
+//      integer :: me, ni, i
+//      character(len=len(values)) :: my_val, expected
+// 
+//      me = 1
+//      ni = 8
+// 
+//      my_val = values(mod(me-1, size(values))+1)
+//      expected = maxval([(values(mod(i-1,size(values))+1), i = 1, ni)])
+// 
+//      print *, my_val, expected
+//  end program
+
+func.func @_QQmain() {
+  %c8 = arith.constant 8 : index
+  %1 = fir.alloca !fir.char<1, 8>
+  %24 = fir.shape %c8 : (index) -> !fir.shape<1>
+  %25 = hlfir.elemental %24 typeparams %c8 unordered : (!fir.shape<1>, index) -> !hlfir.expr<?x!fir.char<1, 8>> {
+  ^bb0(%arg0: index):
+    %dummy = fir.string_lit "A"(8) : !fir.char<1, 8>
+    hlfir.yield_element %dummy : !fir.char<1, 8>
+  }
+  %26 = hlfir.maxval %25 {fastmath = #arith.fastmath<contract>} : (!hlfir.expr<?x!fir.char<1, 8>>) -> !hlfir.expr<!fir.char<1, 8>>
+  hlfir.assign %26 to %1 : !hlfir.expr<!fir.char<1, 8>>, !fir.ref<!fir.char<1, 8>>   // Assign to %1 directly
+  hlfir.destroy %26 : !hlfir.expr<!fir.char<1, 8>>
+  hlfir.destroy %25 : !hlfir.expr<?x!fir.char<1, 8>>
+  return
+}
+
+// CHECK-LABEL:  func.func @_QQmain() {
+// CHECK-NEXT:    %c8 = arith.constant 8 : index
+// CHECK-NEXT:    %[[V0:.*]] = fir.alloca !fir.char<1,8>
+// CHECK-NEXT:    %[[V1:.*]] = fir.shape %c8 : (index) -> !fir.shape<1>
+// CHECK-NEXT:    %[[V2:.*]] = hlfir.elemental %1 typeparams %c8 unordered : (!fir.shape<1>, index) -> !hlfir.expr<?x!fir.char<1,8>> {
+// CHECK-NEXT:    ^bb0(%arg0: index):
+// CHECK-NEXT:      %[[V4:.*]] = fir.string_lit "A"(8) : !fir.char<1,8>
+// CHECK-NEXT:      hlfir.yield_element %[[V4]] : !fir.char<1,8>
+// CHECK-NEXT:    }
+// CHECK-NEXT:    %[[V3:.*]] = hlfir.maxval %[[V2]] {fastmath = #arith.fastmath<contract>} : (!hlfir.expr<?x!fir.char<1,8>>) -> !hlfir.expr<!fir.char<1,8>>
+// CHECK-NEXT:    hlfir.assign %[[V3]] to %[[V0]] : !hlfir.expr<!fir.char<1,8>>, !fir.ref<!fir.char<1,8>>
+// CHECK-NEXT:    hlfir.destroy %[[V3]] : !hlfir.expr<!fir.char<1,8>>
+// CHECK-NEXT:    hlfir.destroy %[[V2]] : !hlfir.expr<?x!fir.char<1,8>>
+// CHECK-NEXT:    return
+// CHECK-NEXT:  }