[firtool] New blackbox/external-module behavior in firtool-1.137.0

[tymcauley]

After updating from firtool-1.136.0 to firtool-1.137.0, I observed two changes in behavior to how external modules are handled, and I wasn't sure if they were intentional.

Setup

Let's say I have this project structure:

./
├── bb-a/
│   └── Bar.sv
├── bb-b/
│   ├── Bar.sv
│   └── Baz.sv
└── Foo.fir

With these files:

$ cat bb-a/Bar.sv
// bb-a/Bar.sv
module Bar;
endmodule
$ cat bb-b/Bar.sv
// bb-b/Bar.sv
module Bar;
$ cat bb-b/Baz.sv
// bb-b/Baz.sv
module Baz;
endmodule
endmodule
$ cat Foo.fir
FIRRTL version 6.0.0
circuit Foo :%[[
  {
    "class":"firrtl.transforms.BlackBoxPathAnno",
    "target":"~|Bar",
    "path":"bb-a/Bar.sv"
  },
  {
    "class":"firrtl.transforms.BlackBoxPathAnno",
    "target":"~|Baz",
    "path":"bb-b/Baz.sv"
  },
  {
    "class":"firrtl.transforms.BlackBoxPathAnno",
    "target":"~|Baz",
    "path":"bb-b/Bar.sv"
  }
]]
  layer Verification, bind, "verification" :
    layer Assert, bind, "verification/assert" :
      layer Temporal, inline :
    layer Assume, bind, "verification/assume" :
      layer Temporal, inline :
    layer Cover, bind, "verification/cover" :
      layer Temporal, inline :

  extmodule Bar :
    defname = Bar

  extmodule Baz :
    defname = Baz

  public module Foo :
    input clock : Clock
    input reset : UInt<1>

    inst bar of Bar
    inst baz of Baz

So, I've got a duplicate definition of the SystemVerilog Bar module in both sub-directories, and I've included bb-a/Bar.sv as the definition of the Bar FIRRTL module, and I've added bb-b/Bar.sv as a file for the Baz FIRRTL module, even those it's not going to be useful in the circuit definition.

Strict handling of duplicate module definitions

When I run this with firtool-1.136.0, it compiles, and I get this result:

$ ./firtool-1.136.0 -default-layer-specialization=enable Foo.fir
// Generated by CIRCT firtool-1.136.0
module Foo(
  input clock,
        reset
);

  Bar bar ();
  Baz baz ();
endmodule


// ----- 8< ----- FILE "./Bar.sv" ----- 8< -----

// Generated by CIRCT firtool-1.136.0
// bb-a/Bar.sv
module Bar;
endmodule
// ----- 8< ----- FILE "./Baz.sv" ----- 8< -----

// Generated by CIRCT firtool-1.136.0
// bb-b/Baz.sv
module Baz;
endmodule

But with firtool-1.137.0, the build fails:

$ ./firtool-1.137.0 -default-layer-specialization=enable Foo.fir
Foo.fir:30:3: error: redefinition of symbol named 'Bar.sv'
  extmodule Baz :
  ^
Foo.fir:30:3: note: see current operation:
"emit.file"() <{file_name = "./Bar.sv", sym_name = "Bar.sv"}> ({
  "emit.verbatim"() <{text = "// bb-a/Bar.sv\0Amodule Bar;\0Aendmodule\0A"}> : () -> ()
}) {output_file = #hw.output_file<"./Bar.sv">} : () -> ()
Foo.fir:27:3: note: see existing symbol definition here
  extmodule Bar :
  ^

Now, this isn't necessarily a bad change, but is this an expected change in behavior? Or a weird edge-case I'm hitting? Is this a change that we should communicate to users?

New unprocessed-annotation warning

If we run this same example with the -warn-on-unprocessed-annotations flag, there's no warning in firtool-1.136.0:

$ ./firtool-1.136.0 -default-layer-specialization=enable -warn-on-unprocessed-annotations Foo.fir
... same output as above

But with firtool-1.137.0:

$ ./firtool-1.137.0 -default-layer-specialization=enable -warn-on-unprocessed-annotations Foo.fir
Foo.fir:27:3: warning: unprocessed annotation:'circt.VerbatimBlackBoxAnno' still remaining after LowerToHW
  extmodule Bar :
  ^
... same error output as above

And if I update the project to remove bb-b/Bar.sv, and remove that annotation from Foo.fir, I still get the warning with a successful result:

$ ./firtool-1.137.0 -default-layer-specialization=enable -warn-on-unprocessed-annotations Foo.fir
Foo.fir:22:3: warning: unprocessed annotation:'circt.VerbatimBlackBoxAnno' still remaining after LowerToHW
  extmodule Bar :
  ^
// Generated by CIRCT firtool-1.137.0
module Foo(
  input clock,
        reset
);

  Bar bar ();
  Baz baz ();
endmodule


// ----- 8< ----- FILE "./Bar.sv" ----- 8< -----

// Generated by CIRCT firtool-1.137.0
// bb-a/Bar.sv
module Bar;
endmodule


// ----- 8< ----- FILE "./Baz.sv" ----- 8< -----

// Generated by CIRCT firtool-1.137.0
// bb-b/Baz.sv
module Baz;
endmodule

Is that warning expected?

Reproduction

Here's a folder containing those files, if you want to reproduce the same results:

blackbox-errors.tar.gz