[Mlir-commits] [mlir] f6ae8e6 - [mlir][irdl] Fix missing verifier in irdl.parametric (#92700)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon May 20 04:52:11 PDT 2024
Author: Théo Degioanni
Date: 2024-05-20T12:52:07+01:00
New Revision: f6ae8e6381a02b10091589d7742fab389f847ea9
URL: https://github.com/llvm/llvm-project/commit/f6ae8e6381a02b10091589d7742fab389f847ea9
DIFF: https://github.com/llvm/llvm-project/commit/f6ae8e6381a02b10091589d7742fab389f847ea9.diff
LOG: [mlir][irdl] Fix missing verifier in irdl.parametric (#92700)
The parametric op was not checking the symbol it points to is a type or
attribute. This PR also fixes a small bug where an invalid IRDL file
would not end processing in mlir-opt. I also improved the error messages
for the already handled irdl.base invalid symbols.
Added:
Modified:
mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
mlir/lib/Dialect/IRDL/IR/IRDL.cpp
mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
mlir/test/Dialect/IRDL/invalid.irdl.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td b/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
index aa6a8e93c0288..d2765dec420ac 100644
--- a/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
+++ b/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
@@ -503,7 +503,8 @@ def IRDL_BaseOp : IRDL_ConstraintOp<"base",
}
def IRDL_ParametricOp : IRDL_ConstraintOp<"parametric",
- [ParentOneOf<["TypeOp", "AttributeOp", "OperationOp"]>, Pure]> {
+ [ParentOneOf<["TypeOp", "AttributeOp", "OperationOp"]>,
+ DeclareOpInterfaceMethods<SymbolUserOpInterface>, Pure]> {
let summary = "Constraints an attribute/type base and its parameters";
let description = [{
`irdl.parametric` defines a constraint that accepts only a single type
diff --git a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp
index 4eae2b03024c2..e4728f55b49d7 100644
--- a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp
+++ b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp
@@ -132,22 +132,37 @@ LogicalResult BaseOp::verify() {
return success();
}
+static LogicalResult
+checkSymbolIsTypeOrAttribute(SymbolTableCollection &symbolTable,
+ Operation *source, SymbolRefAttr symbol) {
+ Operation *targetOp = symbolTable.lookupNearestSymbolFrom(source, symbol);
+ if (!targetOp)
+ return source->emitOpError() << "symbol '" << symbol << "' not found";
+
+ if (!isa<TypeOp, AttributeOp>(targetOp))
+ return source->emitOpError() << "symbol '" << symbol
+ << "' does not refer to a type or attribute "
+ "definition (refers to '"
+ << targetOp->getName() << "')";
+
+ return success();
+}
+
LogicalResult BaseOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
std::optional<SymbolRefAttr> baseRef = getBaseRef();
if (!baseRef)
return success();
- TypeOp typeOp = symbolTable.lookupNearestSymbolFrom<TypeOp>(*this, *baseRef);
- if (typeOp)
- return success();
+ return checkSymbolIsTypeOrAttribute(symbolTable, *this, *baseRef);
+}
- AttributeOp attrOp =
- symbolTable.lookupNearestSymbolFrom<AttributeOp>(*this, *baseRef);
- if (attrOp)
+LogicalResult
+ParametricOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
+ std::optional<SymbolRefAttr> baseRef = getBaseType();
+ if (!baseRef)
return success();
- return emitOpError() << "'" << *baseRef
- << "' does not refer to a type or attribute definition";
+ return checkSymbolIsTypeOrAttribute(symbolTable, *this, *baseRef);
}
/// Parse a value with its variadicity first. By default, the variadicity is
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 44c5e9826f3b7..a1b2893a973b1 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -266,6 +266,8 @@ LogicalResult loadIRDLDialects(StringRef irdlFile, MLIRContext &ctx) {
// Parse the input file.
OwningOpRef<ModuleOp> module(parseSourceFile<ModuleOp>(sourceMgr, &ctx));
+ if (!module)
+ return failure();
// Load IRDL dialects.
return irdl::loadDialects(module.get());
diff --git a/mlir/test/Dialect/IRDL/invalid.irdl.mlir b/mlir/test/Dialect/IRDL/invalid.irdl.mlir
index d62bb498a7ad9..f207d31cf158b 100644
--- a/mlir/test/Dialect/IRDL/invalid.irdl.mlir
+++ b/mlir/test/Dialect/IRDL/invalid.irdl.mlir
@@ -6,7 +6,7 @@ func.func private @foo()
irdl.dialect @testd {
irdl.type @type {
- // expected-error at +1 {{'@foo' does not refer to a type or attribute definition}}
+ // expected-error at +1 {{symbol '@foo' not found}}
%0 = irdl.base @foo
irdl.parameters(%0)
}
@@ -41,3 +41,18 @@ irdl.dialect @testd {
irdl.parameters(%0)
}
}
+
+// -----
+
+irdl.dialect @invalid_parametric {
+ irdl.operation @foo {
+ // expected-error at +1 {{symbol '@not_a_type_or_attr' does not refer to a type or attribute definition}}
+ %param = irdl.parametric @not_a_type_or_attr<>
+ irdl.results(%param)
+ }
+
+ irdl.operation @not_a_type_or_attr {
+ %param = irdl.is i1
+ irdl.results(%param)
+ }
+}
More information about the Mlir-commits
mailing list