[Mlir-commits] [mlir] [mlir][linalg/scf/transform] scalable tiling and fusion for pack/unpack ops (PR #204007)

[Andrzej Warzyński]

================
@@ -66,6 +66,57 @@ struct MergeResult {
   SmallVector<Value> replacements;
 };
 
+/// Per-dimension alignment of a loop tile size to a `linalg.pack` /
+/// `linalg.unpack` inner tile size, supplied by the caller (which performed the
+/// tiling and knows both the tile sizes and the inner tiles) so that
+/// pack/unpack TilingInterface implementations need not re-derive it from the
+/// materialized IR. An absent entry (or `Unknown`) means "no information": the
+/// implementation must fall back to its prior behavior for that dimension.
+///   - `Multiple`: the loop tile size is an integer multiple of the inner tile.
+///   - `Equal`:    the loop tile size equals the inner tile size.
+///
+/// This is a caller assertion, not a checked fact: it is only consulted when
+/// the relationship cannot be decided from the IR (e.g., scalable or dynamic
+/// sizes). When the tile and inner-tile sizes are both statically known,
+/// implementations trust that static comparison instead, so a hint that
+/// contradicts statically known sizes is ignored rather than allowed to produce
+/// incorrect tiling. The hint is otherwise never verified, so an incorrect
+/// assertion produces silently invalid tiling.
+///
+/// Entries are indexed by the dimensions the consulting method reasons about,
+/// i.e. the op's iteration domain (in pre-interchange order -- `interchange`
+/// reorders the generated loops only). This also holds for the
+/// `*FromOperandTiles` consumer-fusion methods: for a pack the iteration domain
+/// coincides with the unpacked operand's source dimensions, while for an unpack
+/// the entry for the i-th inner tile sits at its dest dimension
+/// `inner_dims_pos[i]` (a dimension of the unpacked tensor, not of the packed
+/// operand). Entries are not remapped through indexing maps or
+/// `outer_dims_perm` (for a transposing pack they stay in source order,
+/// pre-permutation), so the caller must pre-arrange them to match that order.
----------------
banach-space wrote:

These two paragraphs are confusing.

Ultimately, this comment is for `InnerTileAlignment` and all that is required is the definition of the enum. The two bottom paragraphs that I highlighted (hopefully GitHub displays this correctly) go into detail with respect to possible "use" scenarios. IMO that is not required.

I have also left some comments in other places suggesting that we make `InnerTileAlignment` take precedence over anything else that the compiler can infer.

Some specific bits I struggle with:
* `Entries are indexed by the dimensions the consulting method reasons about` - "entries" of what?
* ` the op's iteration domain (in pre-interchange order` - why is loop interchange relevant here? Like any transformation, it is prone to metadata invalidation, right?

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