A light functional rewrite API in C++ #157
Conversation
Mogball
force-pushed
the
functional
branch
3 times, most recently
from
3c90b2e to
25f4b7f
Compare
include/Transforms/Functional.h
Outdated
| /// match at all. | ||
| template <typename PatternT, typename... Args, | ||
| bool = PatternConcept<PatternT>::verify()> | ||
| auto applyEach(Operation *scope, PatternT &&pattern, Args &&...args) { |
There was a problem hiding this comment.
auto applyToEachInScope(ScopeOp scope, ... ) ?
There was a problem hiding this comment.
I get that you want this to be general but I'd argue that you could have ScopeOp now and when generalizing you
should have a ScopeOpInterface anyway?
There was a problem hiding this comment.
A ScopeOpInterface would be any op that is isolated from above.... which might be useful to have actually. I'll look at introducing one in a later patch.
include/Transforms/Functional.h
Outdated
| template <typename OpT, typename SequenceT, typename... Args, | ||
| bool = PatternConcept<SequenceT>::verify()> | ||
| auto applySequenceAt(OpT op, SequenceT &&sequence, Args &&...args) { | ||
| Operation *scope = findNearestScope(op); |
There was a problem hiding this comment.
Same comment re ScopeOp vs Operation* and future with interfaces.
But I won't insist anymore if you fell more strongly here :)
There was a problem hiding this comment.
I'll introduce an interface
include/Transforms/Functional.h
Outdated
| SmallVector<std::unique_ptr<Region>> backup = cloneRegionsOf(scope); | ||
|
|
||
| rewriter.setInsertionPoint(op); | ||
| auto result = std::forward<SequenceT>(sequence)(op, rewriter, |
There was a problem hiding this comment.
Could you add a comment here to say what this is forwarded to? I am not clear which of the other functions can get called as the result of that.
I am asking because this relates to your generic findNearestScope but I am wondering whether some paths may bottom out on the wrap/unwrap whose impl. is very specific to ScopeOp.
Outlining generic isolated form above ops can get quite painful (i.e. finding and reconciling terminators)
lib/Transforms/Functional.cpp
Outdated
| using namespace mlir; | ||
| using functional::impl::SequencePattern; | ||
|
|
||
| Operation *mlir::functional::findNearestScope(Operation *op) { |
There was a problem hiding this comment.
Ah sorry, I get it now that I see the impl.
Do you already see your mechanisms apply to generic isolated form above?
I am also wondering how this relates to the ScopeOp if at all.
There was a problem hiding this comment.
This predates ScopeOp (which is a special case/utility op) but I removed some of these changes since they aren't needed (for now). I.e. Scope op might be something specific to linalg, whereas the rewrite system in general will work on any op isolated from above (the name is confusing...)
| tileOp, nextTargets))) | ||
| return failure(); | ||
| } | ||
| auto tileSeq = functional::SequenceBuilder() |
| callFunctional<LinalgGeneralizationPattern>(tileOp.getContext()); | ||
| return [pattern = std::move(pattern)]( | ||
| LinalgOp op, PatternRewriter &rewriter) -> FailureOr<LinalgOp> { | ||
| FailureOr<GenericOp> result = pattern(op, rewriter); |
There was a problem hiding this comment.
Feel free to make LinalgGeneralizationPattern::returningMatchAndRewrite return FailureOr upstream if you think it is better.
There was a problem hiding this comment.
This extra logic is mainly to covert GenericOp to LinalgOp. I can clean it up a little.
Mogball
force-pushed
the
functional
branch
3 times, most recently
from
3b7d475 to
60892be
Compare
Change the linalg transform interpreter to use functional-style patterns instead of relying on attributes to communicate information.
Depends on #156
Introduce a lightweight functional rewrite API in C++ and some utility functions that can be used to build a fully-featured functional rewrite system moving forward (e.g. #149).
Simplifies the linalg transform interpreter, as operation handles can be passed around directly instead of relying on attributes.