[SymbolDCE] Hierpath only used by OMIRTracker should be removed

[uenoku]

Currently OMIRTracker is considered as a valid user of a symbol, which causes suboptimal result regarding SymbolDCE, e.g:

  firrtl.circuit "Top" {
    hw.hierpath private @nla_1 [@Foo::@c, @C]
    firrtl.module private @C() attributes {annotations = [{circt.nonlocal = @nla_1, class = "freechips.rocketchip.objectmodel.OMIRTracker", id = 0 : i64}]} {
    }
    firrtl.module @Foo() {
      firrtl.instance c sym @c @C()
    }
    firrtl.module @Top() {
      firrtl.instance foo @Foo()
    }
  }

circt-opt -symbol-dce -firrtl-inner-symbol-dce bar.mlir should remove @nla_1, but we cannot remove right now.

[dtzSiFive]

This is a valid use, and should not be removed by (Inner)SymbolDCE. All uses are valid uses, and regardless of "validity" there's nothing dead about the symbols here.

I'm uncertain on what basis this annotation is "dead", here or generally, but determining this is not the responsibility of these passes. If we need to remove various annotations, OMIR or otherwise, this should be handled directly in its own capacity. This is useful/worth capturing regardless and once accomplished will open up these symbols to be removed when dead.

If for some reason OMIR has annotations whose "deadness" must somehow be computed as part of a global computation of dead symbols (??), we can find a way to accomplish this, LMK, but that would still be a new thing and not part of these passes (although may warrant finding a way to reuse their logic).

~Will

On Fri, Apr 21, 2023, 4:39 PM Hideto Ueno @.***> wrote:

Currently OMIRTracker is considered as a valid user of a symbol, which causes suboptimal result regarding SymbolDCE, e.g:

firrtl.circuit "Top" { hw.hierpath private @nla_1 @.**@., @C] firrtl.module private @C() attributes {annotations = [{circt.nonlocal = @nla_1, class = "freechips.rocketchip.objectmodel.OMIRTracker", id = 0 : i64}]} { } firrtl.module @Foo() { firrtl.instance c sym @c @C() } firrtl.module @Top() { firrtl.instance foo @Foo() } }

circt-opt -symbol-dce -firrtl-inner-symbol-dce bar.mlir should remove @nla_1, but we cannot remove right now.

— Reply to this email directly, view it on GitHub https://github.com/llvm/circt/issues/5067, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYZWN5ONB3H6TF6RDATBLZDXCMLERANCNFSM6AAAAAAXHNWIRY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[uenoku]

Thanks! My understanding for this problem was incorrect :( It looks like there is something wrong in IMDCE about propagating context-sensitive instance port liveness.