chili-chips-ba / openCologne

Spicing up the first and only EU FPGA chip with a flashy new board, loaded with a suite of engaging demos and examples. https://www.chili-chips.xyz/open-cologne
https://nlnet.nl/project/openCologne
BSD 3-Clause "New" or "Revised" License
37 stars 2 forks source link

Error When Adding synth_gatemate Command in CDC Workflow #29

Closed TarikHamedovic closed 4 hours ago

TarikHamedovic commented 1 month ago

We are currently working on porting the OPL3 Yamaha FM synthesizer to the GateMate FPGA. Within this effort, we're incorporating CDC checks into our workflow. As there aren't many open-source CDC tools available, Larry's cdc_snitch came to rescue (it can be found on the Bedrock github).

The _cdcsnitch is made out of a (1) Python script and a (2) .ys script. The Python script takes the .json output file from Yosys synthesis. There is no problem when tool is used with a general Yosys command like the one given below (taken from the Makefile line 301) :

$(YOSYS) $(YOSYS_QUIET) -p 'read_verilog $(VLOG_SRC); script $(CDC_SNITCH_PROC); write_json $@'

But we get incorrect flop counts because the basic Yosys synthesis isn't the same as the GateMate synthesis being done with synth_gatemate. The resulting resource utilization discrepancy between GateMate and basic Yosys synthesis is quite drastic, see this.

When tool is invoked using this command (line 302 of Makefile):

$(YOSYS) $(YOSYS_QUIET) -p 'read_verilog $(VLOG_SRC); synth_gatemate -top $(TOP) -nomx8; script $(CDC_SNITCH_PROC); write_json $@'

or using the form from line 303 of Makefile):

$(YOSYS) $(YOSYS_QUIET) -p 'read_verilog $(VLOG_SRC); synth_gatemate -top $(TOP) -nomx8 -json opl3_cdc.json; script $(CDC_SNITCH_PROC)'

we get the following error:

ERROR: Module `\CC_LUT4' referenced in module `\opl3' in cell `$abc$33365$auto$blifparse.cc:535:parse_blif$33366' is a blackbox/whitebox module.

We left the opl3_cdc.json file in the 3.build folder.

Replicate this error

To replicate this error, you need to clone the repository. Navigate to openCologne/4.Advanced--4--Yamaha-OPL3-FM-Synth/3.build/, delete the opl3_cdc.json file and run the make cdc command. Before that you need to install a few open-source tools (see Makefile):

# Clock Domain Checking is done by using the tool from Bedrock: https://github.com/BerkeleyLab/Bedrock/blob/master/build-tools/cdc_snitch.md
# In the documentation it says that is is compatible with yosys 0.23, I tried it out with yosys 0.4 and it works fine
# For more information see: https://github.com/BerkeleyLab/Bedrock/blob/master/build-tools/cdc_snitch.md
#                           https://github.com/YosysHQ/yosys/discussions/3956
#                           https://github.com/YosysHQ/yosys/issues/4493#issuecomment-2246025920
# How to use:
# Go to your TOOL_ROOT(if you haven't exported it yet, export it)
$ git clone https://github.com/BerkeleyLab/Bedrock.git
# That's it!
ldoolitt commented 1 month ago

Step 1: Ignore cdc_snitch_proc.ys. Its job is replaced by synth_gatemate. Step 2: Adapt cdc_snitch.py for the slightly different conventions and primitives in the (json-encoded) netlist produced by synth_gatemate. Quick and temporary start here: cdc_snitch_oc.txt, that "fixes" two small problems: finding the top module, and the clock port of a CC_DFF is called CLK instead of C. Result:

OK1: 8321  CDC: 0  OKX: 56  BAD: 149

There's more to do. synth_gatemate seems to have invented a couple of additional clock signals, like channels.ch_abcd_cnt_mem.bankb_sr.clk. cdc_snitch now finds BAD patterns where these new clocks are mixed with the actual clk input port, like

BAD  661 channels.control_operators.operator.phase_generator.log_sin_plus_gain_p6[8]:D clk channels.ch_abcd_cnt_mem.bankb_sr.clk inputs ( 1 x clk, 25 x channels.ch_abcd_cnt_mem.bankb_sr.clk )
  tree 661 from 2 modinput clk
  tree 661 from 656 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.envelope_generator.ksl_add_rom.tmp0_p1[3]
  tree 661 from 1567 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.env_p5[5]
  tree 661 from 1569 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.ws_post_opl_p[12]
  tree 661 from 1570 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.ws_post_opl_p[13]
  tree 661 from 1571 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.ws_post_opl_p[14]
  tree 661 from 1590 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[10]
  tree 661 from 1593 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[11]
  tree 661 from 1595 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[12]
  tree 661 from 1597 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[13]
  tree 661 from 1599 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[14]
  tree 661 from 1601 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[15]
  tree 661 from 1603 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[16]
  tree 661 from 1605 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[17]
  tree 661 from 1625 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.final_phase_p4[18]
  tree 661 from 2190 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.env_p5[0]
  tree 661 from 2195 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.env_p5[1]
  tree 661 from 2200 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.env_p5[2]
  tree 661 from 2205 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.env_p5[3]
  tree 661 from 2210 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.env_p5[4]
  tree 661 from 3757 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.tmp_ws7_p5[3]
  tree 661 from 3759 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.tmp_ws7_p5[4]
  tree 661 from 3761 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.tmp_ws7_p5[5]
  tree 661 from 3763 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.tmp_ws7_p5[6]
  tree 661 from 3765 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.tmp_ws7_p5[7]
  tree 661 from 3767 clk channels.ch_abcd_cnt_mem.bankb_sr.clk name channels.control_operators.operator.phase_generator.tmp_ws7_p5[8]

This problem should be addressed by someone who knows yosys and/or GateMate better than me.

chili-chips-ba commented 1 month ago

@pu-cc any insights on Larry's insights above, from your GateMate and Yosys-for-GateMate expert viewpoint?!

pu-cc commented 3 weeks ago

To be honest, I would first have to familiarize myself with this CDC workflow, which I have not yet managed to do.

But I can say something about the “invented” clock signals. Since the signal names suggests that it is a memory, I assume that it is either emulated asynchronous RAM, which we have already discussed in this thread https://github.com/chili-chips-ba/openCologne/issues/26#issuecomment-2262227377, or it is synchronous RAM that exceeds the size of the existing 20K/40K cells. Yosys then merges several Block RAMs in order to achieve the desired memory width or memory depth. This also “invents” several clock signals that have to be routed to all RAMs.

That's all I can say so far, but maybe it will help you understand and troubleshoot.

chili-chips-ba commented 3 weeks ago

@ldoolitt if this is merely about increased fanout, why would it be reported as BAD: 149, esp. given that we had no BAD findings before?!

chili-chips-ba commented 2 days ago

@ldoolitt please advise if you are OK with having this issue closed.