[PATCH] D78463: [mlir][Linalg] NFC: Refactor fusion on tensors to enable extending itto fusing different kinds of linalg operations on tensors.

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache added a comment.

Mostly looks good but the extra level of indirection through the template class was harder to follow than necessary I think.

Can we make it just an untyped entry point and drop the extra class ?



================
Comment at: mlir/include/mlir/Dialect/Linalg/Utils/Utils.h:77
+/// position `consumerIdx` of the consumer.
+template <typename ConcreteOp>
+Operation *fuseTensorOps(PatternRewriter &rewriter, ConcreteOp consumer,
----------------
This looks like an easy way to get link errors.
Can you avoid templating here, just take `Operation*` as a consumer and do a switch underneath?


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp:417
+  /// result of producer is used as operand at `consumerIdx` in the `consumer`.
+  static Operation *fuse(PatternRewriter &rewriter, ProducerOpTy producer,
+                         ConsumerOpTy consumer, unsigned consumerIdx,
----------------
This level of indirection does not seem to pay for itself.
Can we move the body of this function just above:
```
  if (auto genericOp = dyn_cast<GenericOp>(producer)) {
```
on l. 590


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78463