[PATCH] D73826: [mlir][Linalg][WIP][RFC] OpGen hooks

[Lei Zhang via Phabricator via llvm-commits]

antiagainst added a comment.

The main value I see with the OpDefinitionGen hook is that it allows one to **automatically** insert traits and methods into the generated C++ op classes. This is preferable when a dialect's ops have **a large variation of potential traits and methods**, and that information is already encoded with other fields in the op definitions.

Use Linalg ops as an example, let's say we want to introduce a separate `OpInterface` for the newly introduced `iterator_types`, and let's call it `HasIteratorTypesInterface`. We still want to use the declarative way (`let iterators_types = ["parallel", "reduction"];`)  for specifying them as shown by Nicolas. Then the ODS hook gives us a way to automatically insert the `HasIteratorTypesInterface::Trait` into the C++ op class and synthesize the implementation for any op containing `let iterators_types = [...];` without requiring the op to explicitly list the trait in the ODS. (And IMO it makes sense to omit given the trait is just derived information from `let iterators_types = [...];`). Additionally it allows some Linalg ops to implement `HasIteratorTypesInterface` while some not, and we can use `hasTrait<HasIteratorTypesInterface::Trait>` to quickly check whether an op has iterator types defined. Similarly, one can have a separate `hasIndexingMapsInterface` for other newly introduced fields.

But I'm not sure that's the direction Nicolas is intended to evolve the Linalg dialect. Instead of having smaller and more atomic op interfaces and let each op to opt in suitable ones automatically, maybe the approach of having one gigantic op interface like today and requiring pretty much every op to implement it even if some methods does not make sense (so we have `llvm_unreachable` in the implementation) for some op, will still do for Linalg ops and carry us a long way. Then the above does not really matter.

Concretely for this patch to make progress, I think one way to avoid using ODS hook, as pointed out by Jacques, is to have a completely new TableGen backend that parses the whole `LinalgStructuredOps.td`, filters the ones that have `LinalgNamedStructured_Op`, generate the op interface implementation C++ code into a separate file, and then include that separately into the main Linalg impl file. Without using ODS hooks, one need to handle the class method signature by oneself though; but it shouldn't be too burdensome. I personally view ODS hooks as a simpler and more integrated way for even this case, but as I said in the above, it's not the main value I think ODS hooks bring. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73826