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

[Andrzej Warzyński]

https://github.com/banach-space created https://github.com/llvm/llvm-project/pull/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


>From 5f240e96428a82f82848c03e535188889347ad15 Mon Sep 17 00:00:00 2001
From: Andrzej Warzynski <andrzej.warzynski at arm.com>
Date: Wed, 10 Dec 2025 19:49:06 +0000
Subject: [PATCH] [mlir][linalg] Clarify comments and remove outdated TODO

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
---
 .../mlir/Dialect/Linalg/IR/LinalgRelayoutOps.td     | 13 ++++++++-----
 mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp   |  7 +++++--
 2 files changed, 13 insertions(+), 7 deletions(-)

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))