Page MenuHomePhabricator

[mlir][linalg] adding pass to run the interchange pattern.
ClosedPublic

Authored by gysit on Apr 16 2021, 2:05 AM.

Details

Summary

Instead of interchanging loops during the loop lowering this pass performs the interchange by permuting the indexing maps. It also updates the iterator types and the index accesses in the body of the operation.

Diff Detail

Event Timeline

gysit created this revision.Apr 16 2021, 2:05 AM
gysit requested review of this revision.Apr 16 2021, 2:05 AM
gysit added a comment.Apr 16 2021, 2:09 AM

The pass uses the existing LinalgInterchangePattern extended with the functionality to update the index operations in the body of the interchanged generic op. I relaxed the constraint that the pass can only run ops with buffer semantics but kept the limitation on generic operations. Should we relax these constraints to support all linalgOps?

nicolasvasilache requested changes to this revision.Apr 16 2021, 6:01 AM

I'd think there is very little chance that this pass is going to be useful in real compilation flows (granularity of control, need for heurisitcs, phase orderings etc etc).
I'd rather add a new case + attribute in the test pass that tests individual patterns and call it a day.

This revision now requires changes to proceed.Apr 16 2021, 6:01 AM
gysit updated this revision to Diff 338110.Apr 16 2021, 7:38 AM

Add the pattern as option to the test pass instead of having a separate interchange pass.

gysit updated this revision to Diff 338437.Mon, Apr 19, 12:24 AM

Update to current head.

mlir/lib/Dialect/Linalg/Transforms/Interchange.cpp
87

op.getBlock should do it, the verifier ensures that the region is there.
Does the assertion trigger for you?
I thought all named ops now have a region but I may be wrong ..

90

I'd lift that one level or even to the top of the function.

gysit updated this revision to Diff 338481.Mon, Apr 19, 4:42 AM

Address comments.

gysit marked 2 inline comments as done.Mon, Apr 19, 4:44 AM
gysit added inline comments.
mlir/lib/Dialect/Linalg/Transforms/Interchange.cpp
87

Added a TODO to fix this once all LinalgOps have a body

nicolasvasilache accepted this revision.Mon, Apr 19, 4:44 AM

Nice!

mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
206–207

How about just taking an OpBuilder here ?
That would be more generally applicable.

mlir/lib/Dialect/Linalg/Transforms/Interchange.cpp
65

These days, we should be able to simplify this with something like

SmallVector<AffineMap> newIndexingMaps;
for (auto m : op.indexing_maps().getAsValueRange<AffineMap>()) {
  if (!permutationMap.isEmpty())
      m = m.compose(permutationMap);
    newIndexingMaps.push_back(m);
}
 op->setAttr(getIndexingMapsAttrName(), rewriter.getAffineMapArrayAttr(newIndexingMaps));
87

yes sorry this is a bad suggestion, getBlock() gives you the parent block, please disregard.

This revision is now accepted and ready to land.Mon, Apr 19, 4:44 AM
This revision was automatically updated to reflect the committed changes.
gysit marked an inline comment as done.