This is an archive of the discontinued LLVM Phabricator instance.

[mlir][transform] Add ApplyPatternsOp and PatternRegistry
ClosedPublic

Authored by springerm on Jun 2 2023, 3:10 AM.

Details

Summary

Add a new transform op that applies patterns to a targeted payload op. Patterns can be registered by transform dialect extensions in a pattern registry.

Depends On: D151888

Diff Detail

Event Timeline

springerm created this revision.Jun 2 2023, 3:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2023, 3:10 AM
springerm requested review of this revision.Jun 2 2023, 3:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2023, 3:10 AM
ftynse accepted this revision.Jun 2 2023, 4:31 AM

Nice!

mlir/include/mlir/Dialect/Transform/IR/TransformOps.h
165

Nit: I'd take PopulatePatternsFn && here to avoid an extra constructor of std::function with dynamic allocation.

178

On a second thought, how about making this a DenseMap<StringAttr, Fn>? When verifying/applying the op, we already have a StringAttr so we will only need uniquing at extension injection time, which is likely cheaper that doing string-based search when verifying/applying (which happens more often).

mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
142
mlir/lib/Dialect/Transform/IR/TransformOps.cpp
191

I suppose this may also need to reset errorCounter.

207

Nit: it's now possible to do auto &&[index, value] here. Also prefer && to const & for enumerate as that is slated for change.

227–228

Ultra-nit: this can check it != patterns.end() below.

454

Nit: braces.

462

It's preferable to explicitly silence the diagnostic here by calling .silence(). Optionally, we can also print the contents to the debug stream.

This revision is now accepted and ready to land.Jun 2 2023, 4:31 AM
springerm marked 8 inline comments as done.Jun 2 2023, 5:29 AM
springerm added inline comments.
mlir/lib/Dialect/Transform/IR/TransformOps.cpp
462

You mean silencing the error in case getFailOnPayloadReplacementNotFound() == false, right?

springerm updated this revision to Diff 527817.Jun 2 2023, 5:29 AM
springerm marked an inline comment as done.

address comments

springerm added inline comments.Jun 2 2023, 5:31 AM
mlir/include/mlir/Dialect/Transform/IR/TransformOps.h
165

I kept StringRef here so there's no boilerplate at the call site. But that's why I needed the MLIRContext in TransformDialectData.

ftynse accepted this revision.Jun 2 2023, 5:40 AM

LGTM

ftynse added inline comments.Jun 2 2023, 5:40 AM
mlir/lib/Dialect/Transform/IR/TransformOps.cpp
462

Yep.

This revision was landed with ongoing or failed builds.Jun 2 2023, 6:04 AM
This revision was automatically updated to reflect the committed changes.