[PATCH] D74544: [MLIR] Add naive fusion of parallel loops.

[Stephan Herhut via Phabricator via llvm-commits]

herhut added inline comments.


================
Comment at: mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp:48
+
+/// Returns true if the the defining operation for the memref is inside the body
+/// of parallel loop.
----------------
the the -> the


================
Comment at: mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp:142
+
+static void naivelyFuseParallelOps(FuncOp f) {
+  OpBuilder b(f);
----------------
Can you extend this so it applies to any operation and traverses its regions? That way it can also be used, e.g., on a ParallelLoop to fuse loops in its body.


================
Comment at: mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp:172
+
+struct ParallelLoopFusion : public FunctionPass<ParallelLoopFusion> {
+  void runOnFunction() override { naivelyFuseParallelOps(getFunction()); }
----------------
This could be a pass that runs on any Operation then, not necessarily a function.


================
Comment at: mlir/lib/Dialect/LoopOps/Transforms/ParallelLoopFusion.cpp:183
+static PassRegistration<ParallelLoopFusion> pass("parallel-loop-fusion",
+                                                 "Fuse parallel loop nests");
----------------
This is over-committing a bit. It does not fuse loop nests.


================
Comment at: mlir/test/Dialect/Loops/parallel-loop-fusion.mlir:261
+
+func @do_not_fuse_unmatching_read_write_patterns(
+    %A: memref<2x2xf32>, %B: memref<2x2xf32>, %C: memref<2x2xf32>) {
----------------
How is this different from above?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74544/new/

https://reviews.llvm.org/D74544