[mlir][openacc] Use new reduction design in acc.loop

[clementval]

Summary

Use the new reduction design in acc.loop operation.

Depends on D151146

[jeanPerier]

LGTM

[razvanlupusoru]

Just a naming suggestion - otherwise looks great.

mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td

1050

Can you call this reductionRecipes? It felt natural when you named the private recipes list "privatizations" but it doesn't feel so for reductions. So if you do want them to be consistent, might make sense to rename both.

[clementval]

mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td

1050

I can. Did you mean this also for acc.serial and acc.parallel?

[razvanlupusoru]

mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td

1050

Yes please - for all.

[clementval]

Rename reductions to reductionRecipes

[razvanlupusoru]

This is great. Thank you for the renaming.

mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td

let arguments = (ins OptionalAttr<I64Attr>:$collapse,
                        UnitAttr:$independent,
                        UnitAttr:$auto_,
                        UnitAttr:$hasGang,
                        UnitAttr:$hasWorker,
                        UnitAttr:$hasVector,
                        Variadic<IntOrIndex>:$tileOperands,
                        Variadic<OpenACC_PointerLikeTypeInterface>:$privateOperands,
                        OptionalAttr<SymbolRefArrayAttr>:$privatizations,
-                       OptionalAttr<OpenACC_ReductionOperatorAttr>:$reductionOp,
-                       Variadic<AnyType>:$reductionOperands);
+                       Variadic<AnyType>:$reductionOperands,
+                       OptionalAttr<SymbolRefArrayAttr>:$reductionRecipes);
 
   let results = (outs Variadic<AnyType>:$results);
 
   let regions = (region AnyRegion:$region);
 
   let extraClassDeclaration = [{
     static StringRef getAutoAttrStrName() { return "auto"; }
     static StringRef getGangNumKeyword() { return "num"; }
     static StringRef getGangStaticKeyword() { return "static"; }
   }];
 
   let hasCustomAssemblyFormat = 1;
   let assemblyFormat = [{
     oilist(
         `gang` `` custom<GangClause>($gangNum, type($gangNum), $gangStatic, type($gangStatic), $hasGang)
       | `worker` `` custom<WorkerClause>($workerNum, type($workerNum), $hasWorker)
       | `vector` `` custom<VectorClause>($vectorLength, type($vectorLength), $hasVector)
       | `private` `(` custom<SymOperandList>(
             $privateOperands, type($privateOperands), $privatizations)
         `)`
       | `tile` `(` $tileOperands `:` type($tileOperands) `)`
-      | `reduction` `(` $reductionOperands `:` type($reductionOperands) `)`
+      | `reduction` `(` custom<SymOperandList>(
+            $reductionOperands, type($reductionOperands), $reductionRecipes)
+        `)`
     )
     $region
     ( `(` type($results)^ `)` )?
     attr-dict-with-keyword
   }];
 
   let hasVerifier = 1;
 }

mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp

LogicalResult acc::LoopOp::verify() {
   if (getSeq() && (getHasGang() || getHasWorker() || getHasVector()))
     return emitError("gang, worker or vector cannot appear with the seq attr");
 
   if (failed(checkSymOperandList<mlir::acc::PrivateRecipeOp>(
           *this, getPrivatizations(), getPrivateOperands(), "private",
           "privatizations")))
     return failure();
 
+  if (failed(checkSymOperandList<mlir::acc::ReductionRecipeOp>(
+          *this, getReductionRecipes(), getReductionOperands(), "reduction",
+          "reductions")))
+    return failure();
+
   // Check non-empty body().
   if (getRegion().empty())
     return emitError("expected non-empty body.");
 
   return success();
 }
 
 //===----------------------------------------------------------------------===//

mlir/test/Dialect/OpenACC/ops.mlir

 // CHECK: } destroy {
 // CHECK: ^bb0(%[[ARG0:.*]]: !llvm.struct<(i32, i32)>):
 // CHECK:   func.call @destroy_struct(%[[ARG0]]) : (!llvm.struct<(i32, i32)>) -> ()
 // CHECK:   acc.terminator
 
 // -----
 
 acc.reduction.recipe @reduction_add_i64 : i64 reduction_operator<add> init {
-^bb0(%0: i64):
-  %1 = arith.constant 0 : i64
-  acc.yield %1 : i64
+^bb0(%arg0: i64):
+  %0 = arith.constant 0 : i64
+  acc.yield %0 : i64
 } combiner {
-^bb0(%0: i64, %1: i64):
-  %2 = arith.addi %0, %1 : i64
-  acc.yield %2 : i64
+^bb0(%arg0: i64, %arg1: i64):
+  %0 = arith.addi %arg0, %arg1 : i64
+  acc.yield %0 : i64
 }
 
 // CHECK-LABEL: acc.reduction.recipe @reduction_add_i64 : i64 reduction_operator <add> init {
 // CHECK:       ^bb0(%{{.*}}: i64):
 // CHECK:         %[[C0:.*]] = arith.constant 0 : i64
 // CHECK:         acc.yield %[[C0]] : i64
 // CHECK:       } combiner {
 // CHECK:       ^bb0(%[[ARG0:.*]]: i64, %[[ARG1:.*]]: i64):
 // CHECK:         %[[RES:.*]] = arith.addi %[[ARG0]], %[[ARG1]] : i64
 // CHECK:         acc.yield %[[RES]] : i64
 // CHECK:       }
 
 func.func @acc_reduc_test(%a : i64) -> () {
   acc.parallel reduction(@reduction_add_i64 -> %a : i64) {
+    acc.loop reduction(@reduction_add_i64 -> %a : i64) {
+      acc.yield
+    }
+    acc.yield
   }
   return
 }
 
 // CHECK-LABEL: func.func @acc_reduc_test(
 // CHECK-SAME:    %[[ARG0:.*]]: i64)
 // CHECK:         acc.parallel reduction(@reduction_add_i64 -> %[[ARG0]] : i64)
+// CHECK:           acc.loop reduction(@reduction_add_i64 -> %[[ARG0]] : i64)
 
 // -----
 
 acc.reduction.recipe @reduction_add_i64 : i64 reduction_operator<add> init {
 ^bb0(%0: i64):
   %1 = arith.constant 0 : i64
   acc.yield %1 : i64
 } combiner {