From 43c60fbeefc198eea8c426a9c7a71221d3e9f1a6 Mon Sep 17 00:00:00 2001 From: Matthias Springer Date: Fri, 16 Aug 2024 20:40:39 +0200 Subject: [PATCH] [mlir][Transforms][WIP] Dialect conversion: Fix bug in `computeNecessaryMaterializations` There was a typo in the code path that removes unnecessary materializations. Before: Update `opResult` (result of an op different from `user`) in mapping and remove `user`. ``` replaceMaterialization(rewriterImpl, opResult, inputOperands, inverseMapping); necessaryMaterializations.remove(materializationOps.lookup(user)); ``` After: Update `user->getResults()` in mapping and remove `user`. ``` replaceMaterialization(rewriterImpl, user->getResults(), inputOperands, inverseMapping); necessaryMaterializations.remove(materializationOps.lookup(user)); ``` TODO: test case --- mlir/lib/Transforms/Utils/DialectConversion.cpp | 2 +- mlir/test/Transforms/test-legalize-type-conversion.mlir | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp index 11e593cebc09b..8a4c7463a69a9 100644 --- a/mlir/lib/Transforms/Utils/DialectConversion.cpp +++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp @@ -2680,7 +2680,7 @@ static void computeNecessaryMaterializations( if (!castOp) continue; if (castOp->getResultTypes() == inputOperands.getTypes()) { - replaceMaterialization(rewriterImpl, opResult, inputOperands, + replaceMaterialization(rewriterImpl, user->getResults(), inputOperands, inverseMapping); necessaryMaterializations.remove(materializationOps.lookup(user)); } diff --git a/mlir/test/Transforms/test-legalize-type-conversion.mlir b/mlir/test/Transforms/test-legalize-type-conversion.mlir index 252b990210a18..cf2c9f6a8ec44 100644 --- a/mlir/test/Transforms/test-legalize-type-conversion.mlir +++ b/mlir/test/Transforms/test-legalize-type-conversion.mlir @@ -99,6 +99,7 @@ func.func @test_block_argument_not_converted() { 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-note@below {{see existing live user here}} ^bb0(%arg0: f32): "test.type_consumer"(%arg0) : (f32) -> () "test.return"(%arg0) : (f32) -> ()