[Mlir-commits] [mlir] [MLIR][Linalg] Add aggregate ops decomposition pass and softmax decom… (PR #97582)

[Rolf Morel]

================
@@ -1317,25 +1317,21 @@ def ConvertToLoopsOp : Op<Transform_Dialect, "structured.convert_to_loops",
 def DecomposeInterfaceOp : Op<Transform_Dialect, "structured.decompose_interface",
     [FunctionalStyleTransformOpTrait,
      MemoryEffectsOpInterface,
-     TransformOpInterface,
-     TransformEachOpTrait,
+     DeclareOpInterfaceMethods<TransformOpInterface>,
      ReportTrackingListenerFailuresOpTrait]> {
   let description = [{
-    TODO
+    Decomposes high-level named ops into a sequence of non-aggregate named ops
+    via `AggregatedOpInterface`.
+
+    The operation ignores non-decomposable ops. The return handles point to
+    a sequence of named ops produced by the decomposition.
   }];
 
   let arguments = (ins TransformHandleTypeInterface:$target);
-  let results = (outs TransformHandleTypeInterface:$transformed);
+  let results = (outs Variadic<TransformHandleTypeInterface>:$transformed);
----------------
rolfmorel wrote:

I don't think this is right.

`Variadic<..>` allows instances of an op to have different number of return values, but for each instance the number of result values still needs to be statically known, i.e. there is always a fixed number of results in the IR.

What you are doing in the implementation of `apply()` in `LinalgTransformOps.cpp` is pushing a result for each op in the payload that was associated to `target`. Note that the number of ops in this payload is only known at runtime. Hence it better be that the number of ops associated to `target` equals the number of results that the instance of `transform.structured.decompose_interface` expects, otherwise there will be a runtime error.

This issue also crops up in another situation: you optionally do not push a result if an op associated to `target` does not implement the required interface, i.e. you "ignore" it. Because a user is statically encoding the number of expected results, a user would always need to know upfront how many of the ops passed via `target` do or do not implement the interface. Otherwise the number of results would be wrong and there would be a runtime error.

Hence this scheme does not work in general. It would mean always needing to know statically how many decomposable ops are going to be associated to an instance of `decompose_interface`'s `target` argument at runtime.

Maybe @ftynse has a suggestion for still allowing the `decompose_interface` TransformOp to be "vectorized," but at the moment I don't see a sensible interface for it.

I think the interface that does work is to insist that `target` payload's size is at most one (there a number of TransformOps that insist on this). In that case you just have one return handle (so no `Variadic<..>`) which either carries all the decomposed ops or is empty in case 1) `target` was empty or 2) the associated op didn't implement the decompose interface.

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