Skip to content

Commit

Permalink
[MLIR] Fix assert crash when an unregistered dialect op is encountered
Browse files Browse the repository at this point in the history
Fix assert crash when an unregistered dialect op is encountered during
parsing and `-allow-unregistered-dialect' isn't on. Instead, emit an
error.

While on this, clean up "registered" vs "loaded" on `getDialect()` and
local clang-tidy warnings.

https://llvm.discourse.group/t/assert-behavior-on-unregistered-dialect-ops/4402

Differential Revision: https://reviews.llvm.org/D111628
  • Loading branch information
bondhugula committed Oct 14, 2021
1 parent a8f69be commit 05fb260
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 17 deletions.
2 changes: 1 addition & 1 deletion mlir/include/mlir/IR/Operation.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class alignas(8) Operation final
MLIRContext *getContext() { return location->getContext(); }

/// Return the dialect this operation is associated with, or nullptr if the
/// associated dialect is not registered.
/// associated dialect is not loaded.
Dialect *getDialect() { return getName().getDialect(); }

/// The source location the operation was defined or derived from.
Expand Down
4 changes: 2 additions & 2 deletions mlir/include/mlir/IR/OperationSupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ class OperationName {
/// Return the name of the dialect this operation is registered to.
StringRef getDialectNamespace() const;

/// Return the Dialect this operation is registered to if it is loaded in the
/// context, or nullptr if the dialect isn't loaded.
/// Return the dialect this operation is registered to if the dialect is
/// loaded in the context, or nullptr if the dialect isn't loaded.
Dialect *getDialect() const {
if (const auto *abstractOp = getAbstractOperation())
return &abstractOp->dialect;
Expand Down
2 changes: 1 addition & 1 deletion mlir/lib/IR/Operation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ Operation::Operation(Location location, OperationName name, unsigned numResults,
name.getStringRef() +
" created with unregistered dialect. If this is intended, please call "
"allowUnregisteredDialects() on the MLIRContext, or use "
"-allow-unregistered-dialect with the MLIR opt tool used");
"-allow-unregistered-dialect with the MLIR tool used.");
#endif
}

Expand Down
29 changes: 19 additions & 10 deletions mlir/lib/Parser/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ ParseResult OperationParser::finalize() {
errors.push_back(entry.second.getPointer());
llvm::array_pod_sort(errors.begin(), errors.end());

for (auto entry : errors) {
for (const char *entry : errors) {
auto loc = SMLoc::getFromPointer(entry);
emitError(loc, "use of undeclared SSA value name");
}
Expand Down Expand Up @@ -989,9 +989,18 @@ Operation *OperationParser::parseGenericOperation() {
// Lazy load dialects in the context as needed.
if (!result.name.getAbstractOperation()) {
StringRef dialectName = StringRef(name).split('.').first;
if (!getContext()->getLoadedDialect(dialectName) &&
getContext()->getOrLoadDialect(dialectName)) {
result.name = OperationName(name, getContext());
if (!getContext()->getLoadedDialect(dialectName)) {
if (getContext()->getOrLoadDialect(dialectName)) {
result.name = OperationName(name, getContext());
} else if (!getContext()->allowsUnregisteredDialects()) {
// Emit an error if the dialect couldn't be loaded (i.e., it was not
// registered) and unregistered dialects aren't allowed.
return emitError(
"operation being parsed with an unregistered dialect. If "
"this is intended, please use -allow-unregistered-dialect "
"with the MLIR tool used"),
nullptr;
}
}
}

Expand Down Expand Up @@ -1689,8 +1698,8 @@ ParseResult OperationParser::parseRegionBody(
pushSSANameScope(isIsolatedNameScope);

// Parse the first block directly to allow for it to be unnamed.
auto owning_block = std::make_unique<Block>();
Block *block = owning_block.get();
auto owningBlock = std::make_unique<Block>();
Block *block = owningBlock.get();

// If this block is not defined in the source file, add a definition for it
// now in the assembly state. Blocks with a name will be defined when the name
Expand Down Expand Up @@ -1737,7 +1746,7 @@ ParseResult OperationParser::parseRegionBody(
}

// Parse the rest of the region.
region.push_back(owning_block.release());
region.push_back(owningBlock.release());
while (getToken().isNot(Token::r_brace)) {
Block *newBlock = nullptr;
if (parseBlock(newBlock))
Expand Down Expand Up @@ -2071,13 +2080,13 @@ LogicalResult mlir::parseSourceFile(llvm::StringRef filename,
return emitError(mlir::UnknownLoc::get(context),
"only main buffer parsed at the moment");
}
auto file_or_err = llvm::MemoryBuffer::getFileOrSTDIN(filename);
if (std::error_code error = file_or_err.getError())
auto fileOrErr = llvm::MemoryBuffer::getFileOrSTDIN(filename);
if (std::error_code error = fileOrErr.getError())
return emitError(mlir::UnknownLoc::get(context),
"could not open input file " + filename);

// Load the MLIR source file.
sourceMgr.AddNewSourceBuffer(std::move(*file_or_err), llvm::SMLoc());
sourceMgr.AddNewSourceBuffer(std::move(*fileOrErr), llvm::SMLoc());
return parseSourceFile(sourceMgr, block, context, sourceFileLoc, asmState);
}

Expand Down
4 changes: 1 addition & 3 deletions mlir/test/IR/invalid-unregistered.mlir
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// RUN: mlir-opt %s -split-input-file -verify-diagnostics

// REQUIRES: noasserts

// expected-error @below {{op created with unregistered dialect}}
// expected-error @below {{operation being parsed with an unregistered dialect}}
"unregistered_dialect.op"() : () -> ()

// -----
Expand Down

0 comments on commit 05fb260

Please sign in to comment.