[PATCH] D73190: [mlir] Adds affine loop fusion transformation function to LoopFusionUtils.

[Andy Davis via Phabricator via llvm-commits]

andydavis1 marked 10 inline comments as done.
andydavis1 added a comment.

thanks. Sending update ...



================
Comment at: mlir/include/mlir/Transforms/LoopFusionUtils.h:56
+/// Fuses 'srcForOp' into 'dstForOp' with destination loop block insertion point
+/// and source slice loop bounds specified in 'srcSlice'.
+void fuseLoops(AffineForOp srcForOp, AffineForOp dstForOp,
----------------
bondhugula wrote:
> A bit strange to have the destination insertion point stored in srcSlice! Separate arg?
They are inter-dependent: each source slice is computed for a specific dest insertion point.


================
Comment at: mlir/test/lib/Transforms/TestLoopFusion.cpp:45
+static llvm::cl::opt<bool> clTestLoopFusionTransformation(
+    "test-loop-fusion-transformation",
+    llvm::cl::desc("Enable testing of loop fusion transformation"),
----------------
bondhugula wrote:
> "test-loop-fusion" sounds fine?
That name collides with another flag name. Eventually, we'll be able to get rid of the other test modes and collapse things down to one flag, but probably too soon right now.


================
Comment at: mlir/test/lib/Transforms/TestLoopFusion.cpp:187
+  DenseMap<unsigned, SmallVector<AffineForOp, 2>> depthToLoops;
+  if (clTestLoopFusionTransformation) {
+    // Run loop fusion until a fixed point is reached.
----------------
dcaballe wrote:
> Would it make sense to have only the fixed point optimization "mode"? Do we need to keep the old mode for some reason (other than, maybe, fixing some tests)?
> If so, probably better to create an independent pass (`TestFixedPointLoopFusion`?) for the fixed point mode since it's independent from the other mode. In that way, we wouldn't need to use both flags `-test-loop-fusion -test-loop-fusion-transformation` for testing. 
> 
Once things stabilize, we can move towards having a fixed point mode. For now, its nice to have specific tests that check slice computation and fusion dependence  checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73190