[circt-reduce][FIRRTL] firrtl-remove-unused-ports intsance choice support

[seldridge]

The firrtl-remove-unused-ports reduction does not work if there are instance choices involved. Consider the following:

firrtl.circuit "Foo" {
  firrtl.option @Target attributes {sym_visibility = "private"} {
    firrtl.option_case @FPGA
  }
  firrtl.extmodule private @Divider_ASIC(
    in in: !firrtl.clock,
    out out: !firrtl.clock
  )
  firrtl.module private @Divider_FPGA(
    in %in: !firrtl.clock,
    out %out: !firrtl.clock
  ) {
  }
  firrtl.module @Foo() {
    %divider_in, %divider_out = firrtl.instance_choice divider @Divider_ASIC alternatives @Target { @FPGA -> @Divider_FPGA} (in in: !firrtl.clock, out out: !firrtl.clock)
    %invalid_clock = firrtl.invalidvalue : !firrtl.clock
    firrtl.matchingconnect %divider_in, %invalid_clock : !firrtl.clock
  }
}

This fails with:

# circt-reduce -test /usr/bin/echo --test-arg hello -include firrtl-remove-unused-ports reduced.0.mlir
Reading input
Testing input with `/usr/bin/echo`
  with argument `hello`
hello /tmp/circt-reduce-e77af0.mlir
Initial module has size 669
Trying reduction `firrtl-remove-unused-ports`
circt-reduce: /home/schuylere/scratch/repos/github.com/llvm/circt/llvm/llvm/include/llvm/Support/Casting.h:566: decltype(auto) llvm::cast(From&) [with To = circt::firrtl::InstanceOp; 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 -test /usr/bin/echo --test-arg hello -include firrtl-remove-unused-ports reduced.0.mlir
 #0 0x0000000000725f1b llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x725f1b)
 #1 0x0000000000722e1b llvm::sys::RunSignalHandlers() (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x722e1b)
 #2 0x0000000000722f49 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007fa32b3a2990 __restore_rt (/lib64/libpthread.so.0+0x12990)
 #4 0x00007fa32a1a552f raise (/lib64/libc.so.6+0x4e52f)
 #5 0x00007fa32a178e65 abort (/lib64/libc.so.6+0x21e65)
 #6 0x00007fa32a178d39 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d39)
 #7 0x00007fa32a19de86 (/lib64/libc.so.6+0x46e86)
 #8 0x000000000192807a (anonymous namespace)::RemoveUnusedPortsPass::removeUnusedModulePorts(circt::firrtl::FModuleOp, circt::igraph::InstanceGraphNode*) RemoveUnusedPorts.cpp:0:0
 #9 0x000000000192947e (anonymous namespace)::RemoveUnusedPortsPass::runOnOperation() RemoveUnusedPorts.cpp:0:0
#10 0x0000000001ac6979 mlir::detail::OpToOpPassAdaptor::run(mlir::Pass*, mlir::Operation*, mlir::AnalysisManager, bool, unsigned int) (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x1ac6979)
#11 0x0000000001ac6c96 mlir::detail::OpToOpPassAdaptor::runPipeline(mlir::OpPassManager&, mlir::Operation*, mlir::AnalysisManager, bool, unsigned int, mlir::PassInstrumentor*, mlir::PassInstrumentation::PipelineParentInfo const*) (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x1ac6c96)
#12 0x0000000001ac89d5 mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl(bool)::'lambda'(mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl(bool)::OpPMInfo&)::operator()(mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl(bool)::OpPMInfo&) const Pass.cpp:0:0
#13 0x0000000001ac574d mlir::detail::OpToOpPassAdaptor::runOnOperationAsyncImpl(bool) (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x1ac574d)
#14 0x0000000001ac6802 mlir::detail::OpToOpPassAdaptor::run(mlir::Pass*, mlir::Operation*, mlir::AnalysisManager, bool, unsigned int) (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x1ac6802)
#15 0x0000000001ac6c96 mlir::detail::OpToOpPassAdaptor::runPipeline(mlir::OpPassManager&, mlir::Operation*, mlir::AnalysisManager, bool, unsigned int, mlir::PassInstrumentor*, mlir::PassInstrumentation::PipelineParentInfo const*) (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x1ac6c96)
#16 0x0000000001ac73bd mlir::PassManager::runPasses(mlir::Operation*, mlir::AnalysisManager) (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x1ac73bd)
#17 0x0000000001ac8756 mlir::PassManager::run(mlir::Operation*) (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x1ac8756)
#18 0x00000000006cb5d4 main (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x6cb5d4)
#19 0x00007fa32a1917e5 __libc_start_main (/lib64/libc.so.6+0x3a7e5)
#20 0x00000000006d00be _start (/scratch/schuylere/repos/github.com/llvm/circt/build-gcc-g++-Release-on/bin/circt-reduce+0x6d00be)
Aborted

[uenoku]

I think we should have removed firrtl-remove-unused-ports a long time ago. Only missing feature of IMDCE is "ignore dontTouch"

[seldridge]

IMDCE also needs to gain the ability to run partially, or to not completely rip everything out. See: #10055

circt-reduce will frequently create dead code which is a valid reduction, but which IMDCE will completely remove. Hence, just using IMDCE has been an insufficient replacement as you can't use it to only remove some things which is often the trick to getting a tighter reduction.

Any bright ideas?

[uenoku]

I think the easiest implementation would be to mark all operations except for instance op/block arguments in the initialization.
In that way only module ports will be properly removed.