Skip to content

Conversation

@matthias-springer
Copy link
Member

@matthias-springer matthias-springer commented Oct 30, 2024

This commit changes the format of the materialization error message.

Previously: failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion
Now: failed to legalize unresolved materialization from ('f64') to ('f32') that remained live after conversion

This commit is in preparation of merging the 1:1 and 1:N dialect conversions. At that point, target materializations may create more than one SSA value. I am sending this change as a separate PR to keep the main PR smaller.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir mlir:bufferization Bufferization infrastructure labels Oct 30, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)

Changes

This commit changes the format of the materialization error message.

Previously: failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion
Now: failed to legalize unresolved materialization from ('f64') to ('f32') that remained live after conversion

This commit is in preparation of merging the 1:1 and 1:N dialect conversions. At that point, target materializations may create more than one SSA value. I am sending this change as a separate PR to keep the main PR smaller.


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

4 Files Affected:

  • (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+2-2)
  • (modified) mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir (+1-1)
  • (modified) mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir (+1-1)
  • (modified) mlir/test/Transforms/test-legalize-type-conversion.mlir (+5-5)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 44cf8331d55a73..fef7541f600db6 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -2460,8 +2460,8 @@ legalizeUnresolvedMaterialization(RewriterBase &rewriter,
   InFlightDiagnostic diag = op->emitError()
                             << "failed to legalize unresolved materialization "
                                "from ("
-                            << inputOperands.getTypes() << ") to " << outputType
-                            << " that remained live after conversion";
+                            << inputOperands.getTypes() << ") to (" << outputType
+                            << ") that remained live after conversion";
   diag.attachNote(op->getUsers().begin()->getLoc())
       << "see existing live user here: " << *op->getUsers().begin();
   return failure();
diff --git a/mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir b/mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir
index ab18ce05e355d3..bae94c1be4da90 100644
--- a/mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir
+++ b/mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir
@@ -78,7 +78,7 @@ func.func @static_layout_to_no_layout_cast(%m: memref<?xf32, strided<[1], offset
 // memref.cast.
 func.func @no_layout_to_dyn_layout_cast(%m: memref<?xf32>) -> memref<?xf32, strided<[1], offset: ?>> {
   %0 = bufferization.to_tensor %m : memref<?xf32>
-  // expected-error @+1 {{failed to legalize unresolved materialization from ('memref<?xf32>') to 'memref<?xf32, strided<[1], offset: ?>>' that remained live after conversion}}
+  // expected-error @+1 {{failed to legalize unresolved materialization from ('memref<?xf32>') to ('memref<?xf32, strided<[1], offset: ?>>') that remained live after conversion}}
   %1 = bufferization.to_memref %0 : memref<?xf32, strided<[1], offset: ?>>
   // expected-note @below{{see existing live user here}}
   return %1 : memref<?xf32, strided<[1], offset: ?>>
diff --git a/mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir b/mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir
index 6e8f0162e505d0..031442b0ee2daf 100644
--- a/mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir
+++ b/mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir
@@ -3,7 +3,7 @@
 // Test that an error is emitted when an operation is marked as "erased", but
 // has users that live across the conversion.
 func.func @remove_all_ops(%arg0: i32) -> i32 {
-  // expected-error@below {{failed to legalize unresolved materialization from () to 'i32' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from () to ('i32') that remained live after conversion}}
   %0 = "test.illegal_op_a"() : () -> i32
   // expected-note@below {{see existing live user here}}
   return %0 : i32
diff --git a/mlir/test/Transforms/test-legalize-type-conversion.mlir b/mlir/test/Transforms/test-legalize-type-conversion.mlir
index f130adff42f8cd..db8bd0f6378d29 100644
--- a/mlir/test/Transforms/test-legalize-type-conversion.mlir
+++ b/mlir/test/Transforms/test-legalize-type-conversion.mlir
@@ -2,7 +2,7 @@
 
 
 func.func @test_invalid_arg_materialization(
-  // expected-error@below {{failed to legalize unresolved materialization from () to 'i16' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from () to ('i16') that remained live after conversion}}
   %arg0: i16) {
   // expected-note@below{{see existing live user here}}
   "foo.return"(%arg0) : (i16) -> ()
@@ -21,7 +21,7 @@ func.func @test_valid_arg_materialization(%arg0: i64) {
 // -----
 
 func.func @test_invalid_result_materialization() {
-  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f16' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f16') that remained live after conversion}}
   %result = "test.type_producer"() : () -> f16
   // expected-note@below{{see existing live user here}}
   "foo.return"(%result) : (f16) -> ()
@@ -30,7 +30,7 @@ func.func @test_invalid_result_materialization() {
 // -----
 
 func.func @test_invalid_result_materialization() {
-  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f16' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f16') that remained live after conversion}}
   %result = "test.type_producer"() : () -> f16
   // expected-note@below{{see existing live user here}}
   "foo.return"(%result) : (f16) -> ()
@@ -50,7 +50,7 @@ func.func @test_transitive_use_materialization() {
 // -----
 
 func.func @test_transitive_use_invalid_materialization() {
-  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f16' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f16') that remained live after conversion}}
   %result = "test.another_type_producer"() : () -> f16
   // expected-note@below{{see existing live user here}}
   "foo.return"(%result) : (f16) -> ()
@@ -102,7 +102,7 @@ func.func @test_block_argument_not_converted() {
 // Make sure argument type changes aren't implicitly forwarded.
 func.func @test_signature_conversion_no_converter() {
   "test.signature_conversion_no_converter"() ({
-  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f32') that remained live after conversion}}
   ^bb0(%arg0: f32):
     "test.type_consumer"(%arg0) : (f32) -> ()
     // expected-note@below{{see existing live user here}}

@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2024

@llvm/pr-subscribers-mlir-bufferization

Author: Matthias Springer (matthias-springer)

Changes

This commit changes the format of the materialization error message.

Previously: failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion
Now: failed to legalize unresolved materialization from ('f64') to ('f32') that remained live after conversion

This commit is in preparation of merging the 1:1 and 1:N dialect conversions. At that point, target materializations may create more than one SSA value. I am sending this change as a separate PR to keep the main PR smaller.


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

4 Files Affected:

  • (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+2-2)
  • (modified) mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir (+1-1)
  • (modified) mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir (+1-1)
  • (modified) mlir/test/Transforms/test-legalize-type-conversion.mlir (+5-5)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 44cf8331d55a73..fef7541f600db6 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -2460,8 +2460,8 @@ legalizeUnresolvedMaterialization(RewriterBase &rewriter,
   InFlightDiagnostic diag = op->emitError()
                             << "failed to legalize unresolved materialization "
                                "from ("
-                            << inputOperands.getTypes() << ") to " << outputType
-                            << " that remained live after conversion";
+                            << inputOperands.getTypes() << ") to (" << outputType
+                            << ") that remained live after conversion";
   diag.attachNote(op->getUsers().begin()->getLoc())
       << "see existing live user here: " << *op->getUsers().begin();
   return failure();
diff --git a/mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir b/mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir
index ab18ce05e355d3..bae94c1be4da90 100644
--- a/mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir
+++ b/mlir/test/Dialect/Bufferization/Transforms/finalizing-bufferize.mlir
@@ -78,7 +78,7 @@ func.func @static_layout_to_no_layout_cast(%m: memref<?xf32, strided<[1], offset
 // memref.cast.
 func.func @no_layout_to_dyn_layout_cast(%m: memref<?xf32>) -> memref<?xf32, strided<[1], offset: ?>> {
   %0 = bufferization.to_tensor %m : memref<?xf32>
-  // expected-error @+1 {{failed to legalize unresolved materialization from ('memref<?xf32>') to 'memref<?xf32, strided<[1], offset: ?>>' that remained live after conversion}}
+  // expected-error @+1 {{failed to legalize unresolved materialization from ('memref<?xf32>') to ('memref<?xf32, strided<[1], offset: ?>>') that remained live after conversion}}
   %1 = bufferization.to_memref %0 : memref<?xf32, strided<[1], offset: ?>>
   // expected-note @below{{see existing live user here}}
   return %1 : memref<?xf32, strided<[1], offset: ?>>
diff --git a/mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir b/mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir
index 6e8f0162e505d0..031442b0ee2daf 100644
--- a/mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir
+++ b/mlir/test/Transforms/test-legalize-erased-op-with-uses.mlir
@@ -3,7 +3,7 @@
 // Test that an error is emitted when an operation is marked as "erased", but
 // has users that live across the conversion.
 func.func @remove_all_ops(%arg0: i32) -> i32 {
-  // expected-error@below {{failed to legalize unresolved materialization from () to 'i32' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from () to ('i32') that remained live after conversion}}
   %0 = "test.illegal_op_a"() : () -> i32
   // expected-note@below {{see existing live user here}}
   return %0 : i32
diff --git a/mlir/test/Transforms/test-legalize-type-conversion.mlir b/mlir/test/Transforms/test-legalize-type-conversion.mlir
index f130adff42f8cd..db8bd0f6378d29 100644
--- a/mlir/test/Transforms/test-legalize-type-conversion.mlir
+++ b/mlir/test/Transforms/test-legalize-type-conversion.mlir
@@ -2,7 +2,7 @@
 
 
 func.func @test_invalid_arg_materialization(
-  // expected-error@below {{failed to legalize unresolved materialization from () to 'i16' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from () to ('i16') that remained live after conversion}}
   %arg0: i16) {
   // expected-note@below{{see existing live user here}}
   "foo.return"(%arg0) : (i16) -> ()
@@ -21,7 +21,7 @@ func.func @test_valid_arg_materialization(%arg0: i64) {
 // -----
 
 func.func @test_invalid_result_materialization() {
-  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f16' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f16') that remained live after conversion}}
   %result = "test.type_producer"() : () -> f16
   // expected-note@below{{see existing live user here}}
   "foo.return"(%result) : (f16) -> ()
@@ -30,7 +30,7 @@ func.func @test_invalid_result_materialization() {
 // -----
 
 func.func @test_invalid_result_materialization() {
-  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f16' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f16') that remained live after conversion}}
   %result = "test.type_producer"() : () -> f16
   // expected-note@below{{see existing live user here}}
   "foo.return"(%result) : (f16) -> ()
@@ -50,7 +50,7 @@ func.func @test_transitive_use_materialization() {
 // -----
 
 func.func @test_transitive_use_invalid_materialization() {
-  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f16' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f16') that remained live after conversion}}
   %result = "test.another_type_producer"() : () -> f16
   // expected-note@below{{see existing live user here}}
   "foo.return"(%result) : (f16) -> ()
@@ -102,7 +102,7 @@ func.func @test_block_argument_not_converted() {
 // Make sure argument type changes aren't implicitly forwarded.
 func.func @test_signature_conversion_no_converter() {
   "test.signature_conversion_no_converter"() ({
-  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion}}
+  // expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f32') that remained live after conversion}}
   ^bb0(%arg0: f32):
     "test.type_consumer"(%arg0) : (f32) -> ()
     // expected-note@below{{see existing live user here}}

@github-actions
Copy link

github-actions bot commented Oct 30, 2024

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

This commit changes the format of the materialization error message.

Previously: `failed to legalize unresolved materialization from ('f64') to 'f32' that remained live after conversion`
Now: `failed to legalize unresolved materialization from ('f64') to ('f32') that remained live after conversion`

This commit is in preparation of merging the 1:1 and 1:N dialect conversions. At that point, target materializations may create more than one SSA value. Sending this change as a separate PR to keep the main PR small.
@matthias-springer matthias-springer force-pushed the users/matthias-springer/n_mat_error_msg branch from 875ea32 to 09f6208 Compare October 30, 2024 04:23
Copy link
Contributor

@ingomueller-net ingomueller-net left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@matthias-springer matthias-springer merged commit ea050ab into main Oct 30, 2024
8 checks passed
@matthias-springer matthias-springer deleted the users/matthias-springer/n_mat_error_msg branch October 30, 2024 12:36
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…error message (llvm#114176)

This commit changes the format of the materialization error message.

Previously: `failed to legalize unresolved materialization from ('f64')
to 'f32' that remained live after conversion`
Now: `failed to legalize unresolved materialization from ('f64') to
('f32') that remained live after conversion`

This commit is in preparation of merging the 1:1 and 1:N dialect
conversions. At that point, target materializations may create more than
one SSA value. I am sending this change as a separate PR to keep the
main PR smaller.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:bufferization Bufferization infrastructure mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants