[PATCH] D78327: [mlir][Linalg] Create a named batchmatmul op and pipe it through.

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula added a comment.

> Building region from an OperationState at parse time is not straightforward because the region is not attached to an op, which is required to fill it with a builder. A temporary fake op is 
>  created for this purpose and the resulting region is taken from it.

Is this comment relevant to this revision? What region are you trying to "build" at parse time here? Why can't the typical addRegion() followed parseRegionBody be used?



================
Comment at: mlir/include/mlir/Dialect/Linalg/IR/LinalgTraits.h:123
   }
-  /// Return the `i`-th input buffer type.
+  /// Return the `i`-th input shaped type, irrespective of buffer of tensor
+  /// type.
----------------
of -> or
buffer -> memref


================
Comment at: mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp:266
+  if (failed(BlockArgsVerifier<GenericOpType>::verify(
+          op, region.getBlocks().front())))
     return failure();
----------------
You don't need `getBlocks()` - `region.front()` will work.


================
Comment at: mlir/test/Dialect/Linalg/roundtrip.mlir:628
+                %ta3: tensor<?x?x?xf32>, %tb3: tensor<?x?xf32>, %tc3: tensor<?x?x?xf32>) {
+  linalg.batchmatmul(%a3, %b3, %c3): (memref<?x?x?xf32>, memref<?x?xf32>, memref<?x?x?xf32>) -> ()
+  linalg.batchmatmul(%ta3, %tb3, %c3): (tensor<?x?x?xf32>, tensor<?x?xf32>, memref<?x?x?xf32>) -> ()
----------------
For the custom format, better to drop the parentheses
`linalg.batchmatmul %a3, %b3, %c3 : `

FWIW, it'd be better with an underscore batch_matmul.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78327