[Mlir-commits] [mlir] [mlir] fix a segment fault in `ConversionPatternRewriter::applySignatureConversion` (PR #82530)

Chao Chen llvmlistbot at llvm.org
Mon Feb 26 14:03:25 PST 2024


https://github.com/chencha3 updated https://github.com/llvm/llvm-project/pull/82530

>From 30ba1d56c713b3d56a6ebee2424eeb9764c58e69 Mon Sep 17 00:00:00 2001
From: Chao Chen <chao.chen at intel.com>
Date: Wed, 21 Feb 2024 14:14:02 -0600
Subject: [PATCH 1/3] fix a segment fault

---
 mlir/lib/Transforms/Utils/DialectConversion.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 4989ddc3ec94fb..b5cc8ca223d1af 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -642,8 +642,11 @@ Block *ArgConverter::applySignatureConversion(
 
       // Legalize the argument output type.
       Type outputType = origOutputType;
-      if (Type legalOutputType = converter->convertType(outputType))
-        outputType = legalOutputType;
+      if (converter){
+        if (Type legalOutputType = converter->convertType(outputType))
+          outputType = legalOutputType;
+      }
+        
 
       newArg = buildUnresolvedArgumentMaterialization(
           rewriter, origArg.getLoc(), replArgs, origOutputType, outputType,

>From e42cd2f37056e3e9abf437438644cf23240c9723 Mon Sep 17 00:00:00 2001
From: Chao Chen <chao.chen at intel.com>
Date: Wed, 21 Feb 2024 16:05:58 -0600
Subject: [PATCH 2/3] run code formatter

---
 mlir/lib/Transforms/Utils/DialectConversion.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index b5cc8ca223d1af..84eb352cc7ae8c 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -642,11 +642,10 @@ Block *ArgConverter::applySignatureConversion(
 
       // Legalize the argument output type.
       Type outputType = origOutputType;
-      if (converter){
+      if (converter) {
         if (Type legalOutputType = converter->convertType(outputType))
           outputType = legalOutputType;
       }
-        
 
       newArg = buildUnresolvedArgumentMaterialization(
           rewriter, origArg.getLoc(), replArgs, origOutputType, outputType,

>From 489af22175c08c9885cca6ca5cc7cf8d66edd49d Mon Sep 17 00:00:00 2001
From: Chao Chen <chao.chen at intel.com>
Date: Mon, 26 Feb 2024 16:03:13 -0600
Subject: [PATCH 3/3] add testcase

---
 .../test-legalize-type-conversion.mlir        | 22 +++++-----
 mlir/test/lib/Dialect/Test/TestOps.td         |  5 +++
 mlir/test/lib/Dialect/Test/TestPatterns.cpp   | 40 +++++++++++++++++++
 3 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/mlir/test/Transforms/test-legalize-type-conversion.mlir b/mlir/test/Transforms/test-legalize-type-conversion.mlir
index b35cda8e724f6b..61feea2286f2ba 100644
--- a/mlir/test/Transforms/test-legalize-type-conversion.mlir
+++ b/mlir/test/Transforms/test-legalize-type-conversion.mlir
@@ -30,16 +30,6 @@ func.func @test_invalid_result_materialization() {
 
 // -----
 
-func.func @test_invalid_result_materialization() {
-  // expected-error at below {{failed to materialize conversion for result #0 of operation 'test.type_producer' that remained live after conversion}}
-  %result = "test.type_producer"() : () -> f16
-
-  // expected-note at below {{see existing live user here}}
-  "foo.return"(%result) : (f16) -> ()
-}
-
-// -----
-
 // CHECK-LABEL: @test_transitive_use_materialization
 func.func @test_transitive_use_materialization() {
   // CHECK: %[[V:.*]] = "test.type_producer"() : () -> f64
@@ -101,6 +91,18 @@ func.func @test_block_argument_not_converted() {
 
 // -----
 
+// Make sure OneToN path of ArgConverter::applySignatureConversion can execute without converter
+func.func @test_one_to_n_signature_conversion_no_converter() {
+  "test.one_to_n_signature_conversion_no_converter"() ({
+  // CHECK: ^{{.*}}(%{{.*}}: f64, %{{.*}}: f64):
+  ^bb0(%arg0: vector<2xf64>):
+    "test.return"() : () -> ()
+  }) : () -> ()
+  return
+}
+
+// -----
+
 // Make sure argument type changes aren't implicitly forwarded.
 func.func @test_signature_conversion_no_converter() {
   "test.signature_conversion_no_converter"() ({
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 91ce0af9cd7fd5..f4c676e0dd695e 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -1967,6 +1967,11 @@ def TestSignatureConversionNoConverterOp
   let regions = (region AnyRegion);
 }
 
+def TestOneToNSignatureConversionNoConverterOp
+  : TEST_Op<"one_to_n_signature_conversion_no_converter"> {
+  let regions = (region AnyRegion);
+}
+
 //===----------------------------------------------------------------------===//
 // Test parser.
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index 108cfe8950ef67..e3c7565898d14d 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -17,6 +17,7 @@
 #include "mlir/Transforms/DialectConversion.h"
 #include "mlir/Transforms/FoldUtils.h"
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "mlir/Transforms/OneToNTypeConversion.h"
 #include "llvm/ADT/ScopeExit.h"
 
 using namespace mlir;
@@ -1464,6 +1465,34 @@ struct TestSignatureConversionUndo
   }
 };
 
+
+struct TestTestOneToNSignatureConversionNoConverter
+    : public OpConversionPattern<TestOneToNSignatureConversionNoConverterOp> {
+  TestTestOneToNSignatureConversionNoConverter(const TypeConverter &converter,
+                                               MLIRContext *context)
+      : OpConversionPattern<TestOneToNSignatureConversionNoConverterOp>(context),
+        converter(converter) {}
+
+  LogicalResult
+  matchAndRewrite(TestOneToNSignatureConversionNoConverterOp op, OpAdaptor adaptor,
+                  ConversionPatternRewriter &rewriter) const final {
+    Region &region = op->getRegion(0);
+    Block *entry = &region.front();
+
+    // Convert the original entry arguments.
+    auto argTys = entry->getArgumentTypes();
+    mlir::OneToNTypeMapping argMap(argTys);
+    if (failed(converter.convertSignatureArgs(argTys, argMap)))
+      return failure();
+
+    rewriter.modifyOpInPlace(op, 
+            [&]{ rewriter.applySignatureConversion(&region, argMap); });
+    return success();
+  }
+
+  const TypeConverter &converter;
+};
+
 /// Call signature conversion without providing a type converter to handle
 /// materializations.
 struct TestTestSignatureConversionNoConverter
@@ -1586,6 +1615,12 @@ struct TestTypeConversionDriver
           results.push_back(result);
           return success();
         });
+    converter.addConversion(
+        [&](VectorType type, 
+            SmallVectorImpl<Type> &results) -> std::optional<LogicalResult> {
+          results = SmallVector<Type>(type.getNumElements(), type.getElementType());
+          return success();
+        });
 
     /// Add the legal set of type materializations.
     converter.addSourceMaterialization([](OpBuilder &builder, Type resultType,
@@ -1627,11 +1662,16 @@ struct TestTypeConversionDriver
         [&](TestSignatureConversionNoConverterOp op) {
           return converter.isLegal(op.getRegion().front().getArgumentTypes());
         });
+    target.addDynamicallyLegalOp<TestOneToNSignatureConversionNoConverterOp>(
+        [&](TestOneToNSignatureConversionNoConverterOp op) {
+          return converter.isLegal(op.getRegion().front().getArgumentTypes());
+        });
 
     // Initialize the set of rewrite patterns.
     RewritePatternSet patterns(&getContext());
     patterns.add<TestTypeConsumerForward, TestTypeConversionProducer,
                  TestSignatureConversionUndo,
+                 TestTestOneToNSignatureConversionNoConverter,
                  TestTestSignatureConversionNoConverter>(converter,
                                                          &getContext());
     patterns.add<TestTypeConversionAnotherProducer>(&getContext());



More information about the Mlir-commits mailing list