[Mlir-commits] [mlir] [mlir][TD] update more tests to use the "main" interpreter pass (PR #76963)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Thu Jan 4 06:29:13 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Oleksandr "Alex" Zinenko (ftynse)

<details>
<summary>Changes</summary>

Update several tests under mlir/test/Dialect/Transform to use the "main" transform interpreter pass with named entry points rather than the test interpreter pass.

This helped discover a logic error in the expensive checks mechanism that was exiting too early.

---

Patch is 170.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76963.diff


5 Files Affected:

- (modified) mlir/lib/Dialect/Transform/IR/TransformInterfaces.cpp (+4-3) 
- (modified) mlir/test/Dialect/Transform/apply-foreach-nested.mlir (+16-10) 
- (modified) mlir/test/Dialect/Transform/expensive-checks.mlir (+216-179) 
- (modified) mlir/test/Dialect/Transform/selective-targeting.mlir (+58-46) 
- (modified) mlir/test/Dialect/Transform/test-interpreter.mlir (+1170-957) 


``````````diff
diff --git a/mlir/lib/Dialect/Transform/IR/TransformInterfaces.cpp b/mlir/lib/Dialect/Transform/IR/TransformInterfaces.cpp
index cd66a0e566f6cd..371ad904dcae5a 100644
--- a/mlir/lib/Dialect/Transform/IR/TransformInterfaces.cpp
+++ b/mlir/lib/Dialect/Transform/IR/TransformInterfaces.cpp
@@ -629,9 +629,6 @@ void transform::TransformState::recordOpHandleInvalidation(
   // the IR nested in each payload op associated with the given handle and look
   // for handles associated with each operation and value.
   for (const auto &[region, mapping] : llvm::reverse(mappings)) {
-    // Stop lookup when reaching a region that is isolated from above.
-    if (region->getParentOp()->hasTrait<OpTrait::IsIsolatedFromAbove>())
-      break;
     // Go over all op handle mappings and mark as invalidated any handle
     // pointing to any of the payload ops associated with the given handle or
     // any op nested in them.
@@ -652,6 +649,10 @@ void transform::TransformState::recordOpHandleInvalidation(
                                                    payloadValue, valueHandle,
                                                    newlyInvalidated);
     }
+
+    // Stop lookup when reaching a region that is isolated from above.
+    if (region->getParentOp()->hasTrait<OpTrait::IsIsolatedFromAbove>())
+      break;
   }
 }
 
diff --git a/mlir/test/Dialect/Transform/apply-foreach-nested.mlir b/mlir/test/Dialect/Transform/apply-foreach-nested.mlir
index d2f71644bbc514..23f45b97bc2f84 100644
--- a/mlir/test/Dialect/Transform/apply-foreach-nested.mlir
+++ b/mlir/test/Dialect/Transform/apply-foreach-nested.mlir
@@ -1,5 +1,5 @@
 // RUN: mlir-opt %s --split-input-file --verify-diagnostics \
-// RUN:          --pass-pipeline="builtin.module(test-transform-dialect-interpreter{enable-expensive-checks=1 bind-first-extra-to-ops=scf.for})"
+// RUN:             --transform-interpreter
 
 func.func private @bar()
 
@@ -17,12 +17,15 @@ func.func @foo() {
   return
 }
 
-transform.sequence failures(suppress) {
-^bb0(%arg0: !transform.any_op, %arg1: !transform.op<"scf.for">):
-  %1 = transform.test_reverse_payload_ops %arg1 : (!transform.op<"scf.for">) -> !transform.op<"scf.for">
-  // expected-error @below {{transform operation consumes a handle pointing to an ancestor payload operation before its descendant}}
-  // expected-note @below {{the ancestor is likely erased or rewritten before the descendant is accessed, leading to undefined behavior}}
-  transform.test_consume_operand_each %1 : !transform.op<"scf.for">
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    %0 = transform.structured.match ops{["scf.for"]} in %arg0 : (!transform.any_op) -> !transform.op<"scf.for">
+    %1 = transform.test_reverse_payload_ops %0 : (!transform.op<"scf.for">) -> !transform.op<"scf.for">
+    // expected-error @below {{transform operation consumes a handle pointing to an ancestor payload operation before its descendant}}
+    // expected-note @below {{the ancestor is likely erased or rewritten before the descendant is accessed, leading to undefined behavior}}
+    transform.test_consume_operand_each %1 : !transform.op<"scf.for">
+    transform.yield
+  }
 }
 
 // -----
@@ -42,7 +45,10 @@ func.func @foo() {
 }
 
 // No error here, processing ancestors before descendants.
-transform.sequence failures(suppress) {
-^bb0(%arg0: !transform.any_op, %arg1: !transform.op<"scf.for">):
-  transform.test_consume_operand_each %arg1 : !transform.op<"scf.for">
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    %0 = transform.structured.match ops{["scf.for"]} in %arg0 : (!transform.any_op) -> !transform.op<"scf.for">
+    transform.test_consume_operand_each %0 : !transform.op<"scf.for">
+    transform.yield
+  }
 }
diff --git a/mlir/test/Dialect/Transform/expensive-checks.mlir b/mlir/test/Dialect/Transform/expensive-checks.mlir
index ee9a5af8055247..b532e1892b1add 100644
--- a/mlir/test/Dialect/Transform/expensive-checks.mlir
+++ b/mlir/test/Dialect/Transform/expensive-checks.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt --test-transform-dialect-interpreter='enable-expensive-checks=1' --split-input-file --verify-diagnostics %s
+// RUN: mlir-opt --transform-interpreter --split-input-file --verify-diagnostics %s
 
 // expected-note @below {{ancestor payload op}}
 func.func @func() {
@@ -6,24 +6,29 @@ func.func @func() {
   return
 }
 
-transform.with_pdl_patterns {
-^bb0(%arg0: !transform.any_op):
-  pdl.pattern @return : benefit(1) {
-    %0 = operands
-    %1 = types
-    %2 = operation "func.return"(%0 : !pdl.range<value>) -> (%1 : !pdl.range<type>)
-    rewrite %2 with "transform.dialect"
-  }
-
-  sequence %arg0 : !transform.any_op failures(propagate) {
-  ^bb1(%arg1: !transform.any_op):
-    // expected-note @below {{handle to invalidated ops}}
-    %0 = pdl_match @return in %arg1 : (!transform.any_op) -> !transform.any_op
-    %1 = get_parent_op %0 {isolated_from_above} : (!transform.any_op) -> !transform.any_op
-    // expected-note @below {{invalidated by this transform op that consumes its operand #0}}
-    test_consume_operand %1 : !transform.any_op
-    // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
-    test_print_remark_at_operand %0, "remark" : !transform.any_op
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%root: !transform.any_op) {
+    transform.with_pdl_patterns %root : !transform.any_op {
+    ^bb0(%arg0: !transform.any_op):
+      pdl.pattern @return : benefit(1) {
+        %0 = operands
+        %1 = types
+        %2 = operation "func.return"(%0 : !pdl.range<value>) -> (%1 : !pdl.range<type>)
+        rewrite %2 with "transform.dialect"
+      }
+
+      sequence %arg0 : !transform.any_op failures(propagate) {
+      ^bb1(%arg1: !transform.any_op):
+        // expected-note @below {{handle to invalidated ops}}
+        %0 = pdl_match @return in %arg1 : (!transform.any_op) -> !transform.any_op
+        %1 = get_parent_op %0 {isolated_from_above} : (!transform.any_op) -> !transform.any_op
+        // expected-note @below {{invalidated by this transform op that consumes its operand #0}}
+        test_consume_operand %1 : !transform.any_op
+        // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
+        test_print_remark_at_operand %0, "remark" : !transform.any_op
+      }
+    }
+    transform.yield
   }
 }
 
@@ -35,29 +40,34 @@ func.func @func1() {
 }
 func.func private @func2()
 
-transform.with_pdl_patterns {
-^bb0(%arg0: !transform.any_op):
-  pdl.pattern @func : benefit(1) {
-    %0 = operands
-    %1 = types
-    %2 = operation "func.func"(%0 : !pdl.range<value>) -> (%1 : !pdl.range<type>)
-    rewrite %2 with "transform.dialect"
-  }
-  pdl.pattern @return : benefit(1) {
-    %0 = operands
-    %1 = types
-    %2 = operation "func.return"(%0 : !pdl.range<value>) -> (%1 : !pdl.range<type>)
-    rewrite %2 with "transform.dialect"
-  }
-
-  sequence %arg0 : !transform.any_op failures(propagate) {
-  ^bb1(%arg1: !transform.any_op):
-    %0 = pdl_match @func in %arg1 : (!transform.any_op) -> !transform.any_op
-    %1 = pdl_match @return in %arg1 : (!transform.any_op) -> !transform.any_op
-    %2 = replicate num(%0) %1 : !transform.any_op, !transform.any_op
-    // expected-error @below {{a handle passed as operand #0 and consumed by this operation points to a payload entity more than once}}
-    test_consume_operand %2 : !transform.any_op
-    test_print_remark_at_operand %0, "remark" : !transform.any_op
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%root: !transform.any_op) {
+    transform.with_pdl_patterns %root : !transform.any_op {
+    ^bb0(%arg0: !transform.any_op):
+      pdl.pattern @func : benefit(1) {
+        %0 = operands
+        %1 = types
+        %2 = operation "func.func"(%0 : !pdl.range<value>) -> (%1 : !pdl.range<type>)
+        rewrite %2 with "transform.dialect"
+      }
+      pdl.pattern @return : benefit(1) {
+        %0 = operands
+        %1 = types
+        %2 = operation "func.return"(%0 : !pdl.range<value>) -> (%1 : !pdl.range<type>)
+        rewrite %2 with "transform.dialect"
+      }
+
+      sequence %arg0 : !transform.any_op failures(propagate) {
+      ^bb1(%arg1: !transform.any_op):
+        %0 = pdl_match @func in %arg1 : (!transform.any_op) -> !transform.any_op
+        %1 = pdl_match @return in %arg1 : (!transform.any_op) -> !transform.any_op
+        %2 = replicate num(%0) %1 : !transform.any_op, !transform.any_op
+        // expected-error @below {{a handle passed as operand #0 and consumed by this operation points to a payload entity more than once}}
+        test_consume_operand %2 : !transform.any_op
+        test_print_remark_at_operand %0, "remark" : !transform.any_op
+      }
+    }
+    transform.yield
   }
 }
 
@@ -66,10 +76,9 @@ transform.with_pdl_patterns {
 
 // expected-note @below {{ancestor payload op}}
 // expected-note @below {{nested payload op}}
-module {
+module attributes {transform.with_named_sequence} {
 
-  transform.sequence failures(propagate) {
-  ^bb0(%0: !transform.any_op):
+  transform.named_sequence @__transform_main(%0: !transform.any_op) {
     %1 = transform.test_copy_payload %0 : (!transform.any_op) -> !transform.any_op
     // expected-note @below {{handle to invalidated ops}}
     %2 = transform.test_copy_payload %0 : (!transform.any_op) ->!transform.any_op
@@ -77,6 +86,7 @@ module {
     transform.test_consume_operand %1 : !transform.any_op
     // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
     transform.test_consume_operand %2 : !transform.any_op
+    transform.yield
   }
 }
 
@@ -84,10 +94,9 @@ module {
 
 // expected-note @below {{ancestor payload op}}
 // expected-note @below {{nested payload op}}
-module {
+module attributes {transform.with_named_sequence} {
 
-  transform.sequence failures(propagate) {
-  ^bb0(%0: !transform.any_op):
+  transform.named_sequence @__transform_main(%0: !transform.any_op) {
     %1 = transform.test_copy_payload %0 : (!transform.any_op) -> !transform.any_op
     // expected-note @below {{handle to invalidated ops}}
     %2 = transform.test_copy_payload %0 : (!transform.any_op) -> !transform.any_op
@@ -97,6 +106,7 @@ module {
     // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
     // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates all handles to payload IR entities}}
     transform.test_consume_operand %1, %2 : !transform.any_op, !transform.any_op
+    transform.yield
   }
 }
 
@@ -104,13 +114,13 @@ module {
 
 // Deduplication attribute allows "merge_handles" to take repeated operands.
 
-module {
+module attributes {transform.with_named_sequence} {
 
-  transform.sequence failures(propagate) {
-  ^bb0(%0: !transform.any_op):
+  transform.named_sequence @__transform_main(%0: !transform.any_op) {
     %1 = transform.test_copy_payload %0 : (!transform.any_op) -> !transform.any_op
     %2 = transform.test_copy_payload %0 : (!transform.any_op) -> !transform.any_op
     transform.merge_handles %1, %2 { deduplicate } : !transform.any_op
+    transform.yield
   }
 }
 // -----
@@ -118,16 +128,18 @@ module {
 // expected-note @below {{payload value}}
 %0 = "test.match_anchor"() : () -> (i32)
 
-transform.sequence failures(propagate) {
-^bb1(%arg0: !transform.any_op):
-  %2 = transform.structured.match ops{["test.match_anchor"]} in %arg0 : (!transform.any_op) -> !transform.any_op
-  %3 = test_produce_value_handle_to_result %2, 0 : (!transform.any_op) -> !transform.any_value
-  // expected-note @below {{invalidated handle}}
-  %4 = test_produce_value_handle_to_result %2, 0 : (!transform.any_op) -> !transform.any_value
-  // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates handles to the same values as associated with it}}
-  test_consume_operand %3 : !transform.any_value
-  // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
-  test_consume_operand %4 : !transform.any_value
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    %2 = transform.structured.match ops{["test.match_anchor"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    %3 = transform.test_produce_value_handle_to_result %2, 0 : (!transform.any_op) -> !transform.any_value
+    // expected-note @below {{invalidated handle}}
+    %4 = transform.test_produce_value_handle_to_result %2, 0 : (!transform.any_op) -> !transform.any_value
+    // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates handles to the same values as associated with it}}
+    transform.test_consume_operand %3 : !transform.any_value
+    // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
+    transform.test_consume_operand %4 : !transform.any_value
+    transform.yield
+  }
 }
 
 // -----
@@ -137,15 +149,17 @@ transform.sequence failures(propagate) {
 // expected-note @below {{op defining the value as result #0}}
 %0 = "test.match_anchor"() : () -> (i32)
 
-transform.sequence failures(propagate) {
-^bb1(%arg0: !transform.any_op):
-  %2 = transform.structured.match ops{["test.match_anchor"]} in %arg0 : (!transform.any_op) -> !transform.any_op
-  // expected-note @below {{invalidated handle}}
-  %3 = test_produce_value_handle_to_result %2, 0 : (!transform.any_op) -> !transform.any_value
-  // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates all handles to payload IR entities associated with this operand and entities nested in them}}
-  test_consume_operand %2 : !transform.any_op
-  // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
-  test_consume_operand %3 : !transform.any_value
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    %2 = transform.structured.match ops{["test.match_anchor"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    // expected-note @below {{invalidated handle}}
+    %3 = transform.test_produce_value_handle_to_result %2, 0 : (!transform.any_op) -> !transform.any_value
+    // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates all handles to payload IR entities associated with this operand and entities nested in them}}
+    transform.test_consume_operand %2 : !transform.any_op
+    // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
+    transform.test_consume_operand %3 : !transform.any_value
+    transform.yield
+  }
 }
 
 // -----
@@ -159,16 +173,18 @@ transform.sequence failures(propagate) {
   "test.region_terminator"() : () -> ()
 }) : () -> ()
 
-transform.sequence failures(propagate) {
-^bb1(%arg0: !transform.any_op):
-  %1 = transform.structured.match ops{["test.match_anchor_1"]} in %arg0 : (!transform.any_op) -> !transform.any_op
-  %2 = transform.structured.match ops{["test.match_anchor_2"]} in %arg0 : (!transform.any_op) -> !transform.any_op
-  // expected-note @below {{invalidated handle}}
-  %3 = test_produce_value_handle_to_result %2, 0 : (!transform.any_op) -> !transform.any_value
-  // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates all handles to payload IR entities associated with this operand and entities nested in them}}
-  test_consume_operand %1 : !transform.any_op
-  // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
-  test_consume_operand %3 : !transform.any_value
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    %1 = transform.structured.match ops{["test.match_anchor_1"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    %2 = transform.structured.match ops{["test.match_anchor_2"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    // expected-note @below {{invalidated handle}}
+    %3 = transform.test_produce_value_handle_to_result %2, 0 : (!transform.any_op) -> !transform.any_value
+    // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates all handles to payload IR entities associated with this operand and entities nested in them}}
+    transform.test_consume_operand %1 : !transform.any_op
+    // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
+    transform.test_consume_operand %3 : !transform.any_value
+    transform.yield
+  }
 }
 
 // -----
@@ -182,16 +198,18 @@ transform.sequence failures(propagate) {
   "test.region_terminator"() : () -> ()
 }) : () -> ()
 
-transform.sequence failures(propagate) {
-^bb1(%arg0: !transform.any_op):
-  %1 = transform.structured.match ops{["test.match_anchor_1"]} in %arg0 : (!transform.any_op) -> !transform.any_op
-  %2 = transform.structured.match ops{["test.match_anchor_2"]} in %arg0 : (!transform.any_op) -> !transform.any_op
-  // expected-note @below {{invalidated handle}}
-  %3 = test_produce_value_handle_to_argument_of_parent_block %2, 0 : (!transform.any_op) -> !transform.any_value
-  // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates all handles to payload IR entities associated with this operand and entities nested in them}}
-  test_consume_operand %1 : !transform.any_op
-  // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
-  test_consume_operand %3 : !transform.any_value
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    %1 = transform.structured.match ops{["test.match_anchor_1"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    %2 = transform.structured.match ops{["test.match_anchor_2"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    // expected-note @below {{invalidated handle}}
+    %3 = transform.test_produce_value_handle_to_argument_of_parent_block %2, 0 : (!transform.any_op) -> !transform.any_value
+    // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates all handles to payload IR entities associated with this operand and entities nested in them}}
+    transform.test_consume_operand %1 : !transform.any_op
+    // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
+    transform.test_consume_operand %3 : !transform.any_value
+    transform.yield
+  }
 }
 
 // -----
@@ -208,16 +226,18 @@ transform.sequence failures(propagate) {
   }): () -> ()
 }) : () -> ()
 
-transform.sequence failures(propagate) {
-^bb1(%arg0: !transform.any_op):
-  %1 = transform.structured.match ops{["test.match_anchor_1"]} in %arg0 : (!transform.any_op) -> !transform.any_op
-  %2 = transform.structured.match ops{["test.match_anchor_2"]} in %arg0 : (!transform.any_op) -> !transform.any_op
-  // expected-note @below {{invalidated handle}}
-  %3 = test_produce_value_handle_to_argument_of_parent_block %2, 0 : (!transform.any_op) -> !transform.any_value
-  // expected-note @below {{invalidated by this transform op that consumes its operand #0 and invalidates all handles to payload IR entities associated with this operand and entities nested in them}}
-  test_consume_operand %1 : !transform.any_op
-  // expected-error @below {{op uses a handle invalidated by a previously executed transform op}}
-  test_consume_operand %3 : !transform.any_value
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    %1 = transform.structured.match ops{["test.match_anchor_1"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    %2 = transform.structured.match ops{["test.match_anchor_2"]} in %arg0 : (!transform.any_op) -> !transform.any_op
+    // expected-note @below {{invalidated handle}}
+    %3 = transform.test_produce_value_handle_to_argument_of_parent_b...
[truncated]

``````````

</details>


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


More information about the Mlir-commits mailing list