[Mlir-commits] [mlir] 1ae60e0 - [mlir] Making verification after parsing optional
River Riddle
llvmlistbot at llvm.org
Wed Sep 28 20:47:09 PDT 2022
Author: River Riddle
Date: 2022-09-28T20:38:12-07:00
New Revision: 1ae60e044e40de1752c8c1aa2cf9af243f1e8b5f
URL: https://github.com/llvm/llvm-project/commit/1ae60e044e40de1752c8c1aa2cf9af243f1e8b5f
DIFF: https://github.com/llvm/llvm-project/commit/1ae60e044e40de1752c8c1aa2cf9af243f1e8b5f.diff
LOG: [mlir] Making verification after parsing optional
This is very useful when you want to parse IR even if
its invalid (e.g. bytecode). It's also useful if you don't
want to pay the cost of verification in certain situations.
Differential Revision: https://reviews.llvm.org/D134847
Added:
mlir/unittests/Parser/ParserTest.cpp
Modified:
mlir/include/mlir/IR/AsmState.h
mlir/lib/AsmParser/Parser.cpp
mlir/lib/Bytecode/Reader/BytecodeReader.cpp
mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
mlir/unittests/Parser/CMakeLists.txt
Removed:
################################################################################
diff --git a/mlir/include/mlir/IR/AsmState.h b/mlir/include/mlir/IR/AsmState.h
index 87f3a37b637d..0b00f1fb46dc 100644
--- a/mlir/include/mlir/IR/AsmState.h
+++ b/mlir/include/mlir/IR/AsmState.h
@@ -456,17 +456,22 @@ class FallbackAsmResourceMap {
class ParserConfig {
public:
/// Construct a parser configuration with the given context.
+ /// `verifyAfterParse` indicates if the IR should be verified after parsing.
/// `fallbackResourceMap` is an optional fallback handler that can be used to
/// parse external resources not explicitly handled by another parser.
- ParserConfig(MLIRContext *context,
+ ParserConfig(MLIRContext *context, bool verifyAfterParse = true,
FallbackAsmResourceMap *fallbackResourceMap = nullptr)
- : context(context), fallbackResourceMap(fallbackResourceMap) {
+ : context(context), verifyAfterParse(verifyAfterParse),
+ fallbackResourceMap(fallbackResourceMap) {
assert(context && "expected valid MLIR context");
}
/// Return the MLIRContext to be used when parsing.
MLIRContext *getContext() const { return context; }
+ /// Returns if the parser should verify the IR after parsing.
+ bool shouldVerifyAfterParse() const { return verifyAfterParse; }
+
/// Return the resource parser registered to the given name, or nullptr if no
/// parser with `name` is registered.
AsmResourceParser *getResourceParser(StringRef name) const {
@@ -498,6 +503,7 @@ class ParserConfig {
private:
MLIRContext *context;
+ bool verifyAfterParse;
DenseMap<StringRef, std::unique_ptr<AsmResourceParser>> resourceParsers;
FallbackAsmResourceMap *fallbackResourceMap;
};
diff --git a/mlir/lib/AsmParser/Parser.cpp b/mlir/lib/AsmParser/Parser.cpp
index 9e4e11e81299..e4da76a79546 100644
--- a/mlir/lib/AsmParser/Parser.cpp
+++ b/mlir/lib/AsmParser/Parser.cpp
@@ -802,7 +802,7 @@ ParseResult OperationParser::finalize() {
return failure();
// Verify that the parsed operations are valid.
- if (failed(verify(topLevelOp)))
+ if (state.config.shouldVerifyAfterParse() && failed(verify(topLevelOp)))
return failure();
// If we are populating the parser state, finalize the top-level operation.
diff --git a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
index 2f36de523ff3..d61a8acc1b67 100644
--- a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
+++ b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
@@ -1408,7 +1408,7 @@ LogicalResult BytecodeReader::parseIRSection(ArrayRef<uint8_t> sectionData,
}
// Verify that the parsed operations are valid.
- if (failed(verify(*moduleOp)))
+ if (config.shouldVerifyAfterParse() && failed(verify(*moduleOp)))
return failure();
// Splice the parsed operations over to the provided top-level block.
diff --git a/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
index 93cc4dc33538..deac47e071cb 100644
--- a/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
+++ b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
@@ -342,7 +342,8 @@ MLIRDocument::MLIRDocument(MLIRContext &context, const lsp::URIForFile &uri,
return;
}
- ParserConfig config(&context, &fallbackResourceMap);
+ ParserConfig config(&context, /*verifyAfterParse=*/true,
+ &fallbackResourceMap);
sourceMgr.AddNewSourceBuffer(std::move(memBuffer), SMLoc());
if (failed(parseAsmSourceFile(sourceMgr, &parsedIR, config, &asmState))) {
// If parsing failed, clear out any of the current state.
@@ -1296,7 +1297,8 @@ lsp::MLIRServer::convertFromBytecode(const URIForFile &uri) {
FallbackAsmResourceMap fallbackResourceMap;
// Setup the parser config.
- ParserConfig parserConfig(&tempContext, &fallbackResourceMap);
+ ParserConfig parserConfig(&tempContext, /*verifyAfterParse=*/true,
+ &fallbackResourceMap);
// Try to parse the given source file.
Block parsedBlock;
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 5d02d853b7d9..9481373578f3 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -66,7 +66,7 @@ static LogicalResult performActions(raw_ostream &os, bool verifyDiagnostics,
// untouched.
PassReproducerOptions reproOptions;
FallbackAsmResourceMap fallbackResourceMap;
- ParserConfig config(context, &fallbackResourceMap);
+ ParserConfig config(context, /*verifyAfterParse=*/true, &fallbackResourceMap);
reproOptions.attachResourceParser(config);
// Parse the input file and reset the context threading state.
diff --git a/mlir/unittests/Parser/CMakeLists.txt b/mlir/unittests/Parser/CMakeLists.txt
index c3a5a9b89e32..f5646ff3f2c3 100644
--- a/mlir/unittests/Parser/CMakeLists.txt
+++ b/mlir/unittests/Parser/CMakeLists.txt
@@ -1,4 +1,5 @@
add_mlir_unittest(MLIRParserTests
+ ParserTest.cpp
ResourceTest.cpp
DEPENDS
diff --git a/mlir/unittests/Parser/ParserTest.cpp b/mlir/unittests/Parser/ParserTest.cpp
new file mode 100644
index 000000000000..df78174efc99
--- /dev/null
+++ b/mlir/unittests/Parser/ParserTest.cpp
@@ -0,0 +1,31 @@
+//===- ParserTest.cpp -----------------------------------------------------===//
+//
+// This file is licensed under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Parser/Parser.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Verifier.h"
+
+#include "gmock/gmock.h"
+
+using namespace mlir;
+
+namespace {
+TEST(MLIRParser, ParseInvalidIR) {
+ std::string moduleStr = R"mlir(
+ module attributes {bad} {}
+ )mlir";
+
+ MLIRContext context;
+ ParserConfig config(&context, /*verifyAfterParse=*/false);
+
+ // Check that we properly parse the op, but it fails the verifier.
+ OwningOpRef<ModuleOp> module = parseSourceString<ModuleOp>(moduleStr, config);
+ ASSERT_TRUE(module);
+ ASSERT_TRUE(failed(verify(*module)));
+}
+} // namespace
More information about the Mlir-commits
mailing list