This is an archive of the discontinued LLVM Phabricator instance.

[mlir][TilingInterface] Add pattern to tile using TilingInterface and implement TilingInterface for Linalg ops.
ClosedPublic

Authored by mravishankar on Jun 6 2022, 10:56 AM.

Details

Summary

This patch adds support for tiling operations that implement the TilingInterface.

  • It separates the loop constructs that are used to iterate over tile from the implementation of the tiling itself. For example, the use of destructive updates is more related to use of scf.for for iterating over tiles that are tensors.
  • To test the transformation, TilingInterface is implemented for LinalgOps. The separation of the looping constructs used from the implementation of tile code generation greatly simplifies the latter.
  • The implementation of TilingInterface for LinalgOp is kept as an external model for now till this approach can be fully flushed out to replace the existing tiling + fusion approaches in Linalg.

Diff Detail

Event Timeline

mravishankar created this revision.Jun 6 2022, 10:56 AM
mravishankar requested review of this revision.Jun 6 2022, 10:56 AM

Resolving some of the TODOs from earlier

  • Pattern to generate the tiled loops nest is moved to SCF dialect.
  • The TilingInterface implementation for Linalg ops is moved to use an external model. Once fully functional can be moved into the op definition.
  • Move test pass into test/lib/Interfaces/TilingInterface to handle dependencies beter.

Removing unnecessary include file.

mravishankar edited the summary of this revision. (Show Details)Jun 7 2022, 4:50 PM
mravishankar edited the summary of this revision. (Show Details)

Move test out of test/Dialect/SCF into test/Interfaces/TilingInterface

Adding more tests.

Patch is now ready for review.

mravishankar retitled this revision from [WIP][mlir][TilingInterface] Add pattern to tile using TilingInterface and implement TilingInterface for Linalg ops. to [mlir][TilingInterface] Add pattern to tile using TilingInterface and implement TilingInterface for Linalg ops..Jun 7 2022, 7:28 PM
mravishankar edited the summary of this revision. (Show Details)
mravishankar edited the summary of this revision. (Show Details)

Rebase and add bazel build files.

mlir/include/mlir/Interfaces/TilingInterface.h
67 ↗(On Diff #435421)

I am unclear why all this duplication is needed as well as all the test pass C++, esp. as we know we are moving away from this.

Can we do either of:

  1. add a new op to the transform dialect, drop any filter duplication and test pass and just use a self-contained mlir test ? (preferred)
  2. drop all the filter duplication and have a local pattern in your test pass that wraps the new pattern and adds the filter for testing purposes only?
mlir/include/mlir/Interfaces/TilingInterface.h
33 ↗(On Diff #435421)

I don't think these belong to the interface but they are rather transform configuration objects.
Can these move to the same place as TileUsingSCFForOp for now and be generalized on a per-need basis?

mlir/lib/Dialect/Linalg/Transforms/TilingInterfaceImpl.cpp
2

typo

mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
133

LLVM_DEBUG(if (!loops.empty()) {llvm::dbgs() << msg << *loops.front() < "\n";} );

161

Nesting feels unnecessarily convoluted here; if you quarantined the filter stuff you could write:

if (op->getNumResults() == 0)
  return SCFTilingResult {tiledImplementation[0], std::move(loops)};

if (loops.empty()) {
  rewriter.replaceOp(tiledImplementationop, .front()->getResults());
  return SCFTilingResult {tiledImplementation[0], std::move(loops)};
}

// The interesting case.
...

Address review comments.

mravishankar marked 5 inline comments as done.Jun 9 2022, 2:37 PM
mravishankar added inline comments.
mlir/include/mlir/Interfaces/TilingInterface.h
33 ↗(On Diff #435421)

Ok, Ill move it there, but seems like something that will be good to standardize on.

67 ↗(On Diff #435421)

Went with option 2. The filter is just used in the testing pass. Does reduce the duplication a lot. THanks!

mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
161

Much better! Thanks!

mravishankar marked 3 inline comments as done.

Rebase and add convolution tests.

nicolasvasilache accepted this revision.Jun 13 2022, 8:02 AM
This revision is now accepted and ready to land.Jun 13 2022, 8:02 AM
This revision was landed with ongoing or failed builds.Jun 13 2022, 1:38 PM
This revision was automatically updated to reflect the committed changes.