[Mlir-commits] [mlir] a177be5 - [mlir][Linalg] Bugfix in decompose generic by unfolding permutation (#126737)

[llvmlistbot at llvm.org]

Author: gdehame
Date: 2025-02-17T11:20:58Z
New Revision: a177be5528337575ee1b7079958d4250b2eb749f

URL: https://github.com/llvm/llvm-project/commit/a177be5528337575ee1b7079958d4250b2eb749f
DIFF: https://github.com/llvm/llvm-project/commit/a177be5528337575ee1b7079958d4250b2eb749f.diff

LOG: [mlir][Linalg] Bugfix in decompose generic by unfolding permutation (#126737)

The pattern was returning success() by default which made the greedy
pattern application act as if the IR was modified and even though
nothing was changed and thus it can prevent it from converging for no
legitimate reason.

The patch makes the rewrite pattern return failure() by default and
success() if and only if the IR changed.

An example of unexpected behavior is by running `mlir-opt input.mlir
--linalg-specialize-generic-ops`, we obtain an empty mlir as output with
`input.mlir` as follows:
```
#map = affine_map<(d0) -> (d0)>
func.func @f(%arg0: tensor<8xi32>, %arg1: tensor<8xi32>) -> tensor<8xi32> {
  %0 = tensor.empty() : tensor<8xi32>
  %1 = linalg.generic {indexing_maps = [#map, #map, #map], iterator_types = ["parallel"]} ins(%arg0, %arg1: tensor<8xi32>, tensor<8xi32>) outs(%0: tensor<8xi32>) {
    ^bb0(%in: i32, %in_0: i32, %out: i32):
      %2 = arith.addi %in, %in_0: i32
      linalg.yield %2: i32
  } -> tensor<8xi32>
  return %1 : tensor<8xi32>
}
```

Added: 
    

Modified: 
    mlir/lib/Dialect/Linalg/Transforms/DecomposeGenericByUnfoldingPermutation.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Dialect/Linalg/Transforms/DecomposeGenericByUnfoldingPermutation.cpp b/mlir/lib/Dialect/Linalg/Transforms/DecomposeGenericByUnfoldingPermutation.cpp
index 83c4b5bdf1097..ae8cb94661c76 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/DecomposeGenericByUnfoldingPermutation.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/DecomposeGenericByUnfoldingPermutation.cpp
@@ -223,21 +223,21 @@ LogicalResult DecomposeProjectedPermutation::matchAndRewrite(
     newMap[i] = rewriter.getMultiDimIdentityMap(map.getNumDims());
   }
 
-  if (isChanged) {
-    SmallVector<Value> operands = op->getOperands();
-    ValueRange operandsRef(operands);
-
-    auto newOp = rewriter.create<linalg::GenericOp>(
-        /*location=*/op.getLoc(),
-        /*resultTensorTypes=*/op->getResultTypes(),
-        /*inputs=*/newInitValues,
-        /*outputs=*/operandsRef.drop_front(op.getNumDpsInputs()),
-        /*indexingMaps=*/newMap,
-        /*iteratorTypes=*/op.getIteratorTypesArray());
-
-    newOp.getRegion().takeBody(op->getRegion(0));
-    rewriter.replaceOp(op, newOp->getResults());
-  }
+  if (!isChanged)
+    return failure();
+
+  SmallVector<Value> operands = op->getOperands();
+  ValueRange operandsRef(operands);
+
+  auto newOp = rewriter.create<linalg::GenericOp>(
+      /*location=*/op.getLoc(),
+      /*resultTensorTypes=*/op->getResultTypes(),
+      /*inputs=*/newInitValues,
+      /*outputs=*/operandsRef.drop_front(op.getNumDpsInputs()),
+      /*indexingMaps=*/newMap,
+      /*iteratorTypes=*/op.getIteratorTypesArray());
+  newOp.getRegion().takeBody(op->getRegion(0));
+  rewriter.replaceOp(op, newOp->getResults());
   return success();
 }