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

Mon Feb 3 14:54:02 PST 2020

dcaballe accepted this revision.
dcaballe added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Joonsoo.

LGTM. Just a minor suggestion.

================
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.
----------------
andydavis1 wrote:
> 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.
Ok, it makes sense to me, thanks! I would still suggest moving the transformation part (i.e, the code under the `if` condition (lines 187-200) to its own pass within this file so that we don't have to use both flags to test the transformation flavor. That code seems independent from the rest of the test pass.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73190