[flang] add hlfir.sum operation

[tblah]

Summary

Add an HLFIR operation for the SUM transformational intrinsic, according
to the design set out in flang/doc/HighLevelFIR.md.

I decided to make hlfir.sum lenient about the form of its
arguments. This allows the sum intrinsic to be lowered to only this HLFIR
operation, without needing several operations to convert and box
arguments. Having only one operation generated for the intrinsic
invocation should make optimization passes on HLFIR simpler.
However, the DIM argument will be loaded into memory (not allowing
passing pointers or references).

[clementval]

flang/lib/Optimizer/HLFIR/IR/HLFIRDialect.cpp

109

You don't consider fir.heap and fir.ptr types so it is likely to not work properly with POINTER and ALLOCATABLE arrays/descriptor

110

113

flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp

431

To support polymorphic descriptor as well.

451

spell out auto here.

454

spell out auto

[tblah]

Thanks for the review.

Changes:

• Support fir.heap and fir.ptr types and add tests for this
• Support fir::BaseBoxType
• Spell out auto types

[tblah]

Changes: add accessors SumOp::getDim and SumOp::getMask

[jeanPerier]

Thanks a lot for working on this!

flang/include/flang/Optimizer/HLFIR/HLFIROpBase.td

80

I do not see any uses, do you plan using it?

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

267

That is a bit hard to read, and could maybe re-use some existing logic.

I think it would be simpler to check AnyFortranNumericalArray and to move logic (https://github.com/llvm/llvm-project/blob/main/flang/include/flang/Optimizer/Builder/HLFIRTools.h#L51).

That could be implemented as: isFortranScalarNumericalType(getFortranElementType(type)) && !isBoxAddressType(type);. The last condition excludes "non dereferenced" pointer and allocatable
I do not think transformational should accept them since it adds side effects related to the pointer/allocatable dereference that are probably better kept in a fir.load.

I am missing a short word in "AnyFortranNumericalArray" to express this (NonAllocatableNorPointer is a bit heavy), maybe we could as a convention terminates the predicate that refuses fir.ref<fir.box<>> as "Object" ("AnyFortranNumericalArrayObject").

268

I personally think it would be clearer and simpler to use two Optional<...> here rather than to rely on the type. It makes the story simpler and then, you can use Optional<AnyIntegerType> for DIM and Optional<AnyFortranBooleanObject> for the mask. And you would not need custom accessors.
Under the hood, the MLIR operation is already a vector of mlir::Value anyway.
You will likely need to add the AttrSizedOperandSegments given there is more than one optional argument.

You may also need a fourth Optional<I1>:$mask_is_present to deal with the cases where the mask may be dynamically present/absent (deallocated allocatable/pointer or optional dummy variable). So far, in HLFIR, it should be avoided to assume it will be possible to find if a value is optional or not by looking at its defining operation because it may not be visible . I am starting to think this may need to be changed and to add something at the type level, but for now, it seems safer to me to indicate if an operation needs special care about an argument (and might be harder to optimize because of that).

flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp

495

You should be able to use hlfir::getFortranElementOrSequenceType(array.getType()) that will always return a fir::SequenceType (it translates hlfir::ExprType to it). You can then do

auto arrayTy = hlfir::getFortranElementOrSequenceType(array.getType()).dyn_cast<fir::SequenceType>()
if (!arrayTy)
  return emitOpError("ARRAY argument must be an array");
llvm::ArrayRef<int_64_t> arrayShape = arrayTy.getShape();

You should then get rid of unwrapType and getArrayOrExprTypeShape.

When you need a scalar type from a type (removing all fir.ptr/fir.heap...fir.box...fir.sequence, or hlfir.expr wrapping types), you can use hlfir::getFortranElementType.

503

Inside MLIR builder/verifier, do not assert here unless you the code below would crash if this was wrong. The MLIR verification error will give better feedback than an assert, and the assert will prevent the compiler to gracefully stop.

529

Making this an error might, in rare cases cause false positive: if for any reason SUM is called in an unreachable region (say if (confaormable(...)) then x = sum(....)), then it would be wrong to make it an error. This is rather unlikely (I think) that we get this kind of code with arrays that have compile time constant extents (much more likely with assumed shape, and we could imagine updating SUM after inlining one day).

I think emitWarning might be better as long as hlfir.sum code generation does not crash if this predicate is not true (but it is OK to generate "invalid code" since a warning was emitted and this code is invalid to start with and should never be reached).

[vzakhari]

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

259

This may be done separately, but this will need to support ArithFastMathInterface.

[tschuett]

flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp

448

This looks like std::optional<mlir::Value> tryGetMask().

[tblah]

Thanks for review everyone.

Changes:

• Moved type predicates to HLFIRTools from HLFIRDialect
• Use getFortranElementType() helper
• Renamed IsFortran\*BoxRefArrayOrExprType\* to IsFortran\*ArrayObject\*
• Added ArithFastMathInterface
• Removed varadic arguments, replacing them with two optional arguments
• Modified assembly format to label each optional argument e.g. hlfir.sum %array dim %dim mask %mask
• Added missing tests

[tblah]

flang/include/flang/Optimizer/HLFIR/HLFIROpBase.td

80

Well spotted, thanks!

[jeanPerier]

Some comments on the type predicate. I think the patch is missing the D143503 dependency to apply correctly.
Looks good otherwise!

flang/include/flang/Optimizer/Builder/HLFIRTools.h

367 ↗

(On Diff #495596)

In lowering so far I did not allow the generation of array expression of i1 type (hlfir.expr<?xi1>) (but did not had it to the hlfir.expr verifier, maybe I should). The rational is that it is fine to handle scalar i1, but array of i1 are a bit problematic after bufferization because arrays are placed in memory, and placing i1 in memory would lead to an extra array temp overhead whenever this array needs to be passed to the runtime or another function (since it would need to be converted to an array with logical types).

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

270

MASK can be a scalar in SUM (not a fascinating use case I admit, but legal). Do you plan to deal with this case in another way?

flang/lib/Optimizer/Builder/HLFIRTools.cpp

891 ↗

(On Diff #495596)

It looks like this logic will allow for both scalars and arrays. If that's what you want, then I think Array should be removed from the name, otherwise the logic could be:

if (isBoxAddressType(type))
  return false;
if (auto arrayTy = getFortranElementOrSequenceType(type).dyn_cast<fir::SequenceType>())
  return isFortranScalarNumericalType(arrayTy.getEleTy());
return false;

902 ↗

(On Diff #495596)

Same here. If you are in a template mood, you can even share the core logic:

template<bool (*SomeScalarTypePredicate)(mlir::Type)>
static bool isArrayObjectAndScalarPredicate(mlir::Type) {
  if (hlfir::isBoxAddressType(type))
    return false;
  if (auto arrayTy = hlfir::getFortranElementOrSequenceType(type).dyn_cast<fir::SequenceType>())
    return SomeScalarTypePredicate(arrayTy.getEleTy());
  return false;
}
hlfir::isFortranNumericalArrayObject(mlir::Type type) {
 return isArrayObjectAndScalarPredicate<isFortranScalarNumericalType>(type);
}

hlfir::isFortranLogicalOrI1ArrayObject(mlir::Type type) {
 return isArrayObjectAndScalarPredicate<isLogicalOrI1Type>(type);
}

Although in the case of MASK, I suspect scalar are OK, so your logic is good and the name needs to be changed.

[jeanPerier]

flang/include/flang/Optimizer/Builder/HLFIRTools.h

367 ↗

(On Diff #495596)

However, for scalars, i1 should be OK, so feel free to ignore that one if you change the predicate to allow scalars.

[tblah]

Changes:

• Rename isFortranLogicalOrI1ArrayObject to isMaskArgument
• Don't allow arrays of i1 for the mask argument
• Check that the array argument is an array in the tblgen type predicate rather than in SumOp::verify

flang/include/flang/Optimizer/Builder/HLFIRTools.h

367 ↗

(On Diff #495596)

Yes the intention is to allow i1 scalars. There are tests showing using scalars with this arg.

But that's a good point about arrays of i1. For now I will only allow arrays of logical.

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

268

It seems intrinsic dynamically optional arguments are TODO:
https://github.com/llvm/llvm-project/blob/b83caa32dc86cfc4d7f7b160284c2f7d002dea75/flang/lib/Lower/ConvertCall.cpp#L1093

With this TODO, it is hard for me to test if I need the 4th argument. The lowering in FIR doesn't seem to do anything special here for SUM optional arguments. I'll come back to this once HLFIR supports dynamically optional intrinsic arguments.

flang/lib/Optimizer/Builder/HLFIRTools.cpp

891 ↗

(On Diff #495596)

Thanks, I should have done this in the first place - it will let me simplify SumOp::verify

[tblah]

Changes: allow the DIM argument to be passed by reference

[jeanPerier]

In D142897#4112770, @tblah wrote:

Changes: allow the DIM argument to be passed by reference

What is the motivation to do this ? Given DIM cannot be dynamically optional and is simple integer scalar, I think it would be easier to just call loadTrivialScalar in lowering rather than to have to deal with DIM both in memory and value when doing hlfir.sum transformations. The simpler the op interface, the safer the rewrites/pattern match will be.

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

269

I would keep the requirement

[jeanPerier]

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

268

Agree this can be revisited later. I will get to this TODO at some point.

[tblah]

Thanks for taking another look!

In D142897#4112798, @jeanPerier wrote:

In D142897#4112770, @tblah wrote:

Changes: allow the DIM argument to be passed by reference

What is the motivation to do this ? Given DIM cannot be dynamically optional and is simple integer scalar, I think it would be easier to just call loadTrivialScalar in lowering rather than to have to deal with DIM both in memory and value when doing hlfir.sum transformations. The simpler the op interface, the safer the rewrites/pattern match will be.

For something like this:

subroutine sum_dim(a, s, d)
  integer :: a(:,:), s(:), d
  s = SUM(a, d)
end subroutine

Here d is a !fir.ref<i32>. As I say in the patch description, I have aimed to be as permissive as possible with the arguments (only generating conversion operations when lowering to FIR) so that a hlfir.sum is just one operation which is easy to pattern match and move. In other words, I want this to lower without the extra fir.load:

func.func @sum_dim(%arg0 : !fir.box<!fir.array<?x?xi32>>, %arg1: !fir.box<!fir.array<?xi32>>, %arg2: !fir.ref<i32>) {
  %array = hlfir.declare %arg0 [...]
  %ret = hlfir.declare %arg1 [...]
  %dim = hlfir.declare %arg2 [...]
  %expr = hlfir.sum %array#0 dim %dim#0 : ([...]) -> !hlfir.expr<?xi32>
  hlfir.assign %expr to %ret#0 [...]
  hlfir.destroy %expr [...]
  return
}

[tblah]

Changed to disallow pass-by-ref dim argument

[jeanPerier]

Thanks, looks great!
I guess something might have to be done regarding the result shape, lowering will need to be able to query it from the result, but let's discuss how to best do this when that is needed.

[tblah]

@jeanPerier there's a problem building this with -DBUILD_SHARED_LIBS=ON. Putting the new helper functions in Builder/HLFIRTools.cpp requires a dependency from libHLFIRDialect.so to libFIRBuilder.so. But libFIRBuilder.so also depends upon libHLFIRDialect.so, making it circular.

Would it be okay to move the helper functions back to HLFIRDialect.cpp?

[jeanPerier]

In D142897#4122252, @tblah wrote:

@jeanPerier there's a problem building this with -DBUILD_SHARED_LIBS=ON. Putting the new helper functions in Builder/HLFIRTools.cpp requires a dependency from libHLFIRDialect.so to libFIRBuilder.so. But libFIRBuilder.so also depends upon libHLFIRDialect.so, making it circular.

Would it be okay to move the helper functions back to HLFIRDialect.cpp?

Yes absolutely, sorry I did not catch that. All type related helper that are used in HLFIROps have to be defined in HLFIRDialect (that is why getFortranElementType is already there).

flang/include/flang/Optimizer/HLFIR/HLFIRDialect.h

 #include "flang/Optimizer/Dialect/FIRType.h"
 #include "mlir/IR/Dialect.h"
 
 namespace hlfir {
 /// Is this a type that can be used for an HLFIR variable ?
 bool isFortranVariableType(mlir::Type);
 bool isFortranScalarCharacterType(mlir::Type);
 bool isFortranScalarCharacterExprType(mlir::Type);
+bool isFortranScalarNumericalType(mlir::Type);
+bool isFortranNumericalBoxRefArrayOrExprType(mlir::Type);
+bool isI1Type(mlir::Type);
+bool isFortranLogicalOrI1BoxRefArrayOrExprType(mlir::Type);
 } // namespace hlfir
 
 #include "flang/Optimizer/HLFIR/HLFIRDialect.h.inc"
 
 #define GET_TYPEDEF_CLASSES
 #include "flang/Optimizer/HLFIR/HLFIRTypes.h.inc"
 
 #define GET_ATTRDEF_CLASSES

flang/include/flang/Optimizer/HLFIR/HLFIROpBase.td

let extraClassDeclaration = [{
     unsigned getRank() const {return getShape().size();}
     mlir::Type getElementExprType() const {
       mlir::Type eleTy = getElementType();
       if (fir::isa_trivial(eleTy))
         return eleTy;
       return hlfir::ExprType::get(eleTy.getContext(), Shape{}, eleTy,
                 isPolymorphic());
     }
+
+    // Is the shape of the sequence dynamic?
+    bool hasDynamicExtents() const {
+      for (const auto d : getShape())
+        if (d == getUnknownExtent())
+          return true;
+      return false;
+    }
+
     static constexpr int64_t getUnknownExtent() {
       return mlir::ShapedType::kDynamic;
     }
   }];
 
   let hasCustomAssemblyFormat = 1;
 
 }

 def AnyScalarCharacterEntity : Type<IsFortranScalarCharacterPred,
     "any character scalar type">;
 
 def IsFortranScalarCharacterExprPred
         : CPred<"::hlfir::isFortranScalarCharacterExprType($_self)">;
 def AnyScalarCharacterExpr : Type<IsFortranScalarCharacterExprPred,
     "any character scalar expression type">;
 
+def IsFortranNumericalBoxRefArrayOrExprTypePred
+        : CPred<"::hlfir::isFortranNumericalBoxRefArrayOrExprType($_self)">;
+def AnyFortranNumericalBoxRefArrayOrExpr : Type<IsFortranNumericalBoxRefArrayOrExprTypePred,
+    "any fir.box, fir.ref (optionally containing an array) or hlfir.expr containing a numberical type">;
+
+def IsFortranLogicalOrI1BoxRefArrayOrExprTypePred
+        : CPred<"::hlfir::isFortranLogicalOrI1BoxRefArrayOrExprType($_self)">;
+def AnyFortranLogicalOrI1BoxRefArrayOrExpr : Type<IsFortranLogicalOrI1BoxRefArrayOrExprTypePred,
+    "any fir.box, fir.ref (optionally containing an array) or hlfir.expr containing a fir.logical or i1 type">;
+
 #endif // FORTRAN_DIALECT_HLFIR_OP_BASE

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

def hlfir_SetLengthOp : hlfir_Op<"set_length", []> {
   let assemblyFormat = [{
     $string `len` $length
      attr-dict `:` functional-type(operands, results)
   }];
 
   let builders = [OpBuilder<(ins "mlir::Value":$string,"mlir::Value":$len)>];
 }
 
+def hlfir_SumOp : hlfir_Op<"sum", []> {
+  let summary = "SUM transformational intrinsic";
+  let description = [{
+    Sums the elements of an array, optionally along a particular dimension,
+    optionally if a mask is true.
+  }];
+
+  let arguments = (ins
+    AnyFortranNumericalBoxRefArrayOrExpr:$array,
+    Variadic<AnyType>:$optionals
+  );
+
+  let results = (outs hlfir_ExprType);
+
+  let assemblyFormat = [{
+    $array $optionals attr-dict `:` functional-type(operands, results)
+  }];
+
+  let extraClassDeclaration = [{
+    mlir::Value getDim();
+    mlir::Value getMask();
+  }];
+
+  let hasVerifier = 1;
+}
+
 def hlfir_AssociateOp : hlfir_Op<"associate", [AttrSizedOperandSegments,
     DeclareOpInterfaceMethods<fir_FortranVariableOpInterface>]> {
   let summary = "Create a variable from an expression value";
   let description = [{
     Create a variable from an expression value.
     For expressions, this operation is an incentive to re-use the expression
     storage, if any, after the bufferization pass when possible (if the
     expression is not used afterwards).

flang/lib/Optimizer/HLFIR/IR/HLFIRDialect.cpp

 //===-- HLFIRDialect.cpp --------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Optimizer/HLFIR/HLFIRDialect.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
 #include "flang/Optimizer/HLFIR/HLFIROps.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/DialectImplementation.h"
 #include "mlir/IR/Matchers.h"
 #include "mlir/IR/OpImplementation.h"
 #include "llvm/ADT/TypeSwitch.h"
 

 }
 
 bool hlfir::isFortranScalarCharacterExprType(mlir::Type type) {
   if (auto exprType = type.dyn_cast<hlfir::ExprType>())
     return exprType.isScalar() &&
            exprType.getElementType().isa<fir::CharacterType>();
   return false;
 }
+
+bool hlfir::isFortranScalarNumericalType(mlir::Type type) {
+  return fir::isa_integer(type) || fir::isa_real(type) ||
+         fir::isa_complex(type);
+}
+
+static mlir::Type unwrapBoxArrayOrExpr(mlir::Type type) {
+  while (type.isa<fir::BoxType, fir::ReferenceType, fir::SequenceType,
+                  hlfir::ExprType>())
+    type = mlir::TypeSwitch<mlir::Type, mlir::Type>(type)
+               .Case<fir::BoxType, fir::ReferenceType, fir::SequenceType,
+                     hlfir::ExprType>([](auto t) { return t.getEleTy(); });
+  return type;
+}
+
+bool hlfir::isFortranNumericalBoxRefArrayOrExprType(mlir::Type type) {
+  mlir::Type unwrappedType = unwrapBoxArrayOrExpr(type);
+  if (unwrappedType)
+    return isFortranScalarNumericalType(unwrappedType);
+  return false;
+}
+
+bool hlfir::isI1Type(mlir::Type type) {
+  if (mlir::IntegerType integer = type.dyn_cast<mlir::IntegerType>())
+    if (integer.getWidth() == 1)
+      return true;
+  return false;
+}
+
+bool hlfir::isFortranLogicalOrI1BoxRefArrayOrExprType(mlir::Type type) {
+  mlir::Type unwrappedType = unwrapBoxArrayOrExpr(type);
+  if (unwrappedType)
+    return mlir::isa<fir::LogicalType>(unwrappedType) ||
+           isI1Type(unwrappedType);
+  return false;
+}

flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp

 //===-- HLFIROps.cpp ------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Optimizer/HLFIR/HLFIROps.h"
 #include "flang/Optimizer/Dialect/FIROpsSupport.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/HLFIR/HLFIRDialect.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/DialectImplementation.h"
 #include "mlir/IR/Matchers.h"
 #include "mlir/IR/OpImplementation.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include <optional>
 #include <tuple>
void hlfir::SetLengthOp::build(mlir::OpBuilder &builder,
   auto resultType = hlfir::ExprType::get(
       builder.getContext(), hlfir::ExprType::Shape{},
       fir::CharacterType::get(builder.getContext(), kind, resultTypeLen),
       false);
   build(builder, result, resultType, string, len);
 }
 
 //===----------------------------------------------------------------------===//
+// SumOp
+//===----------------------------------------------------------------------===//
+
+mlir::Value hlfir::SumOp::getDim() {
+  mlir::Value dim;
+
+  for (mlir::Value arg : getOptionals()) {
+    mlir::Type ty = arg.getType();
+
+    if (hlfir::isFortranLogicalOrI1BoxRefArrayOrExprType(ty))
+      // mask argument
+      continue;
+
+    if (fir::isa_integer(ty) || ty.isa<mlir::IndexType>()) {
+      dim = arg;
+      break;
+    }
+  }
+
+  return dim;
+}
+
+mlir::Value hlfir::SumOp::getMask() {
+  mlir::Value mask;
+
+  for (mlir::Value arg : getOptionals()) {
+    mlir::Type ty = arg.getType();
+    if (hlfir::isFortranLogicalOrI1BoxRefArrayOrExprType(ty)) {
+      mask = arg;
+      break;
+    }
+  }
+
+  return mask;
+}
+
+static mlir::Type unwrapType(mlir::Type type) {
+  return mlir::TypeSwitch<mlir::Type, mlir::Type>(type)
+      .Case<fir::BoxType, fir::ReferenceType>(
+          [](auto container) { return container.getEleTy(); })
+      .Default([](mlir::Type type) { return type; });
+}
+
+static std::optional<std::tuple<mlir::Type, llvm::ArrayRef<int64_t>>>
+getArrayOrExprTypeShape(mlir::Type arrayOrExpr) {
+  if (auto array = arrayOrExpr.dyn_cast<fir::SequenceType>())
+    return {{array.getEleTy(), array.getShape()}};
+  if (auto expr = arrayOrExpr.dyn_cast<hlfir::ExprType>()) {
+    if (!expr.isArray())
+      return {};
+    return {{expr.getEleTy(), expr.getShape()}};
+  }
+  return {};
+}
+
+mlir::LogicalResult hlfir::SumOp::verify() {
+  mlir::Operation *op = getOperation();
+
+  auto results = op->getResultTypes();
+  assert(results.size() == 1);
+
+  auto args = op->getOperands();
+  assert(args.size() > 0); // array argument is compulsory
+  if (args.size() > 3)
+    return emitOpError("too many arguments");
+
+  mlir::Value array = args[0];
+  mlir::Value mask = getMask();
+
+  mlir::Type arrayTy = unwrapType(array.getType());
+  hlfir::ExprType resultTy = results[0].cast<hlfir::ExprType>();
+  auto arrayInfo = getArrayOrExprTypeShape(arrayTy);
+  if (!arrayInfo)
+    return emitOpError("ARRAY argument must be an array");
+  auto [numTy, arrayShape] = *arrayInfo;
+
+  // checked in tblgen:
+  assert(hlfir::isFortranScalarNumericalType(numTy));
+
+  if (mask) {
+    mlir::Type maskTy = unwrapType(mask.getType());
+    mlir::Type maskScalarTy;
+    llvm::ArrayRef<int64_t> maskShape;
+
+    auto maskInfo = getArrayOrExprTypeShape(maskTy);
+    if (!maskInfo)
+      maskScalarTy = maskTy;
+    else {
+      maskScalarTy = std::get<0>(*maskInfo);
+      maskShape = std::get<1>(*maskInfo);
+    }
+
+    if (!maskShape.empty()) {
+      if (maskShape.size() != arrayShape.size())
+        return emitOpError("MASK must be conformable to ARRAY");
+      static_assert(fir::SequenceType::getUnknownExtent() ==
+                    hlfir::ExprType::getUnknownExtent());
+      constexpr int64_t unknownExtent = fir::SequenceType::getUnknownExtent();
+      for (std::size_t i = 0; i < arrayShape.size(); ++i) {
+        int64_t arrayExtent = arrayShape[i];
+        int64_t maskExtent = maskShape[i];
+        if ((arrayExtent != maskExtent) && (arrayExtent != unknownExtent) &&
+            (maskExtent != unknownExtent))
+          return emitOpError("MASK must be conformable to ARRAY");
+      }
+    }
+  }
+
+  if (resultTy.isArray()) {
+    // Result is of the same type as ARRAY
+    if (resultTy.getEleTy() != numTy)
+      return emitOpError(
+          "result must have the same element type as ARRAY argument");
+
+    llvm::ArrayRef<int64_t> resultShape = resultTy.getShape();
+
+    // Result has rank n-1
+    if (resultShape.size() != (arrayShape.size() - 1))
+      return emitOpError("result rank must be one less than ARRAY");
+  } else {
+    // Result is of the same type as ARRAY
+    if (resultTy.getElementType() != numTy)
+      return emitOpError(
+          "result must have the same element type as ARRAY argument");
+  }
+
+  return mlir::success();
+}
+
+//===----------------------------------------------------------------------===//
 // AssociateOp
 //===----------------------------------------------------------------------===//
 
 void hlfir::AssociateOp::build(mlir::OpBuilder &builder,
                                mlir::OperationState &result, mlir::Value source,
                                llvm::StringRef uniq_name, mlir::Value shape,
                                mlir::ValueRange typeparams,
                                fir::FortranVariableFlagsAttr fortran_attrs) {

flang/test/HLFIR/invalid.fir

 
 // -----
 func.func @bad_concat_4(%arg0: !fir.ref<!fir.char<1,30>>) {
   %c30 = arith.constant 30 : index
   // expected-error@+1 {{'hlfir.concat' op must be provided at least two string operands}}
   %0 = hlfir.concat %arg0 len %c30 : (!fir.ref<!fir.char<1,30>>, index) -> (!hlfir.expr<!fir.char<1,30>>)
   return
 }
+
+// -----
+func.func @bad_sum0(%arg0: !hlfir.expr<i32>, %arg1: i32, %arg2: !fir.box<!fir.logical<4>>) {
+  // expected-error@+1 {{'hlfir.sum' op ARRAY argument must be an array}}
+  %0 = hlfir.sum %arg0 %arg1, %arg2 : (!hlfir.expr<i32>, i32, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+}
+
+// -----
+func.func @bad_sum1(%arg0: !hlfir.expr<?xi32>, %arg1: i32, %arg2: !fir.box<!fir.logical<4>>) {
+  // expected-error@+1 {{'hlfir.sum' op result must have the same element type as ARRAY argument}}
+  %0 = hlfir.sum %arg0 %arg1, %arg2 : (!hlfir.expr<?xi32>, i32, !fir.box<!fir.logical<4>>) -> !hlfir.expr<f32>
+}
+
+// -----
+func.func @bad_sum2(%arg0: !hlfir.expr<?xi32>, %arg1: i32, %arg2: !fir.box<!fir.array<?x?x?x?x?x!fir.logical<4>>>) {
+  // expected-error@+1 {{'hlfir.sum' op MASK must be conformable to ARRAY}}
+  %0 = hlfir.sum %arg0 %arg1, %arg2 : (!hlfir.expr<?xi32>, i32, !fir.box<!fir.array<?x?x?x?x?x!fir.logical<4>>>) -> !hlfir.expr<i32>
+}
+
+// -----
+func.func @bad_sum3(%arg0: !hlfir.expr<?x5x?xi32>, %arg1: i32, %arg2: !fir.box<!fir.array<2x6x?x!fir.logical<4>>>) {
+  // expected-error@+1 {{'hlfir.sum' op MASK must be conformable to ARRAY}}
+  %0 = hlfir.sum %arg0 %arg1, %arg2 : (!hlfir.expr<?x5x?xi32>, i32, !fir.box<!fir.array<2x6x?x!fir.logical<4>>>) -> !hlfir.expr<i32>
+}
+
+// -----
+func.func @bad_sum4(%arg0: !hlfir.expr<?xi32>, %arg1: i32, %arg2: !fir.box<!fir.logical<4>>) {
+  // expected-error@+1 {{'hlfir.sum' op result rank must be one less than ARRAY}}
+  %0 = hlfir.sum %arg0 %arg1, %arg2 : (!hlfir.expr<?xi32>, i32, !fir.box<!fir.logical<4>>) -> !hlfir.expr<?x?xi32>
+}

flang/test/HLFIR/sum.fir

+// Test hlfir.sum operation parse, verify (no errors), and unparse
+
+// RUN: fir-opt %s | fir-opt | FileCheck %s
+
+// array is an expression of known shape
+func.func @sum0(%arg0: !hlfir.expr<42xi32>) {
+  %mask = fir.alloca !fir.logical<4>
+  %c_1 = arith.constant 1 : index
+  %true = arith.constant true
+  %true_logical = fir.convert %true : (i1) -> !fir.logical<4>
+  fir.store %true_logical to %mask : !fir.ref<!fir.logical<4>>
+  %mask_box = fir.embox %mask : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+  %0 = hlfir.sum %arg0 %c_1, %mask_box : (!hlfir.expr<42xi32>, index, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+  return
+}
+// CHECK:      func.func @sum0(%[[ARRAY:.*]]: !hlfir.expr<42xi32>) {
+// CHECK-NEXT:   %[[MASK:.*]] = fir.alloca !fir.logical<4>
+// CHECK-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   %[[TRUE:.*]] = arith.constant true
+// CHECK-NEXT:   %[[LOGICAL:.*]] = fir.convert %[[TRUE]] : (i1) -> !fir.logical<4>
+// CHECK-NEXT:   fir.store %[[LOGICAL]] to %[[MASK]] : !fir.ref<!fir.logical<4>>
+// CHECK-NEXT:   %[[BOX:.*]] = fir.embox %0 : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+// CHECK-NEXT:   hlfir.sum %[[ARRAY]] %[[C1]], %[[BOX]] : (!hlfir.expr<42xi32>, index, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// array is an expression of assumed shape
+func.func @sum1(%arg0: !hlfir.expr<?xi32>) {
+  %mask = fir.alloca !fir.logical<4>
+  %c_1 = arith.constant 1 : index
+  %true = arith.constant true
+  %true_logical = fir.convert %true : (i1) -> !fir.logical<4>
+  fir.store %true_logical to %mask : !fir.ref<!fir.logical<4>>
+  %mask_box = fir.embox %mask : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+  %0 = hlfir.sum %arg0 %c_1, %mask_box : (!hlfir.expr<?xi32>, index, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+  return
+}
+// CHECK:      func.func @sum1(%[[ARRAY:.*]]: !hlfir.expr<?xi32>) {
+// CHECK-NEXT:   %[[MASK:.*]] = fir.alloca !fir.logical<4>
+// CHECK-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   %[[TRUE:.*]] = arith.constant true
+// CHECK-NEXT:   %[[LOGICAL:.*]] = fir.convert %[[TRUE]] : (i1) -> !fir.logical<4>
+// CHECK-NEXT:   fir.store %[[LOGICAL:.*]] to %[[MASK:.*]] : !fir.ref<!fir.logical<4>>
+// CHECK-NEXT:   %[[BOX:.*]] = fir.embox %[[MASK:.*]] : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+// CHECK-NEXT:   hlfir.sum %[[ARRAY:.*]] %[[C1]], %[[BOX]] : (!hlfir.expr<?xi32>, index, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// boxed array
+func.func @sum2(%arg0: !fir.box<!fir.array<42xi32>>) {
+  %mask = fir.alloca !fir.logical<4>
+  %c_1 = arith.constant 1 : index
+  %true = arith.constant true
+  %true_logical = fir.convert %true : (i1) -> !fir.logical<4>
+  fir.store %true_logical to %mask : !fir.ref<!fir.logical<4>>
+  %mask_box = fir.embox %mask : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+  %0 = hlfir.sum %arg0 %c_1, %mask_box : (!fir.box<!fir.array<42xi32>>, index, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+  return
+}
+// CHECK:      func.func @sum2(%[[ARRAY:.*]]: !fir.box<!fir.array<42xi32>>) {
+// CHECK-NEXT:   %[[MASK:.*]] = fir.alloca !fir.logical<4>
+// CHECK-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   %[[TRUE:.*]] = arith.constant true
+// CHECK-NEXT:   %[[LOGICAL:.*]] = fir.convert %[[TRUE]] : (i1) -> !fir.logical<4>
+// CHECK-NEXT:   fir.store %[[LOGICAL:.*]] to %[[MASK:.*]] : !fir.ref<!fir.logical<4>>
+// CHECK-NEXT:   %[[BOX:.*]] = fir.embox %[[MASK:.*]] : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+// CHECK-NEXT:   hlfir.sum %[[ARRAY:.*]] %[[C1]], %[[BOX]] : (!fir.box<!fir.array<42xi32>>, index, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// assumed shape boxed array
+func.func @sum3(%arg0: !fir.box<!fir.array<?xi32>>) {
+  %mask = fir.alloca !fir.logical<4>
+  %c_1 = arith.constant 1 : index
+  %true = arith.constant true
+  %true_logical = fir.convert %true : (i1) -> !fir.logical<4>
+  fir.store %true_logical to %mask : !fir.ref<!fir.logical<4>>
+  %mask_box = fir.embox %mask : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+  %0 = hlfir.sum %arg0 %c_1, %mask_box : (!fir.box<!fir.array<?xi32>>, index, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+  return
+}
+// CHECK:      func.func @sum3(%[[ARRAY:.*]]: !fir.box<!fir.array<?xi32>>) {
+// CHECK-NEXT:   %[[MASK:.*]] = fir.alloca !fir.logical<4>
+// CHECK-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   %[[TRUE:.*]] = arith.constant true
+// CHECK-NEXT:   %[[LOGICAL:.*]] = fir.convert %[[TRUE]] : (i1) -> !fir.logical<4>
+// CHECK-NEXT:   fir.store %[[LOGICAL:.*]] to %[[MASK:.*]] : !fir.ref<!fir.logical<4>>
+// CHECK-NEXT:   %[[BOX:.*]] = fir.embox %[[MASK:.*]] : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+// CHECK-NEXT:   hlfir.sum %[[ARRAY:.*]] %[[C1]], %[[BOX]] : (!fir.box<!fir.array<?xi32>>, index, !fir.box<!fir.logical<4>>) -> !hlfir.expr<i32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// known shape expr mask
+func.func @sum4(%arg0: !fir.box<!fir.array<?xi32>>, %arg1: !hlfir.expr<42x!fir.logical<4>>) {
+  %c_1 = arith.constant 1 : index
+  %0 = hlfir.sum %arg0 %c_1, %arg1 : (!fir.box<!fir.array<?xi32>>, index, !hlfir.expr<42x!fir.logical<4>>) -> !hlfir.expr<i32>
+  return
+}
+// CHECK:      func.func @sum4(%[[ARRAY:.*]]: !fir.box<!fir.array<?xi32>>, %[[MASK:.*]]: !hlfir.expr<42x!fir.logical<4>>) {
+// CHECK-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   hlfir.sum %[[ARRAY]] %[[C1]], %[[MASK]] : (!fir.box<!fir.array<?xi32>>, index, !hlfir.expr<42x!fir.logical<4>>) -> !hlfir.expr<i32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// assumed shape expr mask
+func.func @sum5(%arg0: !fir.box<!fir.array<?xi32>>, %arg1: !hlfir.expr<?x!fir.logical<4>>) {
+  %c_1 = arith.constant 1 : index
+  %0 = hlfir.sum %arg0 %c_1, %arg1 : (!fir.box<!fir.array<?xi32>>, index, !hlfir.expr<?x!fir.logical<4>>) -> !hlfir.expr<i32>
+  return
+}
+// CHECK:      func.func @sum5(%[[ARRAY:.*]]: !fir.box<!fir.array<?xi32>>, %[[MASK:.*]]: !hlfir.expr<?x!fir.logical<4>>) {
+// CHECK-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   hlfir.sum %[[ARRAY]] %[[C1]], %[[MASK]] : (!fir.box<!fir.array<?xi32>>, index, !hlfir.expr<?x!fir.logical<4>>) -> !hlfir.expr<i32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// known shape array mask
+func.func @sum6(%arg0: !fir.box<!fir.array<?xi32>>, %arg1: !fir.box<!fir.array<42x!fir.logical<4>>>) {
+  %c_1 = arith.constant 1 : index
+  %0 = hlfir.sum %arg0 %c_1, %arg1 : (!fir.box<!fir.array<?xi32>>, index, !fir.box<!fir.array<42x!fir.logical<4>>>) -> !hlfir.expr<i32>
+  return
+}
+// CHECK:      func.func @sum6(%[[ARRAY:.*]]: !fir.box<!fir.array<?xi32>>, %[[MASK:.*]]: !fir.box<!fir.array<42x!fir.logical<4>>>) {
+// CHECK-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   hlfir.sum %[[ARRAY]] %[[C1]], %[[MASK]] : (!fir.box<!fir.array<?xi32>>, index, !fir.box<!fir.array<42x!fir.logical<4>>>) -> !hlfir.expr<i32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// assumed shape array mask
+func.func @sum7(%arg0: !fir.box<!fir.array<?xi32>>, %arg1: !fir.box<!fir.array<?x!fir.logical<4>>>) {
+  %c_1 = arith.constant 1 : index
+  %0 = hlfir.sum %arg0 %c_1, %arg1 : (!fir.box<!fir.array<?xi32>>, index, !fir.box<!fir.array<?x!fir.logical<4>>>) -> !hlfir.expr<i32>
+  return
+}
+// CHECK:      func.func @sum7(%[[ARRAY:.*]]: !fir.box<!fir.array<?xi32>>, %[[MASK:.*]]: !fir.box<!fir.array<?x!fir.logical<4>>>) {
+// CHECK-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+// CHECK-NEXT:   hlfir.sum %[[ARRAY]] %[[C1]], %[[MASK]] : (!fir.box<!fir.array<?xi32>>, index, !fir.box<!fir.array<?x!fir.logical<4>>>) -> !hlfir.expr<i32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// known shape expr return
+func.func @sum8(%arg0: !fir.box<!fir.array<2x2xi32>>, %arg1: i32) {
+  %mask = fir.alloca !fir.logical<4>
+  %true = arith.constant true
+  %true_logical = fir.convert %true : (i1) -> !fir.logical<4>
+  fir.store %true_logical to %mask : !fir.ref<!fir.logical<4>>
+  %mask_box = fir.embox %mask : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+  %0 = hlfir.sum %arg0 %arg1, %mask_box : (!fir.box<!fir.array<2x2xi32>>, i32, !fir.box<!fir.logical<4>>) -> !hlfir.expr<2xi32>
+  return
+}
+// CHECK:      func.func @sum8(%[[ARRAY:.*]]: !fir.box<!fir.array<2x2xi32>>, %[[DIM:.*]]: i32) {
+// CHECK-NEXT:   %[[MASK:.*]] = fir.alloca !fir.logical<4>
+// CHECK-NEXT:   %[[TRUE:.*]] = arith.constant true
+// CHECK-NEXT:   %[[LOGICAL:.*]] = fir.convert %[[TRUE]] : (i1) -> !fir.logical<4>
+// CHECK-NEXT:   fir.store %[[LOGICAL]] to %[[MASK]] : !fir.ref<!fir.logical<4>>
+// CHECK-NEXT:   %[[BOX:.*]] = fir.embox %0 : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+// CHECK-NEXT:   hlfir.sum %[[ARRAY]] %[[DIM]], %[[BOX]] : (!fir.box<!fir.array<2x2xi32>>, i32, !fir.box<!fir.logical<4>>) -> !hlfir.expr<2xi32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }
+
+// assumed shape expr return
+func.func @sum9(%arg0: !fir.box<!fir.array<?x?xi32>>, %arg1: i32) {
+  %mask = fir.alloca !fir.logical<4>
+  %true = arith.constant true
+  %true_logical = fir.convert %true : (i1) -> !fir.logical<4>
+  fir.store %true_logical to %mask : !fir.ref<!fir.logical<4>>
+  %mask_box = fir.embox %mask : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+  %0 = hlfir.sum %arg0 %arg1, %mask_box : (!fir.box<!fir.array<?x?xi32>>, i32, !fir.box<!fir.logical<4>>) -> !hlfir.expr<?xi32>
+  return
+}
+// CHECK:      func.func @sum9(%[[ARRAY:.*]]: !fir.box<!fir.array<?x?xi32>>, %[[DIM:.*]]: i32) {
+// CHECK-NEXT:   %[[MASK:.*]] = fir.alloca !fir.logical<4>
+// CHECK-NEXT:   %[[TRUE:.*]] = arith.constant true
+// CHECK-NEXT:   %[[LOGICAL:.*]] = fir.convert %[[TRUE]] : (i1) -> !fir.logical<4>
+// CHECK-NEXT:   fir.store %[[LOGICAL]] to %[[MASK]] : !fir.ref<!fir.logical<4>>
+// CHECK-NEXT:   %[[BOX:.*]] = fir.embox %0 : (!fir.ref<!fir.logical<4>>) -> !fir.box<!fir.logical<4>>
+// CHECK-NEXT:   hlfir.sum %[[ARRAY]] %[[DIM]], %[[BOX]] : (!fir.box<!fir.array<?x?xi32>>, i32, !fir.box<!fir.logical<4>>) -> !hlfir.expr<?xi32>
+// CHECK-NEXT:   return
+// CHECK-NEXT: }