[Mlir-commits] [mlir] [mlir][Transforms][NFC] Move rollback tests to separate file (PR #151809)

Matthias Springer llvmlistbot at llvm.org
Sat Aug 2 01:55:22 PDT 2025


https://github.com/matthias-springer created https://github.com/llvm/llvm-project/pull/151809

Move dialect conversion tests that require a rollback to a separate file. This is in preparation of the One-Shot Dialect Conversion refactoring, which does no longer support rollbacks.


>From 21dee12cbf9ca750d4f8efffaf7fdcfef831fa99 Mon Sep 17 00:00:00 2001
From: Matthias Springer <me at m-sp.org>
Date: Sat, 2 Aug 2025 08:54:09 +0000
Subject: [PATCH] [mlir][Transforms][NFC] Move rollback tests to separate file

---
 .../test-legalizer-full-rollback.mlir         |  38 ++++
 mlir/test/Transforms/test-legalizer-full.mlir |  84 ++-------
 .../Transforms/test-legalizer-rollback.mlir   | 163 ++++++++++++++++++
 mlir/test/Transforms/test-legalizer.mlir      | 142 ---------------
 4 files changed, 211 insertions(+), 216 deletions(-)
 create mode 100644 mlir/test/Transforms/test-legalizer-full-rollback.mlir
 create mode 100644 mlir/test/Transforms/test-legalizer-rollback.mlir

diff --git a/mlir/test/Transforms/test-legalizer-full-rollback.mlir b/mlir/test/Transforms/test-legalizer-full-rollback.mlir
new file mode 100644
index 0000000000000..e81b38d1459cc
--- /dev/null
+++ b/mlir/test/Transforms/test-legalizer-full-rollback.mlir
@@ -0,0 +1,38 @@
+// RUN: mlir-opt -allow-unregistered-dialect -test-legalize-patterns="test-legalize-mode=full" -split-input-file -verify-diagnostics %s | FileCheck %s
+
+// Test that region inlining can be properly undone.
+
+// expected-remark at +1 {{applyFullConversion failed}}
+builtin.module {
+func.func @test_undo_region_inline() {
+  "test.region"() ({
+    ^bb1(%i0: i64):
+      // expected-error at +1 {{failed to legalize operation 'cf.br'}}
+      cf.br ^bb2(%i0 : i64)
+    ^bb2(%i1: i64):
+      "test.invalid"(%i1) : (i64) -> ()
+  }) {} : () -> ()
+
+  "test.return"() : () -> ()
+}
+}
+
+// -----
+
+// Test that multiple block erases can be properly undone.
+
+// expected-remark at +1 {{applyFullConversion failed}}
+builtin.module {
+func.func @test_undo_block_erase() {
+  // expected-error at +1 {{failed to legalize operation 'test.region'}}
+  "test.region"() ({
+    ^bb1(%i0: i64):
+      cf.br ^bb3(%i0 : i64)
+    ^bb2(%i1: i64):
+      "test.invalid"(%i1) : (i64) -> ()
+    ^bb3(%i2: i64):
+      cf.br ^bb2(%i2 : i64)
+  }) {legalizer.should_clone, legalizer.erase_old_blocks} : () -> ()
+  "test.return"() : () -> ()
+}
+}
diff --git a/mlir/test/Transforms/test-legalizer-full.mlir b/mlir/test/Transforms/test-legalizer-full.mlir
index dcd01728f4357..42cec68b9fbbb 100644
--- a/mlir/test/Transforms/test-legalizer-full.mlir
+++ b/mlir/test/Transforms/test-legalizer-full.mlir
@@ -9,6 +9,8 @@ func.func @multi_level_mapping() {
   "test.return"() : () -> ()
 }
 
+// -----
+
 // Test that operations that are erased don't need to be legalized.
 // CHECK-LABEL: func @dropped_region_with_illegal_ops
 func.func @dropped_region_with_illegal_ops() {
@@ -19,6 +21,9 @@ func.func @dropped_region_with_illegal_ops() {
   }) : () -> ()
   "test.return"() : () -> ()
 }
+
+// -----
+
 // CHECK-LABEL: func @replace_non_root_illegal_op
 func.func @replace_non_root_illegal_op() {
   // CHECK-NEXT: "test.legal_op_b"
@@ -30,15 +35,20 @@ func.func @replace_non_root_illegal_op() {
 // -----
 
 // Test that children of recursively legal operations are ignored.
+
+// CHECK-LABEL: func @recursively_legal_invalid_op
 func.func @recursively_legal_invalid_op() {
   /// Operation that is statically legal.
   builtin.module attributes {test.recursively_legal} {
+    // CHECK: "test.illegal_op_f"
     %ignored = "test.illegal_op_f"() : () -> (i32)
   }
   /// Operation that is dynamically legal, i.e. the function has a pattern
   /// applied to legalize the argument type before it becomes recursively legal.
   builtin.module {
+    // CHECK: func @dynamic_func(%{{.*}}: f64)
     func.func @dynamic_func(%arg: i64) attributes {test.recursively_legal} {
+      // CHECK: "test.illegal_op_f"
       %ignored = "test.illegal_op_f"() : () -> (i32)
       "test.return"() : () -> ()
     }
@@ -49,25 +59,6 @@ func.func @recursively_legal_invalid_op() {
 
 // -----
 
-// expected-remark at +1 {{applyFullConversion failed}}
-builtin.module {
-
-  // Test that region cloning can be properly undone.
-  func.func @test_undo_region_clone() {
-    "test.region"() ({
-      ^bb1(%i0: i64):
-        "test.invalid"(%i0) : (i64) -> ()
-    }) {legalizer.should_clone} : () -> ()
-
-    // expected-error at +1 {{failed to legalize operation 'test.illegal_op_f'}}
-    %ignored = "test.illegal_op_f"() : () -> (i32)
-    "test.return"() : () -> ()
-  }
-
-}
-
-// -----
-
 // expected-remark at +1 {{applyFullConversion failed}}
 builtin.module {
 
@@ -81,58 +72,3 @@ builtin.module {
   }
 
 }
-
-// -----
-
-// expected-remark at +1 {{applyFullConversion failed}}
-builtin.module {
-
-  // Test that region inlining can be properly undone.
-  func.func @test_undo_region_inline() {
-    "test.region"() ({
-      ^bb1(%i0: i64):
-        // expected-error at +1 {{failed to legalize operation 'cf.br'}}
-        cf.br ^bb2(%i0 : i64)
-      ^bb2(%i1: i64):
-        "test.invalid"(%i1) : (i64) -> ()
-    }) {} : () -> ()
-
-    "test.return"() : () -> ()
-  }
-
-}
-
-// -----
-
-// expected-remark at +1 {{applyFullConversion failed}}
-builtin.module {
-
-  // Test that multiple block erases can be properly undone.
-  func.func @test_undo_block_erase() {
-    // expected-error at +1 {{failed to legalize operation 'test.region'}}
-    "test.region"() ({
-      ^bb1(%i0: i64):
-        cf.br ^bb3(%i0 : i64)
-      ^bb2(%i1: i64):
-        "test.invalid"(%i1) : (i64) -> ()
-      ^bb3(%i2: i64):
-        cf.br ^bb2(%i2 : i64)
-    }) {legalizer.should_clone, legalizer.erase_old_blocks} : () -> ()
-
-    "test.return"() : () -> ()
-  }
-
-}
-
-// -----
-
-// expected-remark at +1 {{applyFullConversion failed}}
-builtin.module {
-
-  func.func @create_unregistered_op_in_pattern() -> i32 {
-    // expected-error at +1 {{failed to legalize operation 'test.illegal_op_g'}}
-    %0 = "test.illegal_op_g"() : () -> (i32)
-    "test.return"(%0) : (i32) -> ()
-  }
-
-}
diff --git a/mlir/test/Transforms/test-legalizer-rollback.mlir b/mlir/test/Transforms/test-legalizer-rollback.mlir
new file mode 100644
index 0000000000000..460911fd88ad1
--- /dev/null
+++ b/mlir/test/Transforms/test-legalizer-rollback.mlir
@@ -0,0 +1,163 @@
+// RUN: mlir-opt -allow-unregistered-dialect -split-input-file -test-legalize-patterns -verify-diagnostics -profile-actions-to=- %s | FileCheck %s
+
+// expected-remark at +1 {{applyPartialConversion failed}}
+module {
+func.func @fail_to_convert_illegal_op_in_region() {
+  // expected-error at +1 {{failed to legalize operation 'test.region_builder'}}
+  "test.region_builder"() : () -> ()
+  return
+}
+}
+
+// -----
+
+// Check that the entry block arguments of a region are untouched in the case
+// of failure.
+
+// expected-remark at +1 {{applyPartialConversion failed}}
+module {
+func.func @fail_to_convert_region() {
+  // CHECK: "test.region"
+  // CHECK-NEXT: ^bb{{.*}}(%{{.*}}: i64):
+  "test.region"() ({
+    ^bb1(%i0: i64):
+      // expected-error at +1 {{failed to legalize operation 'test.region_builder'}}
+      "test.region_builder"() : () -> ()
+      "test.valid"() : () -> ()
+  }) : () -> ()
+  return
+}
+}
+
+// -----
+
+// CHECK-LABEL: @create_illegal_block
+func.func @create_illegal_block() {
+  // Check that we can undo block creation, i.e. that the block was removed.
+  // CHECK: test.create_illegal_block
+  // CHECK-NOT: ^{{.*}}(%{{.*}}: i32, %{{.*}}: i32):
+  // expected-remark at +1 {{op 'test.create_illegal_block' is not legalizable}}
+  "test.create_illegal_block"() : () -> ()
+
+  // expected-remark at +1 {{op 'func.return' is not legalizable}}
+  return
+}
+
+// -----
+
+// CHECK-LABEL: @undo_block_arg_replace
+// expected-remark at +1{{applyPartialConversion failed}}
+module {
+func.func @undo_block_arg_replace() {
+  // expected-error at +1{{failed to legalize operation 'test.block_arg_replace' that was explicitly marked illegal}}
+  "test.block_arg_replace"() ({
+  ^bb0(%arg0: i32, %arg1: i16):
+    // CHECK: ^bb0(%[[ARG0:.*]]: i32, %[[ARG1:.*]]: i16):
+    // CHECK-NEXT: "test.return"(%[[ARG0]]) : (i32)
+
+    "test.return"(%arg0) : (i32) -> ()
+  }) {trigger_rollback} : () -> ()
+  return
+}
+}
+
+// -----
+
+// The op in this function is rewritten to itself (and thus remains illegal) by
+// a pattern that removes its second block after adding an operation into it.
+// Check that we can undo block removal successfully.
+// CHECK-LABEL: @undo_block_erase
+func.func @undo_block_erase() {
+  // CHECK: test.undo_block_erase
+  "test.undo_block_erase"() ({
+    // expected-remark at -1 {{not legalizable}}
+    // CHECK: "unregistered.return"()[^[[BB:.*]]]
+    "unregistered.return"()[^bb1] : () -> ()
+    // expected-remark at -1 {{not legalizable}}
+  // CHECK: ^[[BB]]
+  ^bb1:
+    // CHECK: unregistered.return
+    "unregistered.return"() : () -> ()
+    // expected-remark at -1 {{not legalizable}}
+  }) : () -> ()
+}
+
+// -----
+
+// The op in this function is attempted to be rewritten to another illegal op
+// with an attached region containing an invalid terminator. The terminator is
+// created before the parent op. The deletion should not crash when deleting
+// created ops in the inverse order, i.e. deleting the parent op and then the
+// child op.
+// CHECK-LABEL: @undo_child_created_before_parent
+func.func @undo_child_created_before_parent() {
+  // expected-remark at +1 {{is not legalizable}}
+  "test.illegal_op_with_region_anchor"() : () -> ()
+  // expected-remark at +1 {{op 'func.return' is not legalizable}}
+  return
+}
+
+// -----
+
+// expected-remark at +1 {{applyPartialConversion failed}}
+builtin.module {
+func.func @create_unregistered_op_in_pattern() -> i32 {
+  // expected-error at +1 {{failed to legalize operation 'test.illegal_op_g'}}
+  %0 = "test.illegal_op_g"() : () -> (i32)
+  "test.return"(%0) : (i32) -> ()
+}
+}
+
+// -----
+
+// CHECK-LABEL: func @test_move_op_before_rollback()
+func.func @test_move_op_before_rollback() {
+  // CHECK: "test.one_region_op"()
+  // CHECK: "test.hoist_me"()
+  "test.one_region_op"() ({
+    // expected-remark @below{{'test.hoist_me' is not legalizable}}
+    %0 = "test.hoist_me"() : () -> (i32)
+    "test.valid"(%0) : (i32) -> ()
+  }) : () -> ()
+  "test.return"() : () -> ()
+}
+
+// -----
+
+// CHECK-LABEL: func @test_properties_rollback()
+func.func @test_properties_rollback() {
+  // CHECK: test.with_properties a = 32,
+  // expected-remark @below{{op 'test.with_properties' is not legalizable}}
+  test.with_properties
+      a = 32, b = "foo", c = "bar", flag = true, array = [1, 2, 3, 4], array32 = [5, 6]
+      {modify_inplace}
+  "test.return"() : () -> ()
+}
+
+// -----
+
+// expected-remark at +1 {{applyPartialConversion failed}}
+builtin.module {
+// Test that region cloning can be properly undone.
+func.func @test_undo_region_clone() {
+  "test.region"() ({
+    ^bb1(%i0: i64):
+      "test.invalid"(%i0) : (i64) -> ()
+  }) {legalizer.should_clone} : () -> ()
+
+  // expected-error at +1 {{failed to legalize operation 'test.illegal_op_f'}}
+  %ignored = "test.illegal_op_f"() : () -> (i32)
+  "test.return"() : () -> ()
+}
+}
+
+// -----
+
+// expected-remark at +1 {{applyPartialConversion failed}}
+builtin.module {
+func.func @create_unregistered_op_in_pattern() -> i32 {
+  // expected-error at +1 {{failed to legalize operation 'test.illegal_op_g'}}
+  %0 = "test.illegal_op_g"() : () -> (i32)
+  "test.return"(%0) : (i32) -> ()
+}
+}
diff --git a/mlir/test/Transforms/test-legalizer.mlir b/mlir/test/Transforms/test-legalizer.mlir
index 68c863cff69bf..e4406e60ffead 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -258,73 +258,6 @@ builtin.module {
 
 // -----
 
-// expected-remark at +1 {{applyPartialConversion failed}}
-builtin.module {
-
-  func.func @fail_to_convert_illegal_op_in_region() {
-    // expected-error at +1 {{failed to legalize operation 'test.region_builder'}}
-    "test.region_builder"() : () -> ()
-    return
-  }
-
-}
-
-// -----
-
-// Check that the entry block arguments of a region are untouched in the case
-// of failure.
-
-// expected-remark at +1 {{applyPartialConversion failed}}
-builtin.module {
-
-  func.func @fail_to_convert_region() {
-    // CHECK: "test.region"
-    // CHECK-NEXT: ^bb{{.*}}(%{{.*}}: i64):
-    "test.region"() ({
-      ^bb1(%i0: i64):
-        // expected-error at +1 {{failed to legalize operation 'test.region_builder'}}
-        "test.region_builder"() : () -> ()
-        "test.valid"() : () -> ()
-    }) : () -> ()
-    return
-  }
-
-}
-
-// -----
-
-// CHECK-LABEL: @create_illegal_block
-func.func @create_illegal_block() {
-  // Check that we can undo block creation, i.e. that the block was removed.
-  // CHECK: test.create_illegal_block
-  // CHECK-NOT: ^{{.*}}(%{{.*}}: i32, %{{.*}}: i32):
-  // expected-remark at +1 {{op 'test.create_illegal_block' is not legalizable}}
-  "test.create_illegal_block"() : () -> ()
-
-  // expected-remark at +1 {{op 'func.return' is not legalizable}}
-  return
-}
-
-// -----
-
-// CHECK-LABEL: @undo_block_arg_replace
-// expected-remark at +1{{applyPartialConversion failed}}
-module {
-func.func @undo_block_arg_replace() {
-  // expected-error at +1{{failed to legalize operation 'test.block_arg_replace' that was explicitly marked illegal}}
-  "test.block_arg_replace"() ({
-  ^bb0(%arg0: i32, %arg1: i16):
-    // CHECK: ^bb0(%[[ARG0:.*]]: i32, %[[ARG1:.*]]: i16):
-    // CHECK-NEXT: "test.return"(%[[ARG0]]) : (i32)
-
-    "test.return"(%arg0) : (i32) -> ()
-  }) {trigger_rollback} : () -> ()
-  return
-}
-}
-
-// -----
-
 // CHECK-LABEL: @replace_block_arg_1_to_n
 func.func @replace_block_arg_1_to_n() {
   // CHECK: "test.block_arg_replace"
@@ -340,42 +273,6 @@ func.func @replace_block_arg_1_to_n() {
 
 // -----
 
-// The op in this function is rewritten to itself (and thus remains illegal) by
-// a pattern that removes its second block after adding an operation into it.
-// Check that we can undo block removal successfully.
-// CHECK-LABEL: @undo_block_erase
-func.func @undo_block_erase() {
-  // CHECK: test.undo_block_erase
-  "test.undo_block_erase"() ({
-    // expected-remark at -1 {{not legalizable}}
-    // CHECK: "unregistered.return"()[^[[BB:.*]]]
-    "unregistered.return"()[^bb1] : () -> ()
-    // expected-remark at -1 {{not legalizable}}
-  // CHECK: ^[[BB]]
-  ^bb1:
-    // CHECK: unregistered.return
-    "unregistered.return"() : () -> ()
-    // expected-remark at -1 {{not legalizable}}
-  }) : () -> ()
-}
-
-// -----
-
-// The op in this function is attempted to be rewritten to another illegal op
-// with an attached region containing an invalid terminator. The terminator is
-// created before the parent op. The deletion should not crash when deleting
-// created ops in the inverse order, i.e. deleting the parent op and then the
-// child op.
-// CHECK-LABEL: @undo_child_created_before_parent
-func.func @undo_child_created_before_parent() {
-  // expected-remark at +1 {{is not legalizable}}
-  "test.illegal_op_with_region_anchor"() : () -> ()
-  // expected-remark at +1 {{op 'func.return' is not legalizable}}
-  return
-}
-
-// -----
-
 // Check that a conversion pattern on `test.blackhole` can mark the producer
 // for deletion.
 // CHECK-LABEL: @blackhole
@@ -388,19 +285,6 @@ func.func @blackhole() {
 
 // -----
 
-// expected-remark at +1 {{applyPartialConversion failed}}
-builtin.module {
-
-  func.func @create_unregistered_op_in_pattern() -> i32 {
-    // expected-error at +1 {{failed to legalize operation 'test.illegal_op_g'}}
-    %0 = "test.illegal_op_g"() : () -> (i32)
-    "test.return"(%0) : (i32) -> ()
-  }
-
-}
-
-// -----
-
 module {
 // CHECK-LABEL: func.func private @callee() -> (f16, f16)
 func.func private @callee() -> (f32, i24)
@@ -423,32 +307,6 @@ func.func @caller() {
 
 // -----
 
-// CHECK-LABEL: func @test_move_op_before_rollback()
-func.func @test_move_op_before_rollback() {
-  // CHECK: "test.one_region_op"()
-  // CHECK: "test.hoist_me"()
-  "test.one_region_op"() ({
-    // expected-remark @below{{'test.hoist_me' is not legalizable}}
-    %0 = "test.hoist_me"() : () -> (i32)
-    "test.valid"(%0) : (i32) -> ()
-  }) : () -> ()
-  "test.return"() : () -> ()
-}
-
-// -----
-
-// CHECK-LABEL: func @test_properties_rollback()
-func.func @test_properties_rollback() {
-  // CHECK: test.with_properties a = 32,
-  // expected-remark @below{{op 'test.with_properties' is not legalizable}}
-  test.with_properties
-      a = 32, b = "foo", c = "bar", flag = true, array = [1, 2, 3, 4], array32 = [5, 6]
-      {modify_inplace}
-  "test.return"() : () -> ()
-}
-
-// -----
-
 //      CHECK: func.func @use_of_replaced_bbarg(
 // CHECK-SAME:     %[[arg0:.*]]: f64)
 //      CHECK:   "test.valid"(%[[arg0]])



More information about the Mlir-commits mailing list