[FIRRTL] Question about different Annotation handling between SFC and firtool at Inline phase

Nobody has claimed this yet.

Related PR: https://github.com/llvm/circt/pull/2435

Sorry for bringing back old and dead SFC, but I'm rather curious about the reason 'diverged' behaviour between SFC and CIRCT at the inlining pass. I have a old custom SFC transform and want to migrate it to CIRCT. However, turns out CIRCT's `ModuleInliner` ([ref](https://github.com/llvm/circt/blob/main/lib/Dialect/FIRRTL/Transforms/ModuleInliner.cpp)) *consumes* Annotation attached to module when SFC preserved it by 'hoisting' it up to its parent.

# Minimal example

Consider this simple Chisel code:

```scala
case class SampleTargetAnnotation(target: Target)
extends SingleTargetAnnotation[Target] {
override def duplicate(n: Target): Annotation = this.copy(target = n)
}

class AdderBundle extends Bundle {
val in1 = Input(UInt(1.W))
val in2 = Input(UInt(1.W))
val out = Output(UInt(2.W))
}

class DUT extends RawModule with InlineInstance {
val io = IO(new AdderBundle)
io.out := io.in1 + io.in2
}

class DUTWrapper extends RawModule {
val io = IO(new AdderBundle)

val dut = Module(new DUT)
dut.io.in1 := io.in1
dut.io.in2 := io.in2
io.out := dut.io.out

// Before https://github.com/chipsalliance/chisel/pull/4717
annotate {
new ChiselAnnotation {
def toFirrtl: Annotation = SampleTargetAnnotation(dut.toAbsoluteTarget)
}
}

// After that PR
annotate(this)(Seq(SampleTargetAnnotation(dut.toAbsoluteTarget)))
}
```

Both yield corresponding CHIRRTL output. Only picked up relevant annotations:

## CHIRRTL
### Chisel 3.6
```
circuit DUTWrapper :
module DUT :
output io : { flip in1 : UInt<1>, flip in2 : UInt<1>, out : UInt<2>}

node _io_out_T = add(io.in1, io.in2)
node _io_out_T_1 = tail(_io_out_T, 1)
io.out <= _io_out_T_1

module DUTWrapper :
output io : { flip in1 : UInt<1>, flip in2 : UInt<1>, out : UInt<2>}

io is invalid
inst dut of DUT
dut.io.in1 <= io.in1
dut.io.in2 <= io.in2
io.out <= dut.io.out
```
```json
{
"class": "firrtl.passes.InlineAnnotation",
"target": "DUTWrapper.DUT"
},
{
"class": "reprod.SampleTargetAnnotation",
"target": "~DUTWrapper|DUTWrapper/dut:DUT"
},
```

### Chisel 7
```
FIRRTL version 6.0.0
circuit DUTWrapper :%[[
{
"class":"firrtl.passes.InlineAnnotation",
"target":"DUTWrapper.DUT"
},
{
"class":"reprod.SampleTargetAnnotation",
"target":"~DUTWrapper|DUTWrapper/dut:DUT"
}
]]
module DUT :
output io : { flip in1 : UInt<1>, flip in2 : UInt<1>, out : UInt<2>}

node _io_out_T = add(io.in1, io.in2)
node _io_out_T_1 = tail(_io_out_T, 1)
connect io.out, _io_out_T_1

module DUTWrapper :
output io : { flip in1 : UInt<1>, flip in2 : UInt<1>, out : UInt<2>}

inst dut of DUT
connect dut.io.in1, io.in1
connect dut.io.in2, io.in2
connect io.out, dut.io.out
```

And after corresponding inline pass, notice how that annotation's `target` gets changed:

## Annotation target changes
### SFC (firrtl.passes.InlineInstances)

`target` got hoisted, from `~DUTWrapper|DUTWrapper/dut:DUT` to `~DUTWrapper|DUTWrapper`.

```
{
"class":"reprod.SampleTargetAnnotation",
"target":"~DUTWrapper|DUTWrapper/dut:DUT"
}
======== Finished firrtl.passes.ResolveKinds ========
======== Starting firrtl.passes.InlineInstances ========
{
"class":"reprod.SampleTargetAnnotation",
"target":"~DUTWrapper|DUTWrapper"
},
```

### MLIR (firrtl-inliner)

`reprod.SampleTargetAnnotation` itself is now *gone*, with NLA.

```
// -----// IR Dump Before Inliner (firrtl-inliner) //----- //
firrtl.circuit "DUTWrapper" {
hw.hierpath private @nla [@DUTWrapper::@sym, @DUT]
firrtl.module private @DUT(in %io_in1: !firrtl.uint<1>, in %io_in2: !firrtl.uint<1>, out %io_out: !firrtl.uint<2>) attributes {annotations = [{class = "firrtl.passes.InlineAnnotation"}, {class = "firrtl.transforms.NoDedupAnnotation"}, {circt.nonlocal = @nla, class = "reprod.SampleTargetAnnotation"}]} { ... }
firrtl.module @DUTWrapper(in %io_in1: !firrtl.uint<1>, in %io_in2: !firrtl.uint<1>, out %io_out: !firrtl.uint<2>) attributes {convention = #firrtl} { ... }

// -----// IR Dump After Inliner (firrtl-inliner) //----- //
firrtl.circuit "DUTWrapper" {
firrtl.module @DUTWrapper(in %io_in1: !firrtl.uint<1>, in %io_in2: !firrtl.uint<1>, out %io_out: !firrtl.uint<2>) attributes {convention = #firrtl} { ... }
```

---

My guess is that,

1. IMDeadCodeElim can't remove empty module with `Annotation` attached, ends up with degraded verilog output quality:

```cpp
// https://github.com/llvm/circt/blob/23ee54fe33849916be2753d9960677daa7b4d6e9/lib/Dialect/FIRRTL/Transforms/IMDeadCodeElim.cpp#L829
if (!module.getAnnotations().empty()) {
module.emitWarning() << "module `" << module.getName()
<< "` is empty but cannot be removed "
"because the module has annotations "
<< module.getAnnotations();
return;
}
```

2. Rather it was semantically nonsense to record 'inlined' target (though personally I'm the one who believes at least leaving 'this module was inlined into sth' can help downstream applications).

It seems that the first one is a strongest reason, but any other is deeply appreciated, if any.

Compare firrtl.passes.InlineInstances with lib/Dialect/FIRRTL/Transforms/ModuleInliner.cpp, then inspect lib/Dialect/FIRRTL/Transforms/IMDeadCodeElim.cpp around line 829 and the annotation/NLA handling shown in the IR dumps. Determine whether the differing target behavior is intentional and record the expected annotation semantics or a narrowly scoped change.

Written by the indexing model from the issue text.