Assert behavior on unregistered dialect ops

Trying to run something like this (which would be very common during experimentation, demos, etc.) leads to an assert/crash. While the error message on the top is pretty clear, it looks bad to assert here (even on an assert build which is when this is triggered) instead of a clean and short emitError with line number info (which is also missing here). Is an earlier check missing before the Operation ctor gets called and isn’t this a “bug” to be fixed?

$ bin/mlir-opt -canonicalize ../mlir/test/Transforms/canonicalize.mlir 
LLVM ERROR: foo 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
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: bin/mlir-opt -canonicalize ../mlir/test/Transforms/canonicalize.mlir
1.	MLIR Parser: custom op parser 'builtin.func'
2.	MLIR Parser: custom op parser 'affine.for'
3.	MLIR Parser: custom op parser 'affine.for'
4.	MLIR Parser: custom op parser 'affine.for'
 #0 0x0000000000b9ee5f PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
 #1 0x0000000000b9c6ee SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f4a3c9d5b20 __restore_rt sigaction.c:0:0
 #3 0x00007f4a3b6a537f raise (/lib64/libc.so.6+0x3737f)
 #4 0x00007f4a3b68fdb5 abort (/lib64/libc.so.6+0x21db5)
 #5 0x0000000000aaaa23 llvm::report_fatal_error(llvm::Twine const&, bool) (bin/mlir-opt+0xaaaa23)
 #6 0x0000000001a0b080 mlir::Operation::Operation(mlir::Location, mlir::OperationName, unsigned int, unsigned int, unsigned int, mlir::DictionaryAttr, bool) (bin/mlir-opt+0x1a0b080)
 #7 0x0000000001a0ddf1 mlir::Operation::create(mlir::Location, mlir::OperationName, mlir::TypeRange, mlir::ValueRange, mlir::DictionaryAttr, mlir::BlockRange, unsigned int) (bin/mlir-opt+0x1a0ddf1)
 #8 0x0000000001a0e248 mlir::Operation::create(mlir::Location, mlir::OperationName, mlir::TypeRange, mlir::ValueRange, mlir::DictionaryAttr, mlir::BlockRange, mlir::RegionRange) (bin/mlir-opt+0x1a0e248)
 #9 0x0000000001a0e583 mlir::Operation::create(mlir::OperationState const&) (bin/mlir-opt+0x1a0e583)
#10 0x00000000019a2b3f mlir::OpBuilder::createOperation(mlir::OperationState const&) (bin/mlir-opt+0x19a2b3f)
#11 0x000000000185ceef (anonymous namespace)::OperationParser::parseGenericOperation() Parser.cpp:0:0
#12 0x000000000185d728 (anonymous namespace)::OperationParser::parseOperation() Parser.cpp:0:0
...
#33 0x00000000019c3307 mlir::FuncOp::parse(mlir::OpAsmParser&, mlir::OperationState&) (bin/mlir-opt+0x19c3307)
#34 0x000000000185d51a (anonymous namespace)::OperationParser::parseOperation() Parser.cpp:0:0
#35 0x000000000185eb97 mlir::parseSourceFile(llvm::SourceMgr const&, mlir::Block*, mlir::MLIRContext*, mlir::LocationAttr*, mlir::AsmParserState*) (bin/mlir-opt+0x185eb97)
#36 0x0000000001832d3b performActions(llvm::raw_ostream&, bool, bool, llvm::SourceMgr&, mlir::MLIRContext*, mlir::PassPipelineCLParser const&) (.constprop.208) MlirOptMain.cpp:0:0
#37 0x0000000001833adc processBuffer(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, bool, bool, bool, bool, mlir::PassPipelineCLParser const&, mlir::DialectRegistry&, llvm::ThreadPool&) MlirOptMain.cpp:0:0
#38 0x0000000001833f64 mlir::MlirOptMain(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, mlir::PassPipelineCLParser const&, mlir::DialectRegistry&, bool, bool, bool, bool, bool) (bin/mlir-opt+0x1833f64)
#39 0x00000000018359ce mlir::MlirOptMain(int, char**, llvm::StringRef, mlir::DialectRegistry&, bool) (bin/mlir-opt+0x18359ce)
#40 0x0000000000aa4365 main (bin/mlir-opt+0xaa4365)
#41 0x00007f60d7809493 __libc_start_main (/lib64/libc.so.6+0x23493)
#42 0x0000000000a9daee _start (bin/mlir-opt+0xa9daee)
Aborted (core dumped)

It’s pretty common to miss using -allow-unregistered-dialect when testing.

The stacks trace seems not very useful when you know it’s coming from a testing tool parsing an input file, but I think that it is helpful when it points to where this is created from a pass.

It is indeed helpful and perhaps the expected behavior when such an op is being created programmatically (outside of the parser route) but this assert looks like a “bug” or undesirable when the op is being created from input being parsed: a parser error would appear to be the expected behavior in such a case. That again leads to the question I posed in the OP as to whether the parser route is missing an earlier “check and emit error” to prevent this assertion.

Sorry I had missed your question earlier, feel free to send me a patch for adding a friendlier check in parseGenericOperation, otherwise I may have time to look at this later this weekend.

This has been fixed and merged here: ⚙ D111628 [MLIR] Fix assert crash when an unregistered dialect op is encountered

1 Like