[mlir][transform] Consistent linalg transform op syntax for dynamic index lists

[srcarroll]

This patch is a first pass at making consistent syntax across the LinalgTransformOps that use dynamic index lists for size parameters. Previously, there were two different forms: inline types in the list, or place them in the functional style tuple. This patch goes for the latter.

In order to do this, the printPackedOrDynamicIndexList, printDynamicIndexList and their parse counterparts were modified so that the types can be optionally provided to the corresponding custom directives.

All affected ops now use tablegen assemblyFormat, so custom parse/print functions have been removed. There are a couple ops that will likely add dynamic size support, and once that happens it should be made sure that the assembly remains consistent with the changes in this patch.

The affected ops are as follows: pack, pack_greedily, tile_using_forall. The tile_using_for and vectorize ops already used this syntax, but their custom assembly was removed.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[github-actions]

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:

darker --check --diff -r a4d10266d20bfe5930dfed77e17832af341ed66e...9b7b215e1607fa967c4bfedb749273ace416436c mlir/python/mlir/dialects/transform/structured.py mlir/test/python/dialects/transform_structured_ext.py

View the diff from darker here.

--- test/python/dialects/transform_structured_ext.py	2024-05-03 18:23:17.000000 +0000
+++ test/python/dialects/transform_structured_ext.py	2024-05-03 18:28:25.784985 +0000
@@ -509,11 +509,11 @@
 def testTileToForallPackedDynamic(target):
     n = structured.MatchOp.match_op_names(target, ["test.dummy"])
     structured.TileUsingForallOp(target, num_threads=n)
     # CHECK-LABEL: TEST: testTileToForallPackedDynamic
     # CHECK: = transform.structured.tile_using_forall
-    # CHECK-SAME: num_threads *(%0) : (!transform.any_op, !transform.any_op) 
+    # CHECK-SAME: num_threads *(%0) : (!transform.any_op, !transform.any_op)
 
 
 @run
 @create_sequence
 def testTileToForallMapping(target):

[srcarroll]

I went with my own personal preference. But i'll go with the alternative if insisted.

[ftynse]

No need for oilist unless there is an actual list inside it.

[srcarroll]

right. my bad. learned that in the last PR and didn't pay attention here

[srcarroll]

hmm. so actually oilist seems to still serve two purposes here. One is that it makes the vector_sizes keyword optional. Second is that it will elide vector_sizes when the list is empty. If I change to something like

  let assemblyFormat = [{
    $target
      (`vector_sizes` custom<DynamicIndexList>(
        $vector_sizes,
        $static_vector_sizes,
        $scalable_sizes)^)?
    attr-dict
    `:` type($target)(`,`type($vector_sizes)^)? 
  }];

then this still handles the first thing, but not the second. any suggestions?

[srcarroll]

Should I leave as is there another way without it that will keep the current behavior?

[ftynse]

Hmm, I also get to learn something ;). Feel free to keep it and add a comment as this is non-obvious (oilist stands for order-independent list so unclear why it would be used for something that isn't a list).

[srcarroll]

will do

[ftynse]

Same as above.

[srcarroll]

i did remove for this as i believe the intention is that tile_sizes is never optional, nor does it make sense to allow empty list