[Mlir-commits] [mlir] [mlir][OpenMP] inscan reduction modifier and scan op mlir support (PR #114737)

[Sergio Afonso]

================
@@ -2840,6 +2887,55 @@ void MaskedOp::build(OpBuilder &builder, OperationState &state,
   MaskedOp::build(builder, state, clauses.filteredThreadId);
 }
 
+//===----------------------------------------------------------------------===//
+// Spec 5.2: Scan construct (5.6)
+//===----------------------------------------------------------------------===//
+
+void ScanOp::build(OpBuilder &builder, OperationState &state,
+                   const ScanOperands &clauses) {
+  ScanOp::build(builder, state, clauses.inclusiveVars, clauses.exclusiveVars);
+}
+
+LogicalResult ScanOp::verify() {
+  if (hasExclusiveVars() && hasInclusiveVars()) {
+    return emitError(
+        "Exactly one of EXCLUSIVE or INCLUSIVE clause is expected");
+  }
+  const OperandRange &scanVars =
+      hasExclusiveVars() ? getExclusiveVars() : getInclusiveVars();
+  auto verifyScanVarsInReduction = [&scanVars](OperandRange reductionVars) {
+    for (const auto &it : scanVars)
+      if (!llvm::is_contained(reductionVars, it))
+        return false;
+    return true;
+  };
+  if (mlir::omp::WsloopOp parentOp =
+          (*this)->getParentOfType<mlir::omp::WsloopOp>()) {
+    if (parentOp.getReductionModAttr() &&
+        parentOp.getReductionModAttr().getValue() ==
+            mlir::omp::ReductionModifier::InScan) {
+      if (!verifyScanVarsInReduction(parentOp.getReductionVars())) {
----------------
skatrak wrote:

Is the check for the outside value for the reduction variable intentional? It seems like we should be referring to the privatized copy instead (e.g. below pass `%prv` to `omp.scan`, not `%0`):
```mlir
omp.wsloop reduction(Id:InScan, @add_f32 %0 -> %prv : !llvm.ptr) {
  omp.loop_nest ... {
     omp.scan exclusive(%prv : !llvm.ptr)
     omp.yield
  }
}
```
The privatized value can be accessed via the `BlockArgOpenMPOpInterface` that `WsloopOp` and `SimdOp` implement. Specifically, the `getReductionBlockArgs()` method can just replace the call to `getReductionVars()` here.

https://github.com/llvm/llvm-project/pull/114737