[PATCH] D74491: [mlir][Linalg] NFC - Refactor in preparation for automatic Linalg "named" ops.

[Mahesh Ravishankar via Phabricator via llvm-commits]

mravishankar accepted this revision.
mravishankar added a comment.
This revision is now accepted and ready to land.

Overall looks fine to me.



================
Comment at: mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td:368
+    llvm::Optional<SmallVector<AffineMap, 8>> referenceIndexingMaps() {
+      llvm_unreachable("NYI referenceIndexingMaps for MatvecOp");
     }
----------------
Curious why. Isnt it 
`affine_map<(d0, d1) -> (d0, d1)>, affine_map<(d0, d1) -> (d0)>, affine_map<(d0, d1) -> (d1)>`?


================
Comment at: mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td:392
+    llvm::Optional<SmallVector<AffineMap, 8>> referenceIndexingMaps() {
+      llvm_unreachable("NYI referenceIndexingMaps for MatmulOp");
     }
----------------
Same as above. Is there some information missing that blocks this?


================
Comment at: mlir/include/mlir/Dialect/Linalg/IR/LinalgTraits.h:234
+    auto name = this->getOperation()->getName().getStringRef();
+    if (!maybeReferenceIteratorTypes && name != "generic" &&
+        name != "indexed_generic") {
----------------
I think you dont need this. You already have an unreachable in referenceIteratorTypes


================
Comment at: mlir/include/mlir/Dialect/Linalg/IR/LinalgTraits.h:266
+    auto name = this->getOperation()->getName().getStringRef();
+    if (!maybeReferenceIndexingMaps && name != "generic" &&
+        name != "indexed_generic") {
----------------
Same here. You probably dont need this. referenceIndexingMaps already has the unreachable where needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74491