[flang] Avoid optimizing min and max if not valid type

[Saldivarcher]

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

[llvmbot]

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

Author: Miguel Saldivar (Saldivarcher)

Changes

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

---

Full diff: https://github.com/llvm/llvm-project/pull/134972.diff

1 Files Affected:

• (modified) flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp (+8-1)

diff --git a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
index 96a3622f4afee..cb3894965f9aa 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>) {
+      auto 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,

[Saldivarcher]

Note, I need to add a test. First time working here, so still learning!

[tblah]

Thank you for fixing this. The change looks good to me once you have added a test.

I would test this by checking that hlfir.minval etc are not modified by the pass for a few types that aren't integers or floating point. See flang/test/HLFIR/minval-elemental.fir for an example test for this pass.

[Saldivarcher]

Thank you for fixing this. The change looks good to me once you have added a test.

I would test this by checking that hlfir.minval etc are not modified by the pass for a few types that aren't integers or floating point. See flang/test/HLFIR/minval-elemental.fir for an example test for this pass.

Since maxval and minval only take arrays of integer, real, and character, wouldn't this new test and the old test cover everything?

[tblah]

LGTM. Thanks for the update. Feel free to merge without another round of review.

nit: Please could you also test minval in the same way.

[Saldivarcher]

@tblah I actually don't have commit access, can you merge for me please. Thanks again for reviewing this PR btw 👍