[Mlir-commits] [mlir] [mlir][linalg] Canonicalize non-identity `linalg.generic` ops (PR #101430)

[llvmlistbot at llvm.org]

================
@@ -1217,9 +1217,12 @@ struct EraseIdentityLinalgOp : public OpRewritePattern<OpTy> {
 
   LogicalResult matchAndRewrite(OpTy linalgOp,
                                 PatternRewriter &rewriter) const override {
-    // Check all indexing maps are identity.
-    if (llvm::any_of(linalgOp.getIndexingMapsArray(),
-                     [](AffineMap map) { return !map.isIdentity(); }))
+    // All indexing maps must be equal permutations
+    auto indexingMaps = linalgOp.getIndexingMapsArray();
+    if (!llvm::all_equal(indexingMaps))
+      return failure();
+
+    if (!indexingMaps.empty() && !indexingMaps.front().isPermutation())
----------------
MaheshRavishankar wrote:

This check should be unnecessary. Consider this op
```
%2 = linalg.generic {
    indexing_maps = [affine_map<(d0, d1) -> (d0)>, affine_map<(d0, d1) -> (d0)],
    iterator_types = ["parallel", "parallel"]}
    ins(%0 : tensor<10xf32>) outs(%1 : tensor<10xf32>)
```

The extents of `d0`  and `d1` are determined by how they are used to access the operands. So above, `d0` has an extent of 10. But there is no definition for the extent of `d1`. So this is an illegal `linalg.generic`. This is essentially what [this](https://github.com/llvm/llvm-project/blob/86815a1842d308521f46048bb9ed08e47c0d8357/mlir/lib/Dialect/Linalg/IR/LinalgInterfaces.cpp#L1182) check in the verifier enforces. I am not saying you should drop it, but you dont need it.

https://github.com/llvm/llvm-project/pull/101430