[PATCH] D154363: [mlir] Add an interface to decompose complex ops

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: mlir/include/mlir/Interfaces/AggregatedOpInterface.td:21
+  let description = [{
+    Interface for decomposing aggregated operations into a sequence of simpler
+    ops.
----------------
jpienaar wrote:
> qcolombet wrote:
> > qcolombet wrote:
> > > qcolombet wrote:
> > > > qcolombet wrote:
> > > > > jpienaar wrote:
> > > > > > This seems rather abstract. This seems like querying a rewrite set to see if root pattern matcher exists that matches an op, except the rewrite set is coupled with the op rather than generic. Is that what this is?
> > > > > Yes, that's exactly that.
> > > > @jpienaar 
> > > > Do you think it would make sense for this interface to return a pattern instead?
> > > Btw maybe a pass with greedy pattern rewriter is fine. I only reproduced what iree was doing here.
> > > 
> > > What do you all think?
> > @jpienaar I moved this as a linalg only interface for now.
> > Does this work for you?
> > 
> > I think this needs to mature, like @nicolasvasilache said, but what I'd like to capture with this interface is operations that can be rewritten into simpler ops within the same (set of) dialect(s).
> > 
> > Right now, I found it hard to discover the patterns/passes you need to invoke to get the lowering you need. For instance, going back to my examples from yesterday, when lowering `memref` one has to know that they need to use `memref-expand` and `expand-strided-metadata` before being able to use `finalize-memref-to-llvm`.
> > 
> > Therefore, I'd like a way to capture the lowering -rewrite- patterns that stay within the same dialect.
> This allows for a default expansion to "simpler" ops by some metric. It is rather convenient. I _could_ see it being too restrictive in general (but not by much honestly). But also many fit into this and are simple. Not blocking here. This just seems regular macro expansion not specific to linalg.
Agree on the "not specific to linalg".

I'm going to move forward with the current implementation to let it mature.
Given our conversation, I think we need to think how to make this easy to use and probably reuse the pattern rewriter infrastructure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154363