[flang] lower product intrinsic to hlfir.product operation

[jacob-crawley]

Summary

Carries out the initial lowering of the product intrinsic into HLFIR
following a similar method to sum, the --use-hlfir-intrinsic-ops flag
in test/Lower/HLFIR/expr-box is set to false so that the tests will
pass until hlfir.product is lowered into fir.call

[tblah]

Thanks for working on this. Some comments

• This patch needs a dependency on the previous hlfir.product patch, otherwise the CI builders will fail to build it. You can add that using either a Depends on: D1234 tag to the commit message/patch description. Alternately, you could press "Edit Related Revisions" on the right hand sidebar
• --use-hlfir-intrinsic-ops=false will still be required after the patch lowering hlfir.product to fir.call because that test calls bbc -emit-fir -hlfir which emits the HLFIR after lowering and before passes run. Therefore the HLFIR intrinsics pass (where hlfir.product will be lowered to fir.call) will not be run.
• Did you consider sharing the code in flang/lib/Lower/ConvertCall.cpp with hlfir.sum using a templated function as you did in the previous patch?

[jeanPerier]

Nothing to add to Tom's comment. Thanks for working on this!
(adding --use-hlfir-intrinsic-ops=false in the test is fine, I should find another way to test genExprBox without using transformational intrinsics).

[jacob-crawley]

Thanks for the reviews

Following Tom's comments I have moved the shared code in
genHLFIRIntrinsicRefCore to a new lambda called buildOperation.

[tblah]

flang/lib/Lower/ConvertCall.cpp

1355

Please could you name this in a way that shows that it is only for hlfir.sum and hlfir.product. buildOperation sounds like it could build any operation. But this couldn't build hlfir.matmul or hlfir.transpose.

What about buildReductionIntrinsic?

[jacob-crawley]

Changed the name of the buildOperation function to buildReductionIntrinsic

[tblah]

Looks good to me. Thanks for your work on this.

[jeanPerier]

Thanks! Two small nits, LGTM otherwise.

flang/lib/Lower/ConvertCall.cpp

1342

I think you probably need to pass "fir::FirOpBuilder" by reference here and below, otherwise, I am not sure what guarantee we have the the original builder insertion point will be moved properly when using a copy here.

Also nit: it seems the lambda is not capturing anything, you should rather use "[]" in this case to underline it (this helps identifying lamdas that can easily be moved as static function if desired).

[tschuett]

In D148719#4301993, @jeanPerier wrote:

Thanks! Two small nits, LGTM otherwise.

If they are that tiny, they cannot be methods of the builder?

[jacob-crawley]

addressing nit comments in review

[tblah]

Thanks for addressing the nits.

It looks like flang/lib/Lower/ConvertCall.cpp failed clang-format in CI. It also look like the FirOpBuilder is still passed by reference.

[jacob-crawley]

fixes to formatting and passing FirOpBuilder by reference in buildSumOperation/buildProductOperation

[tblah]

Thanks for the fixes. LGTM!

[jeanPerier]

LGTM, tanks

flang/lib/Lower/ConvertCall.cpp

if (auto array = normalisedResult.dyn_cast<fir::SequenceType>()) {
       resultShape = hlfir::ExprType::Shape{array.getShape()};
       elementType = array.getEleTy();
     } else {
       elementType = normalisedResult;
     }
     return hlfir::ExprType::get(builder.getContext(), resultShape, elementType,
                                 /*polymorphic=*/false);
   };
-  const std::string intrinsicName = callContext.getProcedureName();
-  if (intrinsicName == "sum") {
+
+  auto buildSumOperation = [](fir::FirOpBuilder &builder, mlir::Location loc,
+                              mlir::Type resultTy, mlir::Value array,
+                              mlir::Value dim, mlir::Value mask) {
+    return builder.create<hlfir::SumOp>(loc, resultTy, array, dim, mask);
+  };
+
+  auto buildProductOperation = [](fir::FirOpBuilder &builder,
+                                  mlir::Location loc, mlir::Type resultTy,
+                                  mlir::Value array, mlir::Value dim,
+                                  mlir::Value mask) {
+    return builder.create<hlfir::ProductOp>(loc, resultTy, array, dim, mask);
+  };
+
+  auto buildReductionIntrinsic =
+      [&](PreparedActualArguments &loweredActuals, mlir::Location loc,
+          fir::FirOpBuilder &builder, CallContext &callContext,
+          std::function<mlir::Operation *(fir::FirOpBuilder &, mlir::Location,
+                                          mlir::Type, mlir::Value, mlir::Value,
+                                          mlir::Value)>
+              buildFunc) -> std::optional<hlfir::EntityWithAttributes> {
+    // shared logic for building the product and sum operations
     llvm::SmallVector<mlir::Value> operands = getOperandVector(loweredActuals);
     assert(operands.size() == 3);
+    // dim, mask can be NULL if these arguments were not given
     mlir::Value array = operands[0];
     mlir::Value dim = operands[1];
     if (dim)
       dim = hlfir::loadTrivialScalar(loc, builder, hlfir::Entity{dim});
     mlir::Value mask = operands[2];
     mlir::Type resultTy = computeResultType(array, *callContext.resultType);
-    // dim, mask can be NULL if these arguments were not given
-    hlfir::SumOp sumOp =
-        builder.create<hlfir::SumOp>(loc, resultTy, array, dim, mask);
-    return {hlfir::EntityWithAttributes{sumOp.getResult()}};
+    auto *intrinsicOp = buildFunc(builder, loc, resultTy, array, dim, mask);
+    return {hlfir::EntityWithAttributes{intrinsicOp->getResult(0)}};
+  };
+
+  const std::string intrinsicName = callContext.getProcedureName();
+  if (intrinsicName == "sum") {
+    return buildReductionIntrinsic(loweredActuals, loc, builder, callContext,
+                                   buildSumOperation);
+  }
+  if (intrinsicName == "product") {
+    return buildReductionIntrinsic(loweredActuals, loc, builder, callContext,
+                                   buildProductOperation);
   }
   if (intrinsicName == "matmul") {
     llvm::SmallVector<mlir::Value> operands = getOperandVector(loweredActuals);
     mlir::Type resultTy =
         computeResultType(operands[0], *callContext.resultType);
     hlfir::MatmulOp matmulOp = builder.create<hlfir::MatmulOp>(
         loc, resultTy, operands[0], operands[1]);
 

flang/test/Lower/HLFIR/expr-box.f90

 ! Test lowering of of expressions as fir.box
-! RUN: bbc -hlfir -o - %s 2>&1 | FileCheck %s
+! RUN: bbc -hlfir -o - %s 2>&1 --use-hlfir-intrinsic-ops=false | FileCheck %s
 
 ! CHECK-LABEL: func.func @_QPfoo(
 ! CHECK-SAME:  %[[VAL_0:.*]]: !fir.ref<!fir.array<10xi32>>
 subroutine foo(x)
   integer :: x(21:30)
   print *, x
 ! CHECK-DAG:  %[[VAL_3:.*]] = arith.constant 21 : index
 ! CHECK-DAG:  %[[VAL_4:.*]] = arith.constant 10 : index

flang/test/Lower/HLFIR/product.f90

+! Test lowering of PRODUCT intrinsic to HLFIR
+! RUN: bbc -emit-fir -hlfir -o - %s 2>&1 | FileCheck %s
+
+! simple 1 argument PRODUCT
+subroutine product1(a, s)
+  integer :: a(:), s
+  s = PRODUCT(a)
+end subroutine
+! CHECK-LABEL: func.func @_QPproduct1(
+! CHECK:           %[[ARG0:.*]]: !fir.box<!fir.array<?xi32>> {fir.bindc_name = "a"}, %[[ARG1:.*]]: !fir.ref<i32>
+! CHECK-DAG:     %[[ARRAY:.*]]:2 = hlfir.declare %[[ARG0]]
+! CHECK-DAG:     %[[OUT:.*]]:2 = hlfir.declare %[[ARG1]]
+! CHECK-NEXT:    %[[EXPR:.*]] = hlfir.product %[[ARRAY]]#0 {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<?xi32>>) -> !hlfir.expr<i32>
+! CHECK-NEXT:    hlfir.assign %[[EXPR]] to %[[OUT]]#0 : !hlfir.expr<i32>, !fir.ref<i32>
+! CHECK-NEXT:    hlfir.destroy %[[EXPR]]
+! CHECK-NEXT:    return
+! CHECK-NEXT:  }
+
+! product with by-ref DIM argument
+subroutine product2(a, s, d)
+  integer :: a(:,:), s(:), d
+  s = PRODUCT(a, d)
+end subroutine
+! CHECK-LABEL: func.func @_QPproduct2(
+! CHECK:           %[[ARG0:.*]]: !fir.box<!fir.array<?x?xi32>> {fir.bindc_name = "a"}, %[[ARG1:.*]]: !fir.box<!fir.array<?xi32>> {fir.bindc_name = "s"}, %[[ARG2:.*]]: !fir.ref<i32>
+! CHECK-DAG:     %[[ARRAY:.*]]:2 = hlfir.declare %[[ARG0]]
+! CHECK-DAG:     %[[OUT:.*]]:2 = hlfir.declare %[[ARG1]]
+! CHECK-DAG:     %[[DIM_REF:.*]]:2 = hlfir.declare %[[ARG2]]
+! CHECK-NEXT:    %[[DIM:.*]] = fir.load %[[DIM_REF]]#0 : !fir.ref<i32>
+! CHECK-NEXT:    %[[EXPR:.*]] = hlfir.product %[[ARRAY]]#0 dim %[[DIM]] {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<?x?xi32>>, i32) -> !hlfir.expr<?xi32>
+! CHECK-NEXT:    hlfir.assign %[[EXPR]] to %[[OUT]]#0 : !hlfir.expr<?xi32>, !fir.box<!fir.array<?xi32>>
+! CHECK-NEXT:    hlfir.destroy %[[EXPR]]
+! CHECK-NEXT:    return
+! CHECK-NEXT:  }
+
+! product with scalar mask argument
+subroutine product3(a, s, m)
+  integer :: a(:), s
+  logical :: m
+  s = PRODUCT(a, m)
+end subroutine
+! CHECK-LABEL: func.func @_QPproduct3(
+! CHECK:           %[[ARG0:.*]]: !fir.box<!fir.array<?xi32>> {fir.bindc_name = "a"}, %[[ARG1:.*]]: !fir.ref<i32> {fir.bindc_name = "s"}, %[[ARG2:.*]]: !fir.ref<!fir.logical<4>> {fir.bindc_name = "m"})
+! CHECK-DAG:     %[[ARRAY:.*]]:2 = hlfir.declare %[[ARG0]]
+! CHECK-DAG:     %[[OUT:.*]]:2 = hlfir.declare %[[ARG1]]
+! CHECK-DAG:     %[[MASK:.*]]:2 = hlfir.declare %[[ARG2]]
+! CHECK-NEXT:    %[[EXPR:.*]] = hlfir.product %[[ARRAY]]#0 mask %[[MASK]]#0 {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<?xi32>>, !fir.ref<!fir.logical<4>>) -> !hlfir.expr<i32>
+! CHECK-NEXT:    hlfir.assign %[[EXPR]] to %[[OUT]]#0 : !hlfir.expr<i32>, !fir.ref<i32>
+! CHECK-NEXT:    hlfir.destroy %[[EXPR]]
+! CHECK-NEXT:    return
+! CHECK-NEXT:  }
+
+! product with array mask argument
+subroutine product4(a, s, m)
+  integer :: a(:), s
+  logical :: m(:)
+  s = PRODUCT(a, m)
+end subroutine
+
+! CHECK-LABEL: func.func @_QPproduct4(
+! CHECK:           %[[ARG0:.*]]: !fir.box<!fir.array<?xi32>> {fir.bindc_name = "a"}, %arg1: !fir.ref<i32> {fir.bindc_name = "s"}, %arg2: !fir.box<!fir.array<?x!fir.logical<4>>> {fir.bindc_name = "m"})
+! CHECK-DAG:     %[[ARRAY:.*]]:2 = hlfir.declare %[[ARG0]]
+! CHECK-DAG:     %[[OUT:.*]]:2 = hlfir.declare %[[ARG1]]
+! CHECK-DAG:     %[[MASK:.*]]:2 = hlfir.declare %[[ARG2]]
+! CHECK-NEXT:    %[[EXPR:.*]] = hlfir.product %[[ARRAY]]#0 mask %[[MASK]]#0 {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<?xi32>>, !fir.box<!fir.array<?x!fir.logical<4>>>) -> !hlfir.expr<i32>
+! CHECK-NEXT:    hlfir.assign %[[EXPR]] to %[[OUT]]#0 : !hlfir.expr<i32>, !fir.ref<i32>
+! CHECK-NEXT:    hlfir.destroy %[[EXPR]]
+! CHECK-NEXT:    return
+! CHECK-NEXT:  }
+
+! product with all 3 arguments, dim is by-val, array isn't boxed
+subroutine product5(s)
+  integer :: s(2)
+  integer :: a(2,2) = reshape((/1, 2, 3, 4/), [2,2])
+  s = PRODUCT(a, 1, .true.)
+end subroutine
+
+! CHECK-LABEL: func.func @_QPproduct5(
+! CHECK:           %[[ARG0:.*]]: !fir.ref<!fir.array<2xi32>>
+! CHECK-DAG:     %[[ADDR:.*]] = fir.address_of({{.*}}) : !fir.ref<!fir.array<2x2xi32>>
+! CHECK-DAG:     %[[ARRAY_SHAPE:.*]] = fir.shape {{.*}} -> !fir.shape<2>
+! CHECK-DAG:     %[[ARRAY:.*]]:2 = hlfir.declare %[[ADDR]](%[[ARRAY_SHAPE]])
+! CHECK-DAG:     %[[OUT_SHAPE:.*]] = fir.shape {{.*}} -> !fir.shape<1>
+! CHECK-DAG:     %[[OUT:.*]]:2 = hlfir.declare %[[ARG0]](%[[OUT_SHAPE]])
+! CHECK-DAG:     %[[C1:.*]] = arith.constant 1 : i32
+! CHECK-DAG:     %[[TRUE:.*]] = arith.constant true
+! CHECK-NEXT:    %[[EXPR:.*]] = hlfir.product %[[ARRAY]]#0 dim %[[C1]] mask %[[TRUE]]  {fastmath = #arith.fastmath<contract>} : (!fir.ref<!fir.array<2x2xi32>>, i32, i1) -> !hlfir.expr<2xi32>
+! CHECK-NEXT:    hlfir.assign %[[EXPR]] to %[[OUT]]#0 : !hlfir.expr<2xi32>, !fir.ref<!fir.array<2xi32>>
+! CHECK-NEXT:    hlfir.destroy %[[EXPR]] : !hlfir.expr<2xi32>
+! CHECK-NEXT:    return
+! CHECK-NEXT:  }
+
+! product with dimesnsion from pointer
+subroutine product6(a, s, d)
+  integer, pointer :: d
+  real :: a(:,:), s(:)
+  s = PRODUCT(a, (d))
+end subroutine
+
+! CHECK-LABEL: func.func @_QPproduct6(
+! CHECK:           %[[ARG0:.*]]:  !fir.box<!fir.array<?x?xf32>> {fir.bindc_name = "a"}, %[[ARG1:.*]]: !fir.box<!fir.array<?xf32>> {fir.bindc_name = "s"}, %[[ARG2:.*]]: !fir.ref<!fir.box<!fir.ptr<i32>>> {fir.bindc_name = "d"})
+! CHECK-DAG:     %[[ARRAY:.*]]:2 = hlfir.declare %[[ARG0]]
+! CHECK-DAG:     %[[OUT:.*]]:2 = hlfir.declare %[[ARG1]]
+! CHECK-DAG:     %[[DIM:.*]]:2 = hlfir.declare %[[ARG2]]
+! CHECK-NEXT:    %[[DIM_BOX:.*]] = fir.load %[[DIM]]#0 : !fir.ref<!fir.box<!fir.ptr<i32>>>
+! CHECK-NEXT:    %[[DIM_ADDR:.*]] = fir.box_addr %[[DIM_BOX]]  : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+! CHECK-NEXT:    %[[DIM0:.*]] = fir.load %[[DIM_ADDR]] : !fir.ptr<i32>
+! CHECK-NEXT:    %[[DIM1:.*]] = hlfir.no_reassoc %[[DIM0]] : i32
+! CHECK-NEXT:    %[[EXPR:.*]] = hlfir.product %[[ARRAY]]#0 dim %[[DIM1]]  {fastmath = #arith.fastmath<contract>} : (!fir.box<!fir.array<?x?xf32>>, i32) -> !hlfir.expr<?xf32>
+! CHECK-NEXT:    hlfir.assign %[[EXPR]] to %[[OUT]]#0 : !hlfir.expr<?xf32>, !fir.box<!fir.array<?xf32>>
+! CHECK-NEXT:    hlfir.destroy %[[EXPR]]
+! CHECK-NEXT:    return
+! CHECK-NEXT:  }