CIRCT elaboration should uniquify top/model hierarchy about chipyard HOT 15 CLOSED

Just to make sure I understood correctly, we need separate file names for top/model because when running gate-level simulation the simulator does not know if it is using the time-annotated synthesized modules or just the verilog files?

from chipyard.

Not quite. Let me try to be clearer. Suppose we had this module hierarchy:

   Top    Model
  /   \   /   \
ModA   ModB   Mod C

There are instances of ModA and ModB in Top, and there are instances of ModB and ModC in Model.

In <long_name>.top.f:

Top.sv
ModA.sv
ModB.sv

In <long_name>.model.f:

Model.sv
ModB.sv
ModC.sv

So here's the problem sequence:

1. The files in <long_name>.top.f are given to synthesis. We get a single mapped gate-level netlist, say Top.mapped.v.
2. We then combine Top.mapped.v and the files in <long_name>.model.f for gate-level simulation.
3. Problem: ModB in Top.mapped.v collides with ModB in <long_name>.model.f.
4. Current hack in linked commit: remove ModB.sv from <long_name>.model.f.

Why this is not OK: the instance of ModB in Model now refers to the synthesized version of ModB. If the simulation is timing-annotated, the signals crossing the Top/Model boundary will have modified timing compared to the non-timing annotated simulation. Furthermore, if there are more shared modules, internally, the design under Model may have timing violations. In general, the testbench (Model) should not have any gate-level modules.

Desired solution:
In <long_name>.top.f:

Top.sv
ModA.sv
ModB_Top.sv

In <long_name>.model.f:

Model.sv
ModB_Model.sv
ModC.sv

Where the module name of is unique between the uniquified files and the instance declarations are properly changed.

TL;DR we need to restore the uniquification behavior of the old barstools Top/Harness separation pass.

from chipyard.

By instance declarations, do you mean that the module names inside the files should also be changes?
For instance, module Adder to module Adder_Top and module Adder_Model? In my understanding, the tools reads the <long_name>.<top or model>.f and works on those files so it seems like we don't need to change the module names inside the files.

from chipyard.

You do need to change the module names. This way, Adder_Top can be synthesized and simulated inside the Top, while Adder_Model remains behavioral and is simulated inside the Model.

I cannot use the tools (e.g. synthesis) to do this uniquification in Top's hierarchy - it cannot know what modules are also referenced in the Model. I would have to uniquify all of the modules under Top instead, which will break things (such as blackboxes)

from chipyard.

Hmmm I see. I'll look into this

from chipyard.

Bumping this - it needs to be fixed ASAP. We're running into gate-level synthesis unresolved modules when synthesis modifies a module (e.g. optimizes away a bit, creating a modified module name) or there are unsynthesizable modules (e.g. plusarg_reader used in TLMonitors). Thus, those modules that are also used by the TestHarness are no longer in the mapped Verilog file.

I think this is not quite correct:

Related to https://groups.google.com/g/chipyard/c/tXCmoNNpwwg.

from chipyard.

Sorry for the delay, will work on this today

from chipyard.

Closing this issue for now

from chipyard.

I'm still seeing multiple module declarations errors in post-syn simulation.
I might be wrong, but based on my observation it's because instances under DUT are not uniquified.

Taking AsyncQueueSink* for example, here are the error messages:

Error-[MPD] Module previously declared
The module was previously declared at:
"/bwrcq/C/allpan/chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/syn-rundir/ChipTop.mapped.v",
459025
It is redeclared later at:
"/bwrcq/C/allpan/chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/AsyncQueueSink_1.sv",
71: token is 'AsyncQueueSink_1'
module AsyncQueueSink_1(
^
Please remove one of the declarations and compile again.


Error-[MPD] Module previously declared
The module was previously declared at:
"/bwrcq/C/allpan/chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/syn-rundir/ChipTop.mapped.v",
458272
It is redeclared later at:
"/bwrcq/C/allpan/chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/AsyncQueueSink_2.sv",
71: token is 'AsyncQueueSink_2'
module AsyncQueueSink_2(
^
Please remove one of the declarations and compile again.


Error-[MPD] Module previously declared
The module was previously declared at:
"/bwrcq/C/allpan/chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/syn-rundir/ChipTop.mapped.v",
460013
It is redeclared later at:
"/bwrcq/C/allpan/chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/AsyncQueueSink.sv",
71: token is 'AsyncQueueSink'
module AsyncQueueSink(
^
Please remove one of the declarations and compile again.

In sim-inputs.yml, I'm seeing these lines:

sim.inputs:
  top_module: ChipTop
  tb_name: ''
  input_files:
    - "/bwrcq/C/allpan/chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/AsyncQueueSink_1.sv"
    - "/bwrcq/C/allpan/chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/AsyncQueueSink_2.sv"
    - "/bwrcq/C/allpan/chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/AsyncQueueSink_3.sv"
    - "/bwrcq/C/allpan/chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/AsyncQueueSink_3_TestHarness_UNIQUIFIED.sv"
    - "/bwrcq/C/allpan/chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/AsyncQueueSink.sv"

Only AsyncQueueSink_3 is uniquified.
I checked, AsyncQueueSink is under ChipTop but AsyncQueueSink_3 is not.

from chipyard.

@allpan3 Can you share your RTL & config so that I can run tests on it? I don't see those issues in the existing configs in CY (as CI seems to be passing) so it would be great if I could actually test the script on your design.

from chipyard.

@joey0320 This may be a little complicated since I'm using tstech28 and currently the RTL repo is private (and I'm not the owner).

I will ask my advisor if he can make the repo public or invite you, but even if not you can just copy from my area.
And also do you have access to tstech28? I think you have to have permission to the PDK on BWRC machine in order to run through the hammer flow successfully.

from chipyard.

I think I don't need the PDK. I just want to generate the verilog with your design so that I can see what is happening.

from chipyard.

Ok, you can try it. I think if you are just running the steps before sram_generator then you probably won't need PDK.
Btw my project is written in verilog.

You can copy generator/hpu from /bwrcq/C/allpan/chipyard/generators/hpu.
You will also need to copy the modified build.sbt and RoCCAcceleratorConfigs.scala.

The config is HPURocketConfig.

from chipyard.

And another issue is make sim-rtl now has duplicated declarations.
Both inputs.yml and sim-inputs.yml are fed to hammer, and in sim-inputs.yml, sim.inputs.input_files_meta is append, so all the files are specified twice in VCS.

./example-vlsi -e /bwrcq/C/allpan/new-chipyard/vlsi/env-bwrc.yml -p /bwrcq/C/allpan/new-chipyard/vlsi/example-tools.yml -p /bwrcq/C/allpan/new-chipyard/vlsi/tstech28.yml -p /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.RocketConfig-ChipTop/inputs.yml -p /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.RocketConfig-ChipTop/sram_generator-output.json -p /bwrcq/C/allpan/new-chipyard/vlsi/tstech28_sram_macros/sram_generator-output.yml -p /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.RocketConfig-ChipTop/sim-inputs.yml --sim_rundir /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.RocketConfig-ChipTop/sim-rtl-rundir --obj_dir /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.RocketConfig-ChipTop sim

I ran with RocketConfig this time so it should be a common issue.

from chipyard.

I suspect using DFS is not correct with this check:

If the instance hierarchy is as follows, where ModA is the model, and ModB and ModC need to be uniquified:

ModA
-> ModB
    -> ModC
-> ModB
    -> ModC

DFS with the visited check will return:

ModA
-> ModB_UNIQUIFIED
    -> ModC_UNIQUIFIED
-> ModB
    -> ModC

This should actually be done using a BFS, so that all instances of ModB get changed to ModB_UNIQUIFIED, then, ModC.

Regarding plusarg_reader.v missing, I don't see it in the .model.f file, so it doesn't get checked. This points to a potential bug in split-module-files.py, which I think is here, where the model_mods set should not fully subtract the dut_mods set.

from chipyard.