[MLIR] [Transforms] Let `transform.structured.convert_to_loops` return handles to loops #83984

lhunloh · 2024-03-05T09:46:52Z

This lets transform.structured.convert_to_loops return handles to the generated loops, making this transformation more useful to use for (transformation-)nesting purposes. This is modelled after SCFs transform.loop.forall_to_for which returns handles to loops.

Introduced in commit aa2a96a, with a note that they might move out of the Linalg-Dialect, but no reason given for the non-return of handles. As far as I can see, this transform always returns loops.

github-actions · 2024-03-05T09:47:11Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-03-05T09:47:43Z

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-linalg

Author: None (lhunloh)

Changes

This lets transform.structured.convert_to_loops return handles to the generated loops, making this transformation more useful to use for (transformation-)nesting purposes. This is modelled after SCFs transform.loop.forall_to_for which returns handles to loops.

Introduced in commit #aa2a96a, with a note that they might move out of the Linalg-Dialect, but no reason given for the non-return of handles. As far as I can see, this transform always returns loops.

Full diff: https://github.com/llvm/llvm-project/pull/83984.diff

3 Files Affected:

(modified) mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td (+3-3)
(modified) mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp (+3)
(modified) mlir/test/Interfaces/TilingInterface/lower-to-loops-using-interface.mlir (+16-8)

diff --git a/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td b/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td
index 53ed31877c6f24..3d77e308f98f50 100644
--- a/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td
+++ b/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td
@@ -1281,14 +1281,14 @@ def ConvertToLoopsOp : Op<Transform_Dialect, "structured.convert_to_loops",
   let description = [{
     For operations that implement the `TilingInterface`, and implement
     the `generateScalarImplementation` method, lowers the operation to
-    loops. This operation does not return any handles.
+    loops. The return handles point to the generated loops.
   }];
 
   let arguments = (ins TransformHandleTypeInterface:$target);
-  let results = (outs);
+  let results = (outs Variadic<TransformHandleTypeInterface>:$result);
 
   let assemblyFormat = [{
-    $target attr-dict `:` type($target)
+    $target attr-dict `:` functional-type(operands, results)
   }];
 
   let extraClassDeclaration = [{
diff --git a/mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp b/mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp
index 0ac0a89dcc76ae..259d6e1b8f277e 100644
--- a/mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp
+++ b/mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp
@@ -2122,6 +2122,9 @@ DiagnosedSilenceableFailure transform::ConvertToLoopsOp::applyToOne(
   if (failed(loops))
     return emitDefaultDefiniteFailure(target);
   rewriter.eraseOp(target);
+  for(auto &loop: *loops){
+    results.push_back(loop);
+  }
   return DiagnosedSilenceableFailure::success();
 }
 
diff --git a/mlir/test/Interfaces/TilingInterface/lower-to-loops-using-interface.mlir b/mlir/test/Interfaces/TilingInterface/lower-to-loops-using-interface.mlir
index 1b2c553b25ded0..e431e63134eefc 100644
--- a/mlir/test/Interfaces/TilingInterface/lower-to-loops-using-interface.mlir
+++ b/mlir/test/Interfaces/TilingInterface/lower-to-loops-using-interface.mlir
@@ -11,7 +11,8 @@ module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg1 : !transform.any_op {transform.readonly}) {
     %matmul = transform.structured.match ops{["linalg.matmul"]} in %arg1
       : (!transform.any_op) -> !transform.any_op
-    transform.structured.convert_to_loops %matmul : !transform.any_op
+    %0:3 = transform.structured.convert_to_loops %matmul
+      : (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op)
     transform.yield
   }
 }
@@ -66,7 +67,8 @@ module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg1 : !transform.any_op {transform.readonly}) {
     %generic = transform.structured.match ops{["linalg.generic"]} in %arg1
       : (!transform.any_op) -> !transform.any_op
-    transform.structured.convert_to_loops %generic : !transform.any_op
+    %0:2 = transform.structured.convert_to_loops %generic
+      : (!transform.any_op) -> (!transform.any_op, !transform.any_op)
     transform.yield
   }
 }
@@ -111,7 +113,8 @@ module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg1 : !transform.any_op {transform.readonly}) {
     %conv = transform.structured.match ops{["linalg.conv_2d_nhwc_hwcf"]} in %arg1
       : (!transform.any_op) -> !transform.any_op
-    transform.structured.convert_to_loops %conv : !transform.any_op
+    %0:7 = transform.structured.convert_to_loops %conv
+      : (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op, !transform.any_op, !transform.any_op, !transform.any_op, !transform.any_op)
     transform.yield
   }
 }
@@ -165,7 +168,8 @@ module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg1 : !transform.any_op {transform.readonly}) {
     %pool = transform.structured.match ops{["linalg.pooling_nhwc_max"]} in %arg1
       : (!transform.any_op) -> !transform.any_op
-    transform.structured.convert_to_loops %pool : !transform.any_op
+    %0:6 = transform.structured.convert_to_loops %pool
+      : (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op, !transform.any_op, !transform.any_op, !transform.any_op)
     transform.yield
   }
 }
@@ -216,7 +220,8 @@ module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg1 : !transform.any_op {transform.readonly}) {
     %map = transform.structured.match ops{["linalg.map"]} in %arg1
       : (!transform.any_op) -> !transform.any_op
-    transform.structured.convert_to_loops %map : !transform.any_op
+    %0 = transform.structured.convert_to_loops %map
+      : (!transform.any_op) -> (!transform.any_op)
     transform.yield
   }
 }
@@ -248,7 +253,8 @@ module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg1 : !transform.any_op {transform.readonly}) {
     %transpose = transform.structured.match ops{["linalg.transpose"]} in %arg1
       : (!transform.any_op) -> !transform.any_op
-    transform.structured.convert_to_loops %transpose : !transform.any_op
+    %0:3 = transform.structured.convert_to_loops %transpose
+      : (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op)
     transform.yield
   }
 }
@@ -285,7 +291,8 @@ module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg1 : !transform.any_op {transform.readonly}) {
     %reduce = transform.structured.match ops{["linalg.reduce"]} in %arg1
       : (!transform.any_op) -> !transform.any_op
-    transform.structured.convert_to_loops %reduce : !transform.any_op
+    %0:3 = transform.structured.convert_to_loops %reduce
+      : (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op)
     transform.yield
   }
 }
@@ -322,7 +329,8 @@ module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg1 : !transform.any_op {transform.readonly}) {
     %broadcast = transform.structured.match ops{["linalg.broadcast"]} in %arg1
       : (!transform.any_op) -> !transform.any_op
-    transform.structured.convert_to_loops %broadcast : !transform.any_op
+    %0:3 = transform.structured.convert_to_loops %broadcast
+      : (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op)
     transform.yield
   }
 }

github-actions · 2024-03-05T09:49:28Z

✅ With the latest revision this PR passed the C/C++ code formatter.

lhunloh · 2024-03-05T10:05:36Z

Pinging @MaheshRavishankar as I don't have permission to add reviewers and it's your initial code. :)

mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp

ftynse

Could we have a test for this?

mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp

lhunloh · 2024-03-05T17:47:03Z

Test added, review feedback worked in. :)
We now return a single handle to all generated loops, which is easily splittable via transform.split_handle.

qedawkins

nice, thanks!

mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td

This lets `transform.structured.convert_to_loops` return a handle to all generated loops, making this transformation more useful to use for (transformation-)nesting purposes. This handle may be split via `transform.split_handle` to get handles to each individual loop.

lhunloh · 2024-03-05T18:50:48Z

Commits are squashed, I don't have write access to merge, if everythings looks good now. :) @ftynse @qedawkins

github-actions · 2024-03-06T22:07:48Z

@lhunloh Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

…n handles to loops (llvm#83984) This lets `transform.structured.convert_to_loops` return handles to the generated loops, making this transformation more useful to use for (transformation-)nesting purposes. This is modelled after SCFs `transform.loop.forall_to_for` which returns handles to loops. Introduced in commit aa2a96a, with a note that they might move out of the `Linalg`-Dialect, but no reason given for the non-return of handles. As far as I can see, this transform always returns loops. (cherry picked from commit 47bc565)

lhunloh requested review from dcaballe and nicolasvasilache as code owners March 5, 2024 09:46

llvmbot added mlir:linalg mlir labels Mar 5, 2024

lhunloh changed the title ~~Let transform.structured.convert_to_loops return handles to loops~~ [MLIR] [Transforms] Let transform.structured.convert_to_loops return handles to loops Mar 5, 2024

lhunloh force-pushed the return_handles branch 2 times, most recently from d2fa491 to 49b688c Compare March 5, 2024 10:02

MaheshRavishankar requested review from ftynse and qedawkins March 5, 2024 10:24

qedawkins requested changes Mar 5, 2024

View reviewed changes

mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp Show resolved Hide resolved

ftynse reviewed Mar 5, 2024

View reviewed changes

mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp Outdated Show resolved Hide resolved

lhunloh requested review from qedawkins and ftynse March 5, 2024 17:48

qedawkins approved these changes Mar 5, 2024

View reviewed changes

mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td Show resolved Hide resolved

lhunloh force-pushed the return_handles branch from 4e6a641 to 9d91b06 Compare March 5, 2024 18:08

lhunloh force-pushed the return_handles branch from 9d91b06 to cc8c765 Compare March 5, 2024 18:35

qedawkins merged commit 47bc565 into llvm:main Mar 6, 2024

lhunloh deleted the return_handles branch March 7, 2024 07:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MLIR] [Transforms] Let `transform.structured.convert_to_loops` return handles to loops #83984

[MLIR] [Transforms] Let `transform.structured.convert_to_loops` return handles to loops #83984

lhunloh commented Mar 5, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Mar 5, 2024

Uh oh!

llvmbot commented Mar 5, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Mar 5, 2024 •

edited

Loading

Uh oh!

lhunloh commented Mar 5, 2024

Uh oh!

Uh oh!

ftynse left a comment

Uh oh!

Uh oh!

lhunloh commented Mar 5, 2024

Uh oh!

qedawkins left a comment

Uh oh!

Uh oh!

lhunloh commented Mar 5, 2024

Uh oh!

github-actions bot commented Mar 6, 2024

Uh oh!

Uh oh!

[MLIR] [Transforms] Let transform.structured.convert_to_loops return handles to loops #83984

[MLIR] [Transforms] Let transform.structured.convert_to_loops return handles to loops #83984

Conversation

lhunloh commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 5, 2024

Uh oh!

llvmbot commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhunloh commented Mar 5, 2024

Uh oh!

Uh oh!

ftynse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lhunloh commented Mar 5, 2024

Uh oh!

qedawkins left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lhunloh commented Mar 5, 2024

Uh oh!

github-actions bot commented Mar 6, 2024

Uh oh!

Uh oh!

[MLIR] [Transforms] Let `transform.structured.convert_to_loops` return handles to loops #83984

[MLIR] [Transforms] Let `transform.structured.convert_to_loops` return handles to loops #83984

lhunloh commented Mar 5, 2024 •

edited

Loading

llvmbot commented Mar 5, 2024 •

edited

Loading

github-actions bot commented Mar 5, 2024 •

edited

Loading