[FIRRTL] Grand Central DataTap Needs to Tap in Parent Module

[seldridge]

Currently, if you try to data tap into the current module, the generated XMR will have no hierarchy. I don't think this will resolve correctly. Instead, there should be an additional level of hierarchy.

Consider (and sorry for the verbosity) the following. This is trying to tap ~Top|GCTDataTap>r:

circuit Top : %[
[
  {
    "class": "sifive.enterprise.grandcentral.DataTapsAnnotation",
    "blackBox": "~Top|DataTap",
    "keys": [
      {
        "class": "sifive.enterprise.grandcentral.ReferenceDataTapKey",
        "source": "~Top|GCTDataTap>r",
        "portName": "~Top|DataTap>_0"
      }
    ]
  }
]]
  extmodule DataTap :
    output _0 : UInt<1>

    defname = DataTap

  module Bar:
    input clock: Clock
    input a: UInt<1>
    output b: UInt<1>

    reg r : UInt<1>, clock

    r <= a
    b <= r

  module GCTDataTap :
    input clock : Clock
    input a : UInt<1>
    output b : UInt<1>

    reg r : UInt<1>, clock

    inst bar of Bar
    bar.clock <= clock
    bar.a <= a
    r <= bar.b
    b <= r

    wire cloneType : UInt<1>
    inst DataTap of DataTap
    DataTap._0 is invalid
    cloneType <= DataTap._0

  module Top:
    input clock: Clock
    input a: UInt<1>
    output b: UInt<1>

    inst DUT of GCTDataTap
    DUT.clock <= clock
    DUT.a <= a
    b <= DUT.b

The generated XMR is:

module DataTap_impl_0(
  output _0);

  assign _0 = r;        // gct/DataTap.fir:15:13
endmodule

And for confirmation that this is a problem, Verilator can't find r:

%Error: /dev/stdin:37:15: Can't find definition of variable: 'r'
   37 |   assign _0 = r;  
      |               ^
%Error: Exiting due to 1 error(s)

However, if I instead tap ~Top|Bar>r, everything works. This produces the following Verilog and Verilator is happy:

module DataTap_impl_0(
  output _0);

  assign _0 = bar.r;    // gct/DataTap.fir:15:13
endmodule

(Also, who knew that Verilator could handle hierarchical references? To test this, I'm just using firtool Foo.fir | verilator --lint-only /dev/stdin.)

[fabianschuiki]

This is pretty interesting. We might want to prefix the hierarchical name with the module name of the parent at which we want to start descending down. That should make all these cases unambiguous.

There's a larger problem here of what happens when the DataTap module wants a port called bar. That would shadow the bar in the parent module that bar.r wants to look into. We probably want GCTDataTap.bar.r in this case.

[seldridge]

This is fixed on main, though I'm not sure exactly where/when. The current output with the correct XMR is:

module DataTap_impl_0(
  output _0);

  assign _0 = GCTDataTap.r;
endmodule