uniquify module names #1452

joonho3020 · 2023-04-28T03:41:08Z

Related PRs / Issues:

Type of change:

Bug fix
New feature
Other enhancement

Impact:

RTL change
Software change (RISC-V software)
Build system change
Other

Contributor Checklist:

Did you set main as the base branch?
Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
Did you state the type-of-change/impact?
Did you delete any extraneous prints/debugging code?
Did you mark the PR with a changelog: label?
(If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
(If applicable) Did you add documentation for the feature?
(If applicable) Did you add a test demonstrating the PR?

(If applicable) Did you mark the PR as Please Backport?

harrisonliew

Looks like this BFS + DFS approach works.
@allpan3 & @bdngo please give this a try!

common.mk

scripts/uniqify-module-names.py

allpan3 · 2023-04-28T16:42:11Z

I ran sim-rtl with RocketConfig and still saw similar redeclaration messages.
Looks like inputs.yml and sim-inputs.yml contain some duplicated files.
Is sim.mk supposed to filter the common ones out before writing out sim-inputs.yml?

harrisonliew · 2023-04-28T17:58:26Z

@joey0320 when I query the common files between .model.f and .top.f in the archive you sent me, I got:

>> comm -12 <(cat chipyard.TestHarness.RocketConfig.model.f | sort -u) <(cat chipyard.TestHarness.RocketConfig.top.f | sort -u)
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/AsyncQueue.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/AsyncQueueSink_3.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/AsyncQueueSource_3.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/Queue_66.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/Queue_70.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/Repeater_10.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/Repeater_9.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLBuffer_23.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLBuffer_24.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLFragmenter_10.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLFragmenter_11.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLMonitor_60.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLMonitor_61.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLMonitor_62.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLMonitor_63.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLMonitor_64.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLMonitor_65.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLMonitor_66.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLMonitor_67.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLROM_1.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLSerdesser_1.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/TLXbar_11.sv
/scratch/joonho.whangbo/coding/circt-fix-cy/sims/vcs/generated-src/chipyard.TestHarness.RocketConfig/gen-collateral/ram_combMem_0.sv

I don't see these modules in top_module_hierarchy.json, so it's weird that this line is writing these modules from the model into it:

chipyard/scripts/uniquify-module-names.py

Line 183 in ab9ec16

write_filelist(modules_under_top, args.out_dut_filelist)

@allpan3 can you do the same comm command to verify, and see that these are the same modules that you have the redeclaration errors about, and are the only ones common between inputs.yml and sim-inputs.yml?

allpan3 · 2023-04-28T19:33:38Z

@harrisonliew Seems like those are the only ones. I tried removing them from inputs.yml and now sim-rtl can run.

allpan3 · 2023-04-28T21:27:52Z

There's another issue when running sim-syn.

This is my guess:
sim-inputs.yml contains all files in*.model.bb.f. However, when the generator is written in Verilog, the Chisel blackbox wrapper seems to put the .v file in this *.model.bb.f file as well.
This conflicts with the post-syn netlist, which will contain the same module name.

Here is the message I got:

 Error-[MPD] Module previously declared
   The module was previously declared at:
   "/bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/syn-rundir/ChipTop.mapped.v"

   411739
   It is redeclared later at:
   "/bwrcq/C/allpan/new-chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/hpu.preprocessed.v"

   2291: token is 'hdop_1bit'
   module hdop_1bit

hpu.preprocessed.v is the verilog file added by the Chisel wrapper. I think @abejgonzalez is familiar with this since I followed the same approach as in NVDLA and CVA6.

harrisonliew · 2023-04-28T22:24:09Z

@joey0320 I think now the split-bb-files.py file is unnecessary/causing problems now that you're doing this traversing of the hierarchy. Now, blackbox modules would be placed in both the .top/model.f and the .top/model.bb.f files. I think we can essentially remove this script, make sure that the .v file for SimDRAM/JTAG/Serial/etc. is written to .model.f (right now it's the .cc ones given your extension querying), and then copy all remaining .cc files from .all.f into .model.f.
@allpan3 I don't think this explains your error though... it suggests the same module was in the .top.f vs. .model.bb.f files or .top.bb.f vs. .model.bb.f files... can you see if that's the case using comm? I don't see it with RocketConfig.

allpan3 · 2023-04-29T01:58:54Z

Looks like this file is in both .top.f and .model.bb.f, but not in .top.bb.f

comm -12 <(cat chipyard.TestHarness.HPURocketConfig.top.f | sort -u) <(cat chipyard.TestHarness.HPURocketConfig.model.bb.f | sort -u)
/bwrcq/C/allpan/new-chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/hpu.preprocessed.v

@harrisonliew And yes, the error I got is not related to duplicated files/modules. It's conflicting with the post-synthesis netlist. I'm guessing this has always been an issue for blackboxed designs, but it was shadowed because -top option turns it into warning?

Does it have to be in .model.bb.f? If we take it out from .model.bb.f then this should be gone, since .top.f isn't used by sim-syn.
I don't know how simple that is though. I guess it's a default action for all classes extending HasBlackBoxPath?

allpan3 · 2023-04-29T16:50:49Z

I think this explains why I previously got the error in force_regs.ucli. The RTL module definition overrides the netlist definition, so everything inside hpu_hpu instance is removed.

ucli% source /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/sim-syn-rundir/force_regs.ucli
Error: script /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/sim-syn-rundir/force_regs.ucli stopped at line 46369
file /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/sim-syn-rundir/run.tcl, line 12: Error-[UCLI-FORCE-OBJ-NOT-FOUND] Force command error
  Force command on object
  'TestDriver.testHarness.chiptop.system.tile_prci_domain.tile_reset_domain_tile.hpu_hpu.hdop_dp_inst.hdop_inst.\BPE_gen_loop[0].hdop_1bit_inst\.reg_val_reg

For now, I just remove the /bwrcq/C/allpan/new-chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/hpu.preprocessed.v file from .model.bb.f manually.

I think this is a separate issue from this thread, and should be common for all blackboxed generators (e.g. NVDLA, CVA6).

joonho3020 · 2023-04-30T01:22:30Z

@joey0320 I think now the split-bb-files.py file is unnecessary/causing problems now that you're doing this traversing of the hierarchy. Now, blackbox modules would be placed in both the .top/model.f and the .top/model.bb.f files. I think we can essentially remove this script, make sure that the .v file for SimDRAM/JTAG/Serial/etc. is written to .model.f (right now it's the .cc ones given your extension querying), and then copy all remaining .cc files from .all.f into .model.f. @allpan3 I don't think this explains your error though... it suggests the same module was in the .top.f vs. .model.bb.f files or .top.bb.f vs. .model.bb.f files... can you see if that's the case using comm? I don't see it with RocketConfig.

Just pushed fixes regarding this! Thanks

joonho3020 · 2023-04-30T01:23:28Z

I think this explains why I previously got the error in force_regs.ucli. The RTL module definition overrides the netlist definition, so everything inside hpu_hpu instance is removed.
ucli% source /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/sim-syn-rundir/force_regs.ucli
Error: script /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/sim-syn-rundir/force_regs.ucli stopped at line 46369
file /bwrcq/C/allpan/new-chipyard/vlsi/build/chipyard.TestHarness.HPURocketConfig-ChipTop/sim-syn-rundir/run.tcl, line 12: Error-[UCLI-FORCE-OBJ-NOT-FOUND] Force command error
  Force command on object
  'TestDriver.testHarness.chiptop.system.tile_prci_domain.tile_reset_domain_tile.hpu_hpu.hdop_dp_inst.hdop_inst.\BPE_gen_loop[0].hdop_1bit_inst\.reg_val_reg
For now, I just remove the /bwrcq/C/allpan/new-chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/hpu.preprocessed.v file from .model.bb.f manually.

I think this is a separate issue from this thread, and should be common for all blackboxed generators (e.g. NVDLA, CVA6).

Can you make a separate issue regarding this & tag me?

vlsi/Makefile

vlsi/sim.mk

allpan3 · 2023-04-30T19:45:15Z

Looks like this file is in both .top.f and .model.bb.f, but not in .top.bb.f

This comment is no longer true after 30ec980. I didn't merge that commit when I was testing. After that, only .bb.f and .model.bb.f contain hpu.preprocessed.v, and 3f80507 removes .model.bb.f so only.bb.f has it.

jerryz123 · 2023-05-02T01:51:45Z

Failing tests have been fixed on main, this PR is good-to-merge with those tests failing

joonho3020 · 2023-05-02T02:13:13Z

@harrisonliew @allpan3 can you run the vlsi flow with the newest version? I fixed the bb stuff. Thanks!

allpan3 · 2023-05-02T15:12:42Z

@harrisonliew @allpan3 can you run the vlsi flow with the newest version? I fixed the bb stuff. Thanks!

Ok I'm running. Will update later

allpan3 · 2023-05-02T15:42:31Z

@harrisonliew @allpan3 can you run the vlsi flow with the newest version? I fixed the bb stuff. Thanks!

I'm getting this error in synthesis:

Error   : Could not resolve reference. [CDFG-431] [elaborate]
        : The unresolved reference is for instance 'iocell_serial_tl_bits_out_bits' in file '/bwrcq/C/allpan/new-chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/ChipTop.sv' on line 360.

Looks like IOCell.v is not found, which is in .bb.f

[allpan@bwrcrdsl-2]/bwrcq/C/allpan/new-chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig[hotfix2] $ grep IOCell.v *.f
chipyard.TestHarness.HPURocketConfig.all.f:/bwrcq/C/allpan/new-chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/IOCell.v
chipyard.TestHarness.HPURocketConfig.bb.f:/bwrcq/C/allpan/new-chipyard/vlsi/generated-src/chipyard.TestHarness.HPURocketConfig/gen-collateral/IOCell.v

~~I think none of the files in .bb.f is found in syn.f.~~ Some are there, but a few are missing, including the blackboxed generator RTL hpu.preprocessed.v.

joonho3020 · 2023-05-02T17:50:52Z

I think this is happening because of the module names not matching their file names + certain files containing multiple modules. I'll get back to this.

joonho3020 · 2023-05-05T03:18:13Z

@harrisonliew @allpan3 I'm sorry for the low responsiveness, was busy with final projects... I did some code cleanup & took care of the blackbox/c++ file corner cases. Can you try again?

allpan3 · 2023-05-06T02:17:19Z

RTL simulation, synthesis, post-syn simulation all work now. Thanks!

joonho3020 · 2023-05-06T05:46:30Z

@abejgonzalez can you approve this?

abejgonzalez

LGTM

harrisonliew

Thanks @allpan3 for all the testing!

trial 1

c1667ad

joonho3020 requested review from harrisonliew and abejgonzalez April 28, 2023 03:41

cleanup

85abce5

harrisonliew requested changes Apr 28, 2023

View reviewed changes

common.mk Outdated Show resolved Hide resolved

scripts/uniqify-module-names.py Outdated Show resolved Hide resolved

scripts/uniqify-module-names.py Outdated Show resolved Hide resolved

fixes

ab9ec16

fix for ext modules

27728a7

final fix

30ec980

rm split-bb-files.py

3f80507

harrisonliew requested changes Apr 30, 2023

View reviewed changes

vlsi/Makefile Show resolved Hide resolved

oops

a5bf60c

allpan3 reviewed Apr 30, 2023

View reviewed changes

vlsi/sim.mk Show resolved Hide resolved

deal with bb

884cf3c

Cleanup + fixes, think it's stable now

60e80a7

corner case

fec43fc

abejgonzalez approved these changes May 6, 2023

View reviewed changes

joonho3020 requested a review from harrisonliew May 6, 2023 19:10

harrisonliew approved these changes May 6, 2023

View reviewed changes

joonho3020 merged commit 01a03f5 into main May 6, 2023

joonho3020 deleted the uniquify-names-2 branch May 6, 2023 19:29

harrisonliew mentioned this pull request May 7, 2023

VCS compiliation errors with VLSI flow #1462

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

uniquify module names #1452

uniquify module names #1452

joonho3020 commented Apr 28, 2023

harrisonliew left a comment

allpan3 commented Apr 28, 2023 •

edited

Loading

harrisonliew commented Apr 28, 2023 •

edited

Loading

allpan3 commented Apr 28, 2023

allpan3 commented Apr 28, 2023 •

edited

Loading

harrisonliew commented Apr 28, 2023 •

edited

Loading

allpan3 commented Apr 29, 2023 •

edited

Loading

allpan3 commented Apr 29, 2023

joonho3020 commented Apr 30, 2023

joonho3020 commented Apr 30, 2023

allpan3 commented Apr 30, 2023

jerryz123 commented May 2, 2023

joonho3020 commented May 2, 2023

allpan3 commented May 2, 2023

allpan3 commented May 2, 2023 •

edited

Loading

joonho3020 commented May 2, 2023

joonho3020 commented May 5, 2023

allpan3 commented May 6, 2023

joonho3020 commented May 6, 2023

abejgonzalez left a comment

harrisonliew left a comment

uniquify module names #1452

uniquify module names #1452

Conversation

joonho3020 commented Apr 28, 2023

harrisonliew left a comment

Choose a reason for hiding this comment

allpan3 commented Apr 28, 2023 • edited Loading

harrisonliew commented Apr 28, 2023 • edited Loading

allpan3 commented Apr 28, 2023

allpan3 commented Apr 28, 2023 • edited Loading

harrisonliew commented Apr 28, 2023 • edited Loading

allpan3 commented Apr 29, 2023 • edited Loading

allpan3 commented Apr 29, 2023

joonho3020 commented Apr 30, 2023

joonho3020 commented Apr 30, 2023

allpan3 commented Apr 30, 2023

jerryz123 commented May 2, 2023

joonho3020 commented May 2, 2023

allpan3 commented May 2, 2023

allpan3 commented May 2, 2023 • edited Loading

joonho3020 commented May 2, 2023

joonho3020 commented May 5, 2023

allpan3 commented May 6, 2023

joonho3020 commented May 6, 2023

abejgonzalez left a comment

Choose a reason for hiding this comment

harrisonliew left a comment

Choose a reason for hiding this comment

allpan3 commented Apr 28, 2023 •

edited

Loading

harrisonliew commented Apr 28, 2023 •

edited

Loading

allpan3 commented Apr 28, 2023 •

edited

Loading

harrisonliew commented Apr 28, 2023 •

edited

Loading

allpan3 commented Apr 29, 2023 •

edited

Loading

allpan3 commented May 2, 2023 •

edited

Loading