[Mlir-commits] [mlir] 79f736c - Switch generatedTypeParser/generatedAttributeParser to return an OptionalParseResult
Mehdi Amini
llvmlistbot at llvm.org
Tue Mar 9 11:50:16 PST 2021
Author: Mehdi Amini
Date: 2021-03-09T19:43:45Z
New Revision: 79f736c150c5a533f836727cb247a72f9ad96096
URL: https://github.com/llvm/llvm-project/commit/79f736c150c5a533f836727cb247a72f9ad96096
DIFF: https://github.com/llvm/llvm-project/commit/79f736c150c5a533f836727cb247a72f9ad96096.diff
LOG: Switch generatedTypeParser/generatedAttributeParser to return an OptionalParseResult
This allows the caller to distinguish between a parse error or an
unmatched keyword. It fixes the redundant error that was emitted by the
caller when the generated parser would fail.
Differential Revision: https://reviews.llvm.org/D98162
Added:
Modified:
mlir/lib/Dialect/ArmSVE/IR/ArmSVEDialect.cpp
mlir/lib/Dialect/Async/IR/Async.cpp
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
mlir/lib/Dialect/PDL/IR/PDLTypes.cpp
mlir/test/Dialect/LLVMIR/invalid.mlir
mlir/test/Dialect/PDL/invalid-types.mlir
mlir/test/lib/Dialect/Test/TestAttributes.cpp
mlir/test/lib/Dialect/Test/TestTypes.cpp
mlir/test/mlir-tblgen/attrdefs.td
mlir/test/mlir-tblgen/typedefs.td
mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
Removed:
################################################################################
diff --git a/mlir/lib/Dialect/ArmSVE/IR/ArmSVEDialect.cpp b/mlir/lib/Dialect/ArmSVE/IR/ArmSVEDialect.cpp
index c10458a442b2..a796b0725d36 100644
--- a/mlir/lib/Dialect/ArmSVE/IR/ArmSVEDialect.cpp
+++ b/mlir/lib/Dialect/ArmSVE/IR/ArmSVEDialect.cpp
@@ -43,9 +43,13 @@ void arm_sve::ArmSVEDialect::initialize() {
Type arm_sve::ArmSVEDialect::parseType(DialectAsmParser &parser) const {
llvm::SMLoc typeLoc = parser.getCurrentLocation();
- auto genType = generatedTypeParser(getContext(), parser, "vector");
- if (genType != Type())
- return genType;
+ {
+ Type genType;
+ auto parseResult = generatedTypeParser(parser.getBuilder().getContext(),
+ parser, "vector", genType);
+ if (parseResult.hasValue())
+ return genType;
+ }
parser.emitError(typeLoc, "unknown type in ArmSVE dialect");
return Type();
}
diff --git a/mlir/lib/Dialect/Async/IR/Async.cpp b/mlir/lib/Dialect/Async/IR/Async.cpp
index 23b17d287f63..6f2f69d2627c 100644
--- a/mlir/lib/Dialect/Async/IR/Async.cpp
+++ b/mlir/lib/Dialect/Async/IR/Async.cpp
@@ -331,9 +331,14 @@ void AsyncDialect::printType(Type type, DialectAsmPrinter &os) const {
/// Parse a type registered to this dialect.
Type AsyncDialect::parseType(DialectAsmParser &parser) const {
- StringRef mnemonic;
- if (parser.parseKeyword(&mnemonic))
+ StringRef typeTag;
+ if (parser.parseKeyword(&typeTag))
return Type();
-
- return generatedTypeParser(getContext(), parser, mnemonic);
+ Type genType;
+ auto parseResult = generatedTypeParser(parser.getBuilder().getContext(),
+ parser, typeTag, genType);
+ if (parseResult.hasValue())
+ return genType;
+ parser.emitError(parser.getNameLoc(), "unknown async type: ") << typeTag;
+ return {};
}
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index ca8f39a9f9fc..07f4217a2479 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2533,11 +2533,14 @@ Attribute LLVMDialect::parseAttribute(DialectAsmParser &parser,
StringRef attrKind;
if (parser.parseKeyword(&attrKind))
return {};
- if (auto attr =
- generatedAttributeParser(getContext(), parser, attrKind, type))
- return attr;
-
- parser.emitError(parser.getNameLoc(), "Unknown attribute type: ") << attrKind;
+ {
+ Attribute attr;
+ auto parseResult =
+ generatedAttributeParser(getContext(), parser, attrKind, type, attr);
+ if (parseResult.hasValue())
+ return attr;
+ }
+ parser.emitError(parser.getNameLoc(), "unknown attribute type: ") << attrKind;
return {};
}
diff --git a/mlir/lib/Dialect/PDL/IR/PDLTypes.cpp b/mlir/lib/Dialect/PDL/IR/PDLTypes.cpp
index 37cdf772338e..b16fade224fc 100644
--- a/mlir/lib/Dialect/PDL/IR/PDLTypes.cpp
+++ b/mlir/lib/Dialect/PDL/IR/PDLTypes.cpp
@@ -27,19 +27,23 @@ using namespace mlir::pdl;
//===----------------------------------------------------------------------===//
static Type parsePDLType(DialectAsmParser &parser) {
- StringRef keyword;
- if (parser.parseKeyword(&keyword))
+ StringRef typeTag;
+ if (parser.parseKeyword(&typeTag))
return Type();
- if (Type type = generatedTypeParser(parser.getBuilder().getContext(), parser,
- keyword))
- return type;
+ {
+ Type genType;
+ auto parseResult = generatedTypeParser(parser.getBuilder().getContext(),
+ parser, typeTag, genType);
+ if (parseResult.hasValue())
+ return genType;
+ }
// FIXME: This ends up with a double error being emitted if `RangeType` also
// emits an error. We should rework the `generatedTypeParser` to better
// support when the keyword is valid but the individual type parser itself
// emits an error.
parser.emitError(parser.getNameLoc(), "invalid 'pdl' type: `")
- << keyword << "'";
+ << typeTag << "'";
return Type();
}
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index 91041f29f0da..172a79d38c4d 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -790,8 +790,7 @@ module {
module {
llvm.func @loopOptions() {
- // expected-error at +2 {{unknown loop option: name}}
- // expected-error at below {{Unknown attribute type: loopopts}}
+ // expected-error at below {{unknown loop option: name}}
llvm.br ^bb4 {llvm.loop = {options = #llvm.loopopts<name>}}
^bb4:
llvm.return
@@ -802,8 +801,7 @@ module {
module {
llvm.func @loopOptions() {
- // expected-error at +2 {{loop option present twice}}
- // expected-error at below {{Unknown attribute type: loopopts}}
+ // expected-error at below {{loop option present twice}}
llvm.br ^bb4 {llvm.loop = {options = #llvm.loopopts<disable_licm = true, disable_licm = true>}}
^bb4:
llvm.return
diff --git a/mlir/test/Dialect/PDL/invalid-types.mlir b/mlir/test/Dialect/PDL/invalid-types.mlir
index 8d677db27adf..a99cf3bdb238 100644
--- a/mlir/test/Dialect/PDL/invalid-types.mlir
+++ b/mlir/test/Dialect/PDL/invalid-types.mlir
@@ -4,6 +4,5 @@
// pdl::RangeType
//===----------------------------------------------------------------------===//
-// expected-error at +2 {{element of pdl.range cannot be another range, but got'!pdl.range<value>'}}
-// expected-error at +1 {{invalid 'pdl' type}}
+// expected-error at below {{element of pdl.range cannot be another range, but got'!pdl.range<value>'}}
#invalid_element = !pdl.range<range<value>>
diff --git a/mlir/test/lib/Dialect/Test/TestAttributes.cpp b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
index d13cd5a0bd72..72921a22e475 100644
--- a/mlir/test/lib/Dialect/Test/TestAttributes.cpp
+++ b/mlir/test/lib/Dialect/Test/TestAttributes.cpp
@@ -105,9 +105,13 @@ Attribute TestDialect::parseAttribute(DialectAsmParser &parser,
StringRef attrTag;
if (failed(parser.parseKeyword(&attrTag)))
return Attribute();
- if (auto attr = generatedAttributeParser(getContext(), parser, attrTag, type))
- return attr;
-
+ {
+ Attribute attr;
+ auto parseResult =
+ generatedAttributeParser(getContext(), parser, attrTag, type, attr);
+ if (parseResult.hasValue())
+ return attr;
+ }
parser.emitError(parser.getNameLoc(), "unknown test attribute");
return Attribute();
}
diff --git a/mlir/test/lib/Dialect/Test/TestTypes.cpp b/mlir/test/lib/Dialect/Test/TestTypes.cpp
index 792ccfb0084d..7b6dd7395c53 100644
--- a/mlir/test/lib/Dialect/Test/TestTypes.cpp
+++ b/mlir/test/lib/Dialect/Test/TestTypes.cpp
@@ -138,10 +138,12 @@ static Type parseTestType(MLIRContext *ctxt, DialectAsmParser &parser,
if (failed(parser.parseKeyword(&typeTag)))
return Type();
- auto genType = generatedTypeParser(ctxt, parser, typeTag);
- if (genType != Type())
- return genType;
-
+ {
+ Type genType;
+ auto parseResult = generatedTypeParser(ctxt, parser, typeTag, genType);
+ if (parseResult.hasValue())
+ return genType;
+ }
if (typeTag == "test_type")
return TestType::get(parser.getBuilder().getContext());
diff --git a/mlir/test/mlir-tblgen/attrdefs.td b/mlir/test/mlir-tblgen/attrdefs.td
index 802c2a1f793d..38815fcd3434 100644
--- a/mlir/test/mlir-tblgen/attrdefs.td
+++ b/mlir/test/mlir-tblgen/attrdefs.td
@@ -18,12 +18,18 @@ include "mlir/IR/OpBase.td"
// DEF: ::mlir::test::IndexAttr,
// DEF: ::mlir::test::SingleParameterAttr
-// DEF-LABEL: ::mlir::Attribute generatedAttributeParser(::mlir::MLIRContext *context,
+// DEF-LABEL: OptionalParseResult generatedAttributeParser(::mlir::MLIRContext *context,
// DEF-NEXT: ::mlir::DialectAsmParser &parser,
-// DEF-NEXT: ::llvm::StringRef mnemonic, ::mlir::Type type) {
-// DEF: if (mnemonic == ::mlir::test::CompoundAAttr::getMnemonic()) return ::mlir::test::CompoundAAttr::parse(context, parser, type);
-// DEF-NEXT: if (mnemonic == ::mlir::test::IndexAttr::getMnemonic()) return ::mlir::test::IndexAttr::parse(context, parser, type);
-// DEF: return ::mlir::Attribute();
+// DEF-NEXT: ::llvm::StringRef mnemonic, ::mlir::Type type,
+// DEF-NEXT: ::mlir::Attribute &value) {
+// DEF: if (mnemonic == ::mlir::test::CompoundAAttr::getMnemonic()) {
+// DEF-NEXT: value = ::mlir::test::CompoundAAttr::parse(context, parser, type);
+// DEF-NEXT: return success(!!value);
+// DEF-NEXT: }
+// DEF-NEXT: if (mnemonic == ::mlir::test::IndexAttr::getMnemonic()) {
+// DEF-NEXT: value = ::mlir::test::IndexAttr::parse(context, parser, type);
+// DEF-NEXT: return success(!!value);
+// DEF: return {};
def Test_Dialect: Dialect {
// DECL-NOT: TestDialect
diff --git a/mlir/test/mlir-tblgen/typedefs.td b/mlir/test/mlir-tblgen/typedefs.td
index 57103ccc7db9..c94241eda518 100644
--- a/mlir/test/mlir-tblgen/typedefs.td
+++ b/mlir/test/mlir-tblgen/typedefs.td
@@ -19,11 +19,18 @@ include "mlir/IR/OpBase.td"
// DEF: ::mlir::test::SingleParameterType,
// DEF: ::mlir::test::IntegerType
-// DEF-LABEL: ::mlir::Type generatedTypeParser(::mlir::MLIRContext *context,
+// DEF-LABEL: OptionalParseResult generatedTypeParser(::mlir::MLIRContext *context,
// DEF-NEXT: ::mlir::DialectAsmParser &parser,
-// DEF-NEXT: ::llvm::StringRef mnemonic) {
-// DEF: if (mnemonic == ::mlir::test::CompoundAType::getMnemonic()) return ::mlir::test::CompoundAType::parse(context, parser);
-// DEF: return ::mlir::Type();
+// DEF-NEXT: ::llvm::StringRef mnemonic,
+// DEF-NEXT: ::mlir::Type &value) {
+// DEF: if (mnemonic == ::mlir::test::CompoundAType::getMnemonic()) {
+// DEF-NEXT: value = ::mlir::test::CompoundAType::parse(context, parser);
+// DEF-NEXT: return success(!!value);
+// DEF-NEXT: }
+// DEF-NEXT: if (mnemonic == ::mlir::test::IndexType::getMnemonic()) {
+// DEF-NEXT: value = ::mlir::test::IndexType::parse(context, parser);
+// DEF-NEXT: return success(!!value);
+// DEF: return {};
def Test_Dialect: Dialect {
// DECL-NOT: TestDialect
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index b7e5c7d27ff4..bfac87ffed70 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -412,9 +412,10 @@ void DefGenerator::emitTypeDefList(ArrayRef<AttrOrTypeDef> defs) {
/// {0}: The name of the base value type, e.g. Attribute or Type.
/// {1}: Additional parser parameters.
static const char *const defParserDispatchStartStr = R"(
-static ::mlir::{0} generated{0}Parser(::mlir::MLIRContext *context,
+static OptionalParseResult generated{0}Parser(::mlir::MLIRContext *context,
::mlir::DialectAsmParser &parser,
- ::llvm::StringRef mnemonic{1}) {{
+ ::llvm::StringRef mnemonic{1},
+ ::mlir::{0} &value) {{
)";
/// The code block used to start the auto-generated printer function.
@@ -805,22 +806,22 @@ void DefGenerator::emitParsePrintDispatch(ArrayRef<AttrOrTypeDef> defs) {
isAttrGenerator ? ", ::mlir::Type type" : "");
for (const AttrOrTypeDef &def : defs) {
if (def.getMnemonic()) {
- os << formatv(
- " if (mnemonic == {0}::{1}::getMnemonic()) return {0}::{1}::",
- def.getDialect().getCppNamespace(), def.getCppClassName());
+ os << formatv(" if (mnemonic == {0}::{1}::getMnemonic()) { \n"
+ " value = {0}::{1}::",
+ def.getDialect().getCppNamespace(), def.getCppClassName());
// If the def has no parameters and no parser code, just invoke a normal
// `get`.
if (def.getNumParameters() == 0 && !def.getParserCode()) {
- os << "get(context);\n";
+ os << "get(context);\n return success(!!value);\n }\n";
continue;
}
os << "parse(context, parser" << (isAttrGenerator ? ", type" : "")
- << ");\n";
+ << ");\n return success(!!value);\n }\n";
}
}
- os << " return ::mlir::" << valueType << "();\n";
+ os << " return {};\n";
os << "}\n\n";
// The printer dispatch uses llvm::TypeSwitch to find and call the correct
More information about the Mlir-commits
mailing list