[Mlir-commits] [flang] [mlir] [openmp] [MLIR][OpenMP] Add scan reduction lowering to llvm (PR #167031)

[Sergio Afonso]

================
@@ -2320,12 +2326,52 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
 
 static mlir::omp::ScanOp
 genScanOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
-          semantics::SemanticsContext &semaCtx, mlir::Location loc,
-          const ConstructQueue &queue, ConstructQueue::const_iterator item) {
+          semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
+          mlir::Location loc, const ConstructQueue &queue,
+          ConstructQueue::const_iterator item) {
   mlir::omp::ScanOperands clauseOps;
   genScanClauses(converter, semaCtx, item->clauses, loc, clauseOps);
-  return mlir::omp::ScanOp::create(converter.getFirOpBuilder(),
-                                   converter.getCurrentLocation(), clauseOps);
+  mlir::omp::ScanOp scanOp = mlir::omp::ScanOp::create(
+      converter.getFirOpBuilder(), converter.getCurrentLocation(), clauseOps);
+
+  /// Scan redution is not implemented with nested workshare loops, linear
+  /// clause, tiling
+  mlir::omp::LoopNestOp loopNestOp =
+      scanOp->getParentOfType<mlir::omp::LoopNestOp>();
+  mlir::omp::WsloopOp wsLoopOp = scanOp->getParentOfType<mlir::omp::WsloopOp>();
+  bool isNested =
+      (loopNestOp.getNumLoops() > 1) ||
+      (wsLoopOp && (wsLoopOp->getParentOfType<mlir::omp::WsloopOp>()));
----------------
skatrak wrote:

Do we need to guard against this case? Doesn't the `scan` directive just apply to the innermost loop in the case of something like this?
```f90
!$omp do
do i=1,10
  !$omp parallel do reduction(inscan,+: x)
  do j=1,10
    x = x + 1
    !$omp scan inclusive(x)
    v(i, j) = x
  end do
end do
```
That would be the sort of case where `wsloopOp` would have an `omp.wsloop` parent. In MLIR:
```mlir
omp.wsloop {
  omp.loop_nest %i... {
    omp.parallel {
      // Below would be the wsloopOp picked up
      omp.wsloop reduction(...) {
        omp.loop_nest %j... {
          // Loop body with omp.scan here...
          omp.yield
        }
      }
      omp.terminator
    }
    omp.yield
  }
}
```
I get that handling collapsed loops (e.g. `loopNestOp.getNumLoops() > 1`) would have to be somewhat different, as `scan` would have to work across all of them, but I guess I don't quite see the need to also disallow regular nested loops.

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