This patch adds a mixin for ApplyPatternsOp to _transform_ops_ext.py
with syntactic sugar for construction such ops. Curiously, the op did
not have any constructors yet, probably because its tablegen definition
said to skip the default builders. The new constructor is thus quite
straightforward. The commit also adds a refined region property which
returns the first block of the single region.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Nit: can we retitle the commit to explicitly refer to python bindings? Currently it reads as if it were actually adding an op.
mlir/python/mlir/dialects/_transform_ops_ext.py | ||
---|---|---|
54 | It is confusing that region property returns a block rather than a region. Can we give it a different name? |
mlir/python/mlir/dialects/_transform_ops_ext.py | ||
---|---|---|
54 | Ha, I had overlooked this comment until now. Yes, I agree. Relatedly, the name region in the td file isn't particularly meaningful. Maybe that should be changed? Maybe body or patterns? (Then the name of this property would be the same.) |
mlir/python/mlir/dialects/_transform_ops_ext.py | ||
---|---|---|
54 | Can we make it consistent with what's in ODS? |
mlir/python/mlir/dialects/_transform_ops_ext.py | ||
---|---|---|
54 | That's the current state: it is called "region" in ODS. If we want a different name *and* consistency, we need to change ODS. |
mlir/python/mlir/dialects/_transform_ops_ext.py | ||
---|---|---|
54 | In case it isn't clear: I can't address both of your requests at the same time, @ftynse, rename region and make it consistent with ODS, because it is called region in ODS. My preferred solution is to rename the region to patterns in ODS and do the same here. Does that work for you? |
mlir/python/mlir/dialects/_transform_ops_ext.py | ||
---|---|---|
54 | Works for me, thanks! |
- Rebase on top of https://reviews.llvm.org/D155810.
- Follow its renamig from region to patterns.
- Rename test label to make them prefix-free.
It is confusing that region property returns a block rather than a region. Can we give it a different name?