[mlir] Dialect conversion passes try to rewrite top-level op

[rkayaith]

Some dialect conversion passes will attempt to rewrite top-level ops, for example convert-func-to-spirv is op-agnostic and has a pattern that rewrites func.func -> spirv.func. Since mlir::applyPartialConversion adds the parent op to the worklist, the pattern applies and we get a crash:

// mlir-opt %s -no-implicit-module -convert-func-to-spirv -debug-only=dialect-conversion
func.func private @foo()

//===-------------------------------------------===//
Legalizing operation : 'func.func'(0x4fadff0) {
  * Fold {
  } -> FAILURE : unable to fold

  * Pattern : 'func.func -> ()' {
    ** Insert  : 'spirv.func'(0x4fbff00)
    ** Erase   : 'func.func'(0x4fadff0)

    //===-------------------------------------------===//
    Legalizing operation : 'spirv.func'(0x4fbff00) {
    } -> SUCCESS : operation marked legal by the target
    //===-------------------------------------------===//
  } -> SUCCESS : pattern applied successfully
} -> SUCCESS
//===-------------------------------------------===//
func.func private @foo()

free(): double free detected in tcache 2
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.      Program arguments: mlir-opt /home/rk/tmp/test.mlir --convert-func-to-spirv --no-implicit-module -debug-only=dialect-conversion
 #0 0x0000000001305a77 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/rk/tmp/llvm-project/llvm/lib/Support/Unix/Signals.inc:567:13
 #1 0x0000000001303d20 llvm::sys::RunSignalHandlers() /home/rk/tmp/llvm-project/llvm/lib/Support/Signals.cpp:105:18
 #2 0x000000000130638f SignalHandler(int) /home/rk/tmp/llvm-project/llvm/lib/Support/Unix/Signals.inc:412:1
 #3 0x00007f1f32fa6420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #4 0x00007f1f3126f00b raise /build/glibc-SzIz7B/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x00007f1f3124e859 abort /build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:81:7
 #6 0x00007f1f312b926e __libc_message /build/glibc-SzIz7B/glibc-2.31/libio/../sysdeps/posix/libc_fatal.c:155:5
 #7 0x00007f1f312c12fc /build/glibc-SzIz7B/glibc-2.31/malloc/malloc.c:5348:3
 #8 0x00007f1f312c2f6d _int_free /build/glibc-SzIz7B/glibc-2.31/malloc/malloc.c:4201:3
 #9 0x00000000028efef0 llvm::DenseMap<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> >, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> > > >::getNumBuckets() const /home/rk/tmp/llvm-project/llvm/include/llvm/ADT/DenseMap.h:861:12
#10 0x00000000028efef0 llvm::DenseMapBase<llvm::DenseMap<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> >, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> > > >, llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> >, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> > > >::getNumBuckets() const /home/rk/tmp/llvm-project/llvm/include/llvm/ADT/DenseMap.h:525:49
#11 0x00000000028efef0 llvm::DenseMapBase<llvm::DenseMap<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> >, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> > > >, llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> >, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> > > >::destroyAll() /home/rk/tmp/llvm-project/llvm/include/llvm/ADT/DenseMap.h:361:9
#12 0x00000000028efef0 llvm::DenseMap<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> >, llvm::DenseMapInfo<llvm::StringRef, void>, llvm::detail::DenseMapPair<llvm::StringRef, std::unique_ptr<mlir::AsmResourceParser, std::default_delete<mlir::AsmResourceParser> > > >::~DenseMap() /home/rk/tmp/llvm-project/llvm/include/llvm/ADT/DenseMap.h:754:11
#13 0x00000000028efef0 mlir::ParserConfig::~ParserConfig() /home/rk/tmp/llvm-project/mlir/include/mlir/IR/AsmState.h:458:7
#14 0x00000000028efef0 performActions(llvm::raw_ostream&, bool, bool, std::shared_ptr<llvm::SourceMgr> const&, mlir::MLIRContext*, llvm::function_ref<mlir::LogicalResult (mlir::PassManager&)>, bool, bool) /home/rk/tmp/llvm-project/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp:105:1
#15 0x00000000028efa89 processBuffer(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, bool, bool, bool, bool, bool, bool, llvm::function_ref<mlir::LogicalResult (mlir::PassManager&)>, mlir::DialectRegistry&, llvm::ThreadPool*) /home/rk/tmp/llvm-project/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp:138:12
#16 0x00000000028efa89 mlir::MlirOptMain(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::function_ref<mlir::LogicalResult (mlir::PassManager&)>, mlir::DialectRegistry&, bool, bool, bool, bool, bool, bool, bool)::$_0::operator()(std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::raw_ostream&) const /home/rk/tmp/llvm-project/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp:180:12
#17 0x00000000028efa89 mlir::LogicalResult llvm::function_ref<mlir::LogicalResult (std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::raw_ostream&)>::callback_fn<mlir::MlirOptMain(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::function_ref<mlir::LogicalResult (mlir::PassManager&)>, mlir::DialectRegistry&, bool, bool, bool, bool, bool, bool, bool)::$_0>(long, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::raw_ostream&) /home/rk/tmp/llvm-project/llvm/include/llvm/ADT/STLFunctionalExtras.h:45:12
#18 0x0000000002966adb llvm::function_ref<mlir::LogicalResult (std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::raw_ostream&)>::operator()(std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::raw_ostream&) const /home/rk/tmp/llvm-project/llvm/include/llvm/ADT/STLFunctionalExtras.h:68:12
#19 0x0000000002966adb mlir::splitAndProcessBuffer(std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::function_ref<mlir::LogicalResult (std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::raw_ostream&)>, llvm::raw_ostream&, bool, bool) /home/rk/tmp/llvm-project/mlir/lib/Support/ToolUtilities.cpp:28:12
#20 0x00000000028ee65e mlir::MlirOptMain(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete<llvm::MemoryBuffer> >, llvm::function_ref<mlir::LogicalResult (mlir::PassManager&)>, mlir::DialectRegistry&, bool, bool, bool, bool, bool, bool, bool) /home/rk/tmp/llvm-project/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp:185:10
#21 0x00000000028eeaed 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, bool, bool, bool) /home/rk/tmp/llvm-project/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp:208:10
#22 0x00000000028eeaed mlir::MlirOptMain(int, char**, llvm::StringRef, mlir::DialectRegistry&, bool) /home/rk/tmp/llvm-project/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp:305:14
#23 0x000000000129d7a0 mlir::LogicalResult::succeeded() const /home/rk/tmp/llvm-project/mlir/include/mlir/Support/LogicalResult.h:41:35
#24 0x000000000129d7a0 mlir::asMainReturnCode(mlir::LogicalResult) /home/rk/tmp/llvm-project/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h:97:12
#25 0x000000000129d7a0 main /home/rk/tmp/llvm-project/mlir/tools/mlir-opt/mlir-opt.cpp:249:10
#26 0x00007f1f31250083 __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:342:3
#27 0x000000000129d4ae _start (/home/rk/tmp/llvm-project/build/bin/mlir-opt+0x129d4ae)
Aborted (core dumped)

[llvmbot]

@llvm/issue-subscribers-mlir

[rikhuijzer]

I've tried to fix this, but give up for now. I'm unable to pinpoint where the crash occurs exactly. I suspect that the operation gets converted successfully, but is slightly wrong. Then, when converting the object to a textual representation, it fails. I suspect this because

$ temp.mlir -no-implicit-module -convert-func-to-spirv

crashes on OperationVerfier::verifyOpAndDominance, according to the stacktrace, whereas the version with the debug flag

$ temp.mlir -no-implicit-module -convert-func-to-spirv -debug

crashes on Operation::print.

What might be related is that op.getAttrDictionary().print(llvm::outs()) shows

{function_type = () -> (), sym_name = "foo", sym_visibility = "private"}

in the -no-implicit-module mode, whereas it shows

{function_control = #spirv.function_control<None>, function_type = () -> (), sym_name = "foo", sym_visibility = "private"}

in the default mode.

[rkayaith]

I'm unable to pinpoint where the crash occurs exactly.

Replacing the top-level op means the operation memory is freed, while other objects (e.g. the pass manager) are still referencing it. The point where it actually crashes isn't too important here, the issue is that the pass is replacing its current op in the first place. This violates the first restriction here:

Must not modify any state referenced or relied upon outside the current operation being operated on. This includes adding or removing operations from the parent block [...]

(the -no-implicit-module option just makes the issue more obvious, since it results in a crash)

This is an API issue with mlir::applyPartialConversion (and similar functions) imo, I think applyConversion(op) should only convert ops under op, and not op itself. This would be consistent with applyPatternsAndFoldGreedily

[joker-eph]

I was discussing this with River, we likely should add an entry point for applyPartialConversion that takes regions instead (like applyPatternsAndFoldGreedily has), which could be used by the passes instead of passing the op their operating on.