[Mlir-commits] [mlir] 60010b3 - [mlir][linalg] Clarify comments and remove outdated TODO (nfc) (#171695)

[llvmlistbot at llvm.org]

Author: Andrzej Warzyński
Date: 2025-12-15T18:46:00Z
New Revision: 60010b37347756d5690f679cc425a8f61c067a6d

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

LOG: [mlir][linalg] Clarify comments and remove outdated TODO (nfc) (#171695)

The TODO being removed was originally added at my request in:
  * https://github.com/llvm/llvm-project/pull/160246

Upon revisiting the issue, it becomes clear that implementing the TODO
would require supporting IR of the following form (note the static tile
size `8` and the corresponding dynamic dimension in the result type):

```mlir
%pack = linalg.pack %src
  padding_value(%c5 : i32)
  inner_dims_pos = [0, 1]
  inner_tiles = [8, 1]
  into %dest : tensor<?x?xi32> -> tensor<1x1x?x1xi32>
```

Allowing this would be invalid, as discussed here:
* https://discourse.llvm.org/t/tensor-ops-with-dynamic-sizes-which-behaviour-is-more-correct

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.td
    mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.td b/mlir/include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.td
index 784bdd8e22f1f..f80c302ba7c51 100644
--- a/mlir/include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.td
+++ b/mlir/include/mlir/Dialect/Linalg/IR/LinalgRelayoutOps.td
@@ -58,11 +58,14 @@ class Linalg_RelayoutOp<string mnemonic, list<Trait> traits = []> :
     /// tile factors.
     DenseMap<int64_t, OpFoldResult> getDimAndTileMapping();
 
-    // TODO: Return the folded result.
-    /// Return the tile sizes as OpFoldResult. Will return the Value
-    /// of the constant Op, not the constant Attribute.
-    /// E.g., for %size = arith.constant 1 : i32 will return %size,
-    /// not 1.
+    /// Return the tile sizes as OpFoldResult(s). Note, for Ops that simply
+    /// define constants (e.g. `arith.constant`), this method returns
+    /// Value/result of the Op (as opposed to the corresponding constant
+    /// Attribute). E.g., for:
+    ///   %size = arith.constant 1 : i32
+    /// it will return %size, not 1. This is intended - replacing an SSA value
+    /// with a constant attribute would require updating the corresponding dim
+    /// from dynamic to static.
     SmallVector<OpFoldResult> getMixedTiles();
 
     /// Return the tile sizes as `int64_t`. If a tile size is dynamic

diff  --git a/mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp b/mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp
index 67e2b9f8d6077..96cc378f6c21a 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp
@@ -1219,8 +1219,11 @@ LogicalResult DecomposeOuterUnitDimsPackOpPattern::matchAndRewrite(
   }
   shapeForEmptyOp.append(packOp.getMixedTiles());
 
-  // getMixedTiles() may contain Values pointing to constant ops, not the
-  // constant attributes. Replace them with a true OpFoldResult.
+  // getMixedTiles() may contain Values pointing to constant ops (as opposed to
+  // constant attributes with the corresponding value). Replace those with
+  // attributes. This is to match the behaviour in
+  // `getPackOpSourceOrPaddedSource`, which replaces constant SSA values with
+  // attributes.
   llvm::transform(shapeForEmptyOp, shapeForEmptyOp.begin(),
                   [&](OpFoldResult ofr) {
                     if (auto val = llvm::dyn_cast<Value>(ofr))