[PATCH] D73819: [mlir] Add $_op hook to RewriterGen.

[Lucy Fox via Phabricator via llvm-commits]

lucyrfox abandoned this revision.
lucyrfox marked an inline comment as done.
lucyrfox added inline comments.


================
Comment at: mlir/test/lib/TestDialect/TestOps.td:608
+            (OpNativeCodeCall3 $input),
+            [(CheckResultCount $result_count)]>;
+
----------------
antiagainst wrote:
> I think this is misleading at least, if not problematic. The current way to support this is that we capture both the op's result and its operands and then pass them in as positional parameters so `CheckResultCount` can be written as
> 
> ```
> def CheckResultCount : Constraint<CPred<"$0.getOperation()->getNumResults() == $1.getInt()">>;
> ```
> 
> and used as
> 
> ```
> [(CheckResultCount $op, $result_count)]
> ```
> 
> One thing to note that we can have multiple source ops matched (as the source pattern can match a DAG of ops) so it's unclear which matchec op  `$_op` will point to, so it's misleading at least. I think the above explicitly capturing is a better way to handle this case.
Thanks for the reply, Lei.

`$0.getOperation()` doesn't compile -- `$0`, or `$op`, is an OpResult -- but `CheckResultCount` can be written as:

```
def CheckResultCount : Constraint<CPred<"$0.getOwner()->getNumResults() == $1.getInt()">>;
```

This works for the use case I have in the TF dialect as well. --> I'll abandon this change as the `$_op` hook is not necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73819