[Mlir-commits] [mlir] [mlir] load dialects for non-namespaced attrs (PR #94838)
Jeremy Kun
llvmlistbot at llvm.org
Tue Jun 18 14:20:12 PDT 2024
https://github.com/j2kun updated https://github.com/llvm/llvm-project/pull/94838
>From d46c1bcbd6deeeeca48ebcf106cad752ff675d10 Mon Sep 17 00:00:00 2001
From: Jeremy Kun <j2kun at users.noreply.github.com>
Date: Fri, 7 Jun 2024 21:27:57 -0700
Subject: [PATCH 1/6] [mlir] load dialects for non-namespaced attrs
The mlir-translate tool calls into the parser without loading registered
dependent dialects, and the parser only loads attributes if the
fully-namespaced attribute is present in the textual IR. This causes
parsing to break when an op has an attribute that prints/parses
without the namespaced attribute.
---
mlir/test/Target/LLVMIR/test.mlir | 15 +++++++++++++++
mlir/test/lib/Dialect/Test/CMakeLists.txt | 5 +++--
mlir/test/lib/Dialect/Test/TestAttrDefs.td | 9 +++++++++
mlir/test/lib/Dialect/Test/TestAttributes.h | 1 +
mlir/test/lib/Dialect/Test/TestOps.td | 7 ++++++-
.../lib/Dialect/Test/TestToLLVMIRTranslation.cpp | 2 ++
6 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/mlir/test/Target/LLVMIR/test.mlir b/mlir/test/Target/LLVMIR/test.mlir
index 0ab1b7267d959..c7a28a2892da4 100644
--- a/mlir/test/Target/LLVMIR/test.mlir
+++ b/mlir/test/Target/LLVMIR/test.mlir
@@ -40,3 +40,18 @@ llvm.func @dialect_attr_translation_multi(%a: i64, %b: i64, %c: i64) -> i64 {
// CHECK-DAG: ![[MD_ID_ADD]] = !{!"annotation_from_test: add"}
// CHECK-DAG: ![[MD_ID_MUL]] = !{!"annotation_from_test: mul"}
// CHECK-DAG: ![[MD_ID_RET]] = !{!"annotation_from_test: ret"}
+
+
+// -----
+
+// This is a regression test for a bug where, during an mlir-translate call the
+// parser would only load the dialect if the fully namespaced attribute was
+// present in the IR.
+#attr = #test.nested_polynomial<<1 + x**2>>
+llvm.func @parse_correctly() {
+ // CHECK: <1 + x**2>
+ test.containing_int_polynomial_attr #attr
+
+ return
+}
+
diff --git a/mlir/test/lib/Dialect/Test/CMakeLists.txt b/mlir/test/lib/Dialect/Test/CMakeLists.txt
index fab8937809332..967101242e26b 100644
--- a/mlir/test/lib/Dialect/Test/CMakeLists.txt
+++ b/mlir/test/lib/Dialect/Test/CMakeLists.txt
@@ -16,8 +16,8 @@ mlir_tablegen(TestOpInterfaces.cpp.inc -gen-op-interface-defs)
add_public_tablegen_target(MLIRTestInterfaceIncGen)
set(LLVM_TARGET_DEFINITIONS TestOps.td)
-mlir_tablegen(TestAttrDefs.h.inc -gen-attrdef-decls)
-mlir_tablegen(TestAttrDefs.cpp.inc -gen-attrdef-defs)
+mlir_tablegen(TestAttrDefs.h.inc -gen-attrdef-decls -attrdefs-dialect=test)
+mlir_tablegen(TestAttrDefs.cpp.inc -gen-attrdef-defs -attrdefs-dialect=test)
add_public_tablegen_target(MLIRTestAttrDefIncGen)
set(LLVM_TARGET_DEFINITIONS TestTypeDefs.td)
@@ -86,6 +86,7 @@ add_mlir_library(MLIRTestDialect
MLIRLinalgTransforms
MLIRLLVMDialect
MLIRPass
+ MLIRPolynomialDialect
MLIRReduce
MLIRTensorDialect
MLIRTransformUtils
diff --git a/mlir/test/lib/Dialect/Test/TestAttrDefs.td b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
index 12635e107bd42..9e25acf5f5ba4 100644
--- a/mlir/test/lib/Dialect/Test/TestAttrDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
@@ -16,6 +16,7 @@
// To get the test dialect definition.
include "TestDialect.td"
include "TestEnumDefs.td"
+include "mlir/Dialect/Polynomial/IR/PolynomialAttributes.td"
include "mlir/Dialect/Utils/StructuredOpsUtils.td"
include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/BuiltinAttributeInterfaces.td"
@@ -351,4 +352,12 @@ def TestCustomFloatAttr : Test_Attr<"TestCustomFloat"> {
}];
}
+def NestedPolynomialAttr : Test_Attr<"NestedPolynomialAttr"> {
+ let mnemonic = "nested_polynomial";
+ let parameters = (ins Polynomial_IntPolynomialAttr:$poly);
+ let assemblyFormat = [{
+ `<` $poly `>`
+ }];
+}
+
#endif // TEST_ATTRDEFS
diff --git a/mlir/test/lib/Dialect/Test/TestAttributes.h b/mlir/test/lib/Dialect/Test/TestAttributes.h
index ef6eae51fdd62..7099bcf317294 100644
--- a/mlir/test/lib/Dialect/Test/TestAttributes.h
+++ b/mlir/test/lib/Dialect/Test/TestAttributes.h
@@ -17,6 +17,7 @@
#include <tuple>
#include "TestTraits.h"
+#include "mlir/Dialect/Polynomial/IR/PolynomialAttributes.h"
#include "mlir/Dialect/Utils/StructuredOpsUtils.h"
#include "mlir/IR/Attributes.h"
#include "mlir/IR/Diagnostics.h"
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 9d7e0a7928ab8..d7097e4d77452 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -232,6 +232,11 @@ def FloatElementsAttrOp : TEST_Op<"float_elements_attr"> {
);
}
+def ContainingIntPolynomialAttrOp : TEST_Op<"containing_int_polynomial_attr"> {
+ let arguments = (ins NestedPolynomialAttr:$attr);
+ let assemblyFormat = "$attr attr-dict";
+}
+
// A pattern that updates dense<[3.0, 4.0]> to dense<[5.0, 6.0]>.
// This tests both matching and generating float elements attributes.
def UpdateFloatElementsAttr : Pat<
@@ -2204,7 +2209,7 @@ def ForwardBufferOp : TEST_Op<"forward_buffer", [Pure]> {
def ReifyBoundOp : TEST_Op<"reify_bound", [Pure]> {
let description = [{
Reify a bound for the given index-typed value or dimension size of a shaped
- value. "LB", "EQ" and "UB" bounds are supported. If `scalable` is set,
+ value. "LB", "EQ" and "UB" bounds are supported. If `scalable` is set,
`vscale_min` and `vscale_max` must be provided, which allows computing
a bound in terms of "vector.vscale" for a given range of vscale.
}];
diff --git a/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp b/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
index 57e7d658fb501..c4e5d259adc24 100644
--- a/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
+++ b/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
@@ -15,6 +15,7 @@
#include "mlir/IR/Builders.h"
#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/IR/BuiltinOps.h"
+#include "mlir/Dialect/Polynomial/IR/PolynomialDialect.h"
#include "mlir/Target/LLVMIR/Dialect/Builtin/BuiltinToLLVMIRTranslation.h"
#include "mlir/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.h"
#include "mlir/Target/LLVMIR/LLVMTranslationInterface.h"
@@ -127,6 +128,7 @@ void registerTestToLLVMIR() {
},
[](DialectRegistry ®istry) {
registry.insert<test::TestDialect>();
+ registry.insert<polynomial::PolynomialDialect>();
registerBuiltinDialectTranslation(registry);
registerLLVMDialectTranslation(registry);
registry.addExtension(
>From 7454a7298dfba4979eb9af800b9d7f13f6745a5c Mon Sep 17 00:00:00 2001
From: Jeremy Kun <j2kun at users.noreply.github.com>
Date: Wed, 12 Jun 2024 13:52:53 -0700
Subject: [PATCH 2/6] ensure all registered dialects are loaded
---
mlir/lib/Tools/mlir-translate/Translation.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mlir/lib/Tools/mlir-translate/Translation.cpp b/mlir/lib/Tools/mlir-translate/Translation.cpp
index a39a4675c2762..efb5bd4ba7311 100644
--- a/mlir/lib/Tools/mlir-translate/Translation.cpp
+++ b/mlir/lib/Tools/mlir-translate/Translation.cpp
@@ -144,6 +144,10 @@ TranslateFromMLIRRegistration::TranslateFromMLIRRegistration(
DialectRegistry registry;
dialectRegistration(registry);
context->appendDialectRegistry(registry);
+ // Ensure all registered dialects are loaded
+ for (const auto &dialectName : registry.getDialectNames()) {
+ context->getOrLoadDialect(dialectName);
+ }
bool implicitModule =
(!clOptions.isConstructed() || !clOptions->noImplicitModule);
OwningOpRef<Operation *> op =
>From 2ce12000086754ad5a2677f657f0a51218177b18 Mon Sep 17 00:00:00 2001
From: Jeremy Kun <j2kun at users.noreply.github.com>
Date: Wed, 12 Jun 2024 13:55:22 -0700
Subject: [PATCH 3/6] fix llvm.return
---
mlir/test/Target/LLVMIR/test.mlir | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mlir/test/Target/LLVMIR/test.mlir b/mlir/test/Target/LLVMIR/test.mlir
index c7a28a2892da4..e6c931a521d60 100644
--- a/mlir/test/Target/LLVMIR/test.mlir
+++ b/mlir/test/Target/LLVMIR/test.mlir
@@ -48,10 +48,9 @@ llvm.func @dialect_attr_translation_multi(%a: i64, %b: i64, %c: i64) -> i64 {
// parser would only load the dialect if the fully namespaced attribute was
// present in the IR.
#attr = #test.nested_polynomial<<1 + x**2>>
+// CHECK-lABLE: @parse_correctly
llvm.func @parse_correctly() {
- // CHECK: <1 + x**2>
test.containing_int_polynomial_attr #attr
-
- return
+ llvm.return
}
>From 68a7892e8bf6679b86009e0724cb3e08e295d39e Mon Sep 17 00:00:00 2001
From: Jeremy Kun <j2kun at users.noreply.github.com>
Date: Wed, 12 Jun 2024 14:05:41 -0700
Subject: [PATCH 4/6] add test translation for polynomial op
---
mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp b/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
index c4e5d259adc24..7f491a2d114e3 100644
--- a/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
+++ b/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
@@ -107,6 +107,10 @@ LogicalResult TestDialectLLVMIRTranslationInterface::convertOperation(
mod->getOrInsertGlobal(symOp.getSymName(), i32Type);
return success();
})
+ .Case([&](test::ContainingIntPolynomialAttrOp polyOp) {
+ // Discardable, as this op existed only for testing parsin.
+ return success();
+ })
.Default([&](Operation *) {
return op->emitOpError("unsupported translation of test operation");
});
>From 0d0546d667b6d58f55b4120cc6c5a98e88fe39ca Mon Sep 17 00:00:00 2001
From: Jeremy Kun <j2kun at users.noreply.github.com>
Date: Wed, 12 Jun 2024 14:06:49 -0700
Subject: [PATCH 5/6] clang-format
---
mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp b/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
index 7f491a2d114e3..e1b3c2ac45922 100644
--- a/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
+++ b/mlir/test/lib/Dialect/Test/TestToLLVMIRTranslation.cpp
@@ -12,10 +12,10 @@
#include "TestDialect.h"
#include "TestOps.h"
+#include "mlir/Dialect/Polynomial/IR/PolynomialDialect.h"
#include "mlir/IR/Builders.h"
#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/IR/BuiltinOps.h"
-#include "mlir/Dialect/Polynomial/IR/PolynomialDialect.h"
#include "mlir/Target/LLVMIR/Dialect/Builtin/BuiltinToLLVMIRTranslation.h"
#include "mlir/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.h"
#include "mlir/Target/LLVMIR/LLVMTranslationInterface.h"
>From 574e4858cb24a306072186c394299ae7898db717 Mon Sep 17 00:00:00 2001
From: Jeremy Kun <j2kun at users.noreply.github.com>
Date: Tue, 18 Jun 2024 14:20:01 -0700
Subject: [PATCH 6/6] force qualified
---
mlir/lib/Tools/mlir-translate/Translation.cpp | 4 ----
mlir/test/Target/LLVMIR/test.mlir | 2 +-
mlir/test/lib/Dialect/Test/TestAttrDefs.td | 2 +-
3 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/mlir/lib/Tools/mlir-translate/Translation.cpp b/mlir/lib/Tools/mlir-translate/Translation.cpp
index efb5bd4ba7311..a39a4675c2762 100644
--- a/mlir/lib/Tools/mlir-translate/Translation.cpp
+++ b/mlir/lib/Tools/mlir-translate/Translation.cpp
@@ -144,10 +144,6 @@ TranslateFromMLIRRegistration::TranslateFromMLIRRegistration(
DialectRegistry registry;
dialectRegistration(registry);
context->appendDialectRegistry(registry);
- // Ensure all registered dialects are loaded
- for (const auto &dialectName : registry.getDialectNames()) {
- context->getOrLoadDialect(dialectName);
- }
bool implicitModule =
(!clOptions.isConstructed() || !clOptions->noImplicitModule);
OwningOpRef<Operation *> op =
diff --git a/mlir/test/Target/LLVMIR/test.mlir b/mlir/test/Target/LLVMIR/test.mlir
index e6c931a521d60..a94c09cebc211 100644
--- a/mlir/test/Target/LLVMIR/test.mlir
+++ b/mlir/test/Target/LLVMIR/test.mlir
@@ -47,7 +47,7 @@ llvm.func @dialect_attr_translation_multi(%a: i64, %b: i64, %c: i64) -> i64 {
// This is a regression test for a bug where, during an mlir-translate call the
// parser would only load the dialect if the fully namespaced attribute was
// present in the IR.
-#attr = #test.nested_polynomial<<1 + x**2>>
+#attr = #test.nested_polynomial<#polynomial.int_polynomial<1 + x**2>>
// CHECK-lABLE: @parse_correctly
llvm.func @parse_correctly() {
test.containing_int_polynomial_attr #attr
diff --git a/mlir/test/lib/Dialect/Test/TestAttrDefs.td b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
index 9e25acf5f5ba4..b2533c43bac84 100644
--- a/mlir/test/lib/Dialect/Test/TestAttrDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestAttrDefs.td
@@ -356,7 +356,7 @@ def NestedPolynomialAttr : Test_Attr<"NestedPolynomialAttr"> {
let mnemonic = "nested_polynomial";
let parameters = (ins Polynomial_IntPolynomialAttr:$poly);
let assemblyFormat = [{
- `<` $poly `>`
+ `<` qualified($poly) `>`
}];
}
More information about the Mlir-commits
mailing list