fix: guard eaglerinliner with detecting a valid circuit

[tianrui-wei]

A crash could as here NLATable assumes each module always contains a CircuitOp. However, this assumption might not be valid as the reduction process goes on. This commit adds a guard to check for a circuit before creating the NLATable or skipping inlining when no circuit exists.

A corresponding stach trace (which this PR fixes)

circt-reduce: /scratch/tianruiwei/tmp/circt/llvm/llvm/include/llvm/Support/Casting.h:578: decltype(auto) llvm::cast(From*) [with To = circt::firrtl::CircuitOp; From = mlir::Operation]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
PLEASE submit a bug report to https://github.com/llvm/circt and include the crash backtrace.
Stack dump:
0.      Program arguments: circt-reduce v3.mlirbc -keep-best -o Reduced.mlirbc --exclude=canonicalize -emit-bytecode -test=test.sh
 #0 0x000056c982349fb9 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /scratch/tianruiwei/tmp/circt/llvm/llvm/lib/Support/Unix/Signals.inc:838:3
 #1 0x000056c982346f84 llvm::sys::RunSignalHandlers() /scratch/tianruiwei/tmp/circt/llvm/llvm/lib/Support/Signals.cpp:104:20
 #2 0x000056c98234779c SignalHandler(int, siginfo_t*, void*) /scratch/tianruiwei/tmp/circt/llvm/llvm/lib/Support/Unix/Signals.inc:426:14
 #3 0x000079774603e540 (/usr/lib/libc.so.6+0x3e540)
 #4 0x000079774609894c (/usr/lib/libc.so.6+0x9894c)
 #5 0x000079774603e410 raise (/usr/lib/libc.so.6+0x3e410)
 #6 0x000079774602557a abort (/usr/lib/libc.so.6+0x2557a)
 #7 0x00007977460254e3 __assert_perror_fail (/usr/lib/libc.so.6+0x254e3)
 #8 0x000056c9822ec48f bool llvm::DenseMapBase<llvm::DenseMap<mlir::StringAttr, mlir::Operation*, llvm::DenseMapInfo<mlir::StringAttr, void>, llvm::detail::DenseMapPair<mlir::StringAttr, mlir::Operation*>>, mlir::StringAttr, mlir::Operation*, llvm::DenseMapInfo<mlir::StringAttr, void>, llvm::detail::DenseMapPair<mlir::StringAttr, mlir::Operation*>>::LookupBucketFor<mlir::StringAttr>(mlir::StringAttr const&, llvm::detail::DenseMapPair<mlir::StringAttr, mlir::Operation*>*&) /scratch/tianruiwei/tmp/circt/llvm/llvm/include/llvm/Support/Casting.h:578:3
 #9 0x000056c9822ec48f std::pair<llvm::detail::DenseMapPair<mlir::StringAttr, mlir::Operation*>*, bool> llvm::DenseMapBase<llvm::DenseMap<mlir::StringAttr, mlir::Operation*, llvm::DenseMapInfo<mlir::StringAttr, void>, llvm::detail::DenseMapPair<mlir::StringAttr, mlir::Operation*>>, mlir::StringAttr, mlir::Operation*, llvm::DenseMapInfo<mlir::StringAttr, void>, llvm::detail::DenseMapPair<mlir::StringAttr, mlir::Operation*>>::lookupOrInsertIntoBucket<mlir::StringAttr>(mlir::StringAttr&&) /scratch/tianruiwei/tmp/circt/llvm/llvm/include/llvm/ADT/DenseMap.h:455:24
#10 0x000056c9822ec48f llvm::DenseMapBase<llvm::DenseMap<mlir::StringAttr, mlir::Operation*, llvm::DenseMapInfo<mlir::StringAttr, void>, llvm::detail::DenseMapPair<mlir::StringAttr, mlir::Operation*>>, mlir::StringAttr, mlir::Operation*, llvm::DenseMapInfo<mlir::StringAttr, void>, llvm::detail::DenseMapPair<mlir::StringAttr, mlir::Operation*>>::operator[](mlir::StringAttr&&) /scratch/tianruiwei/tmp/circt/llvm/llvm/include/llvm/ADT/DenseMap.h:327:36
#11 0x000056c9822ec48f circt::firrtl::NLATable::NLATable(mlir::Operation*) (.cold) /scratch/tianruiwei/tmp/circt/lib/Dialect/FIRRTL/NLATable.cpp:28:41
#12 0x000056c98310760d std::__uniq_ptr_impl<circt::firrtl::NLATable, std::default_delete<circt::firrtl::NLATable>>::reset(circt::firrtl::NLATable*) /usr/include/c++/15.2.1/bits/unique_ptr.h:202:16
#13 0x000056c98310760d std::__uniq_ptr_impl<circt::firrtl::NLATable, std::default_delete<circt::firrtl::NLATable>>::operator=(std::__uniq_ptr_impl<circt::firrtl::NLATable, std::default_delete<circt::firrtl::NLATable>>&&) /usr/include/c++/15.2.1/bits/unique_ptr.h:185:7
#14 0x000056c98310760d std::__uniq_ptr_data<circt::firrtl::NLATable, std::default_delete<circt::firrtl::NLATable>, true, true>::operator=(std::__uniq_ptr_data<circt::firrtl::NLATable, std::default_delete<circt::firrtl::NLATable>, true, true>&&) /usr/include/c++/15.2.1/bits/unique_ptr.h:237:24
#15 0x000056c98310760d std::unique_ptr<circt::firrtl::NLATable, std::default_delete<circt::firrtl::NLATable>>::operator=(std::unique_ptr<circt::firrtl::NLATable, std::default_delete<circt::firrtl::NLATable>>&&) /usr/include/c++/15.2.1/bits/unique_ptr.h:409:19
#16 0x000056c98310760d (anonymous namespace)::EagerInliner::beforeReduction(mlir::ModuleOp) /scratch/tianruiwei/tmp/circt/lib/Dialect/FIRRTL/FIRRTLReductions.cpp:1137:45
#17 0x000056c9823024a3 execute /scratch/tianruiwei/tmp/circt/tools/circt-reduce/circt-reduce.cpp:271:30
#18 0x000056c9823024a3 main /scratch/tianruiwei/tmp/circt/tools/circt-reduce/circt-reduce.cpp:532:14
#19 0x0000797746027675 (/usr/lib/libc.so.6+0x27675)
#20 0x0000797746027729 __libc_start_main (/usr/lib/libc.so.6+0x27729)
#21 0x000056c982305f95 _start (/scratch/tianruiwei/tmp/circt/build/bin/circt-reduce+0x1cef95)

[fabianschuiki]

Thanks for finding and fixing this!

[fabianschuiki]

I wonder if we could create a dense map of multiple NLATables, one for each circuit. That would allow us to run this reduction on inputs with multiple circuit ops. Maybe something like this works:

nlaTables.clear();
for (auto circuit : op.getOps<CircuitOp>())
  nlaTables.insert({circuit, std::make_unique<NLATable>(circuit)});

We could then define the tables using a map instead of a single unique pointer:

DenseMap<CircuitOp, std::unique_ptr<NLATable>> nlaTables;

And inside the reduction when we need the NLATable, we can look it up using the local circuit:

auto &nlaTable = nlaTables.lookup(op.getParentOfType<CircuitOp>());
if (!nlaTable)
  return 0;

What do you think about this?

[tianrui-wei]

Sounds good! I'll implement your suggestions and test it on my local setup :)

[tianrui-wei]

@fabianschuiki Sorry for the delay in response, I've had a little trouble getting it to work properly. Let me know if you have any othe rsuggestions :)

[fabianschuiki]

LGTM! Sorry for taking so long to get back to this!