Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Any plans to add CDC linting? #4493

Open
chili-chips-ba opened this issue Jul 16, 2024 · 9 comments
Open

Any plans to add CDC linting? #4493

chili-chips-ba opened this issue Jul 16, 2024 · 9 comments

Comments

@chili-chips-ba
Copy link

Feature Description

With a good number of opensource RTL developers being converted software engineers, a CDC validation option would greatly help improve the quality of IP in opensource domain.

Is there any thinking in that direction within Yosys dev community?

@widlarizer
Copy link
Collaborator

Clock domain crossing correctness can be implemented prior to passing RTLIL or Verilog to Yosys by language specific tooling, for example, see Amaranth HDL. To my knowledge it's not within scope of any current efforts to integrate related features to Yosys. If anybody is interested in building and maintaining something like that, feel free to ask to reopen this issue

@widlarizer widlarizer closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2024
@nakengelhardt
Copy link
Member

Actually we need to determine clock domains for various functionality inside yosys, which is currently done in an ad-hoc manner, but I'd like to eventually build a helper for (similar to the sigmap and modwalker utilities). Once you have a utility that can tell you for each wire bit the clock domains it's driven from and used in, writing a pass that highlights CDC locations would be the easy part. I've been meaning to look at that for about a year now, there's just never the time... (cf #3956 also)

@wsnyder
Copy link

wsnyder commented Jul 19, 2024

FWIW Verilator had CDC warnings but they were rarely used, and lost the maintainer, so were removed.

@chili-chips-ba
Copy link
Author

chili-chips-ba commented Jul 19, 2024

It's worth a ton!!!

  • Any chance to revive the CDC linting activity within Verilator?

Similarly to what we've recently seen for language linting, where Slang , Verible and Verilator were called into action, the availability of second opinion for CDC Linting would be a welcome addition, esp. in the early days, when both tools are yet to mature.

@ldoolitt
Copy link
Contributor

See discussion #3956 CDC detection with yosys. I have a working tool "cdc_snitch" posted and documented.
By "working" I mean "works for me", and in fact deployed in production CI. I've had no contributions or feedback from others, yet. It would be great to get more people involved!

@chili-chips-ba
Copy link
Author

Great lead! We will be trying it on all of our active projects, and are also spreading the word for the others to kick some CDC tires with it.

  • Any additional insights you can share on ...and in fact deployed in production CI... ?!

@ldoolitt
Copy link
Contributor

Any additional insights you can share on ...and in fact deployed in production CI... ?!

In Bedrock and an additional internal project, we check the portable layer of synthesizable Verilog with cdc_snitch, and don't pass CI unless cdc_snitch reports zero BAD. Sorry, the output of that check is not visible publicly. The gitlab CI job definitions are in (https://github.com/BerkeleyLab/Bedrock/blob/master/.gitlab/ci/cdc_check.gitlab-ci.yml).

In Bedrock, those two code bases have 3001 and 5211 (non-blank, non-comment) lines of code, with some overlap.
The additional internal project checks 15711 LoC, that's heavy on the DSP.

If you peek at our gitlab CI configuration, you can tell that we take CI pretty seriously.
This includes hardware-in-the-loop testing.

Here's a cut-and-paste of the relevant output from the last master-branch CI run, job badger_cdc:

yosys --version
Yosys 0.23 (git sha1 7ce5011c24b)
yosys -q -p "read_verilog -DBUGGY_FORLOOP hw_test.v hw_test_skin.v rtefi_blob.v /builds/hdl-libraries/bedrock/badger/rtefi_center.v /builds/hdl-libraries/bedrock/badger/scanner.v /builds/hdl-libraries/bedrock/badger/pbuf_writer.v /builds/hdl-libraries/bedrock/badger/udp_port_cam.v /builds/hdl-libraries/bedrock/badger/crc8e_guts.v /builds/hdl-libraries/bedrock/badger/construct.v construct_tx_table.v /builds/hdl-libraries/bedrock/badger/ones_chksum.v /builds/hdl-libraries/bedrock/badger/xformer.v /builds/hdl-libraries/bedrock/badger/ethernet_crc_add.v /builds/hdl-libraries/bedrock/badger/hack_icmp_cksum.v /builds/hdl-libraries/bedrock/dsp/reg_delay.v /builds/hdl-libraries/bedrock/dsp/reg_tech_cdc.v /builds/hdl-libraries/bedrock/badger/mac_subset.v /builds/hdl-libraries/bedrock/badger/precog.v /builds/hdl-libraries/bedrock/badger/test_tx_mac.v /builds/hdl-libraries/bedrock/badger/hello.v /builds/hdl-libraries/bedrock/badger/speed_test.v /builds/hdl-libraries/bedrock/badger/mem_gateway.v /builds/hdl-libraries/bedrock/badger/spi_flash.v /builds/hdl-libraries/bedrock/badger/spi_flash_engine.v mac_compat_dpram.v /builds/hdl-libraries/bedrock/homeless/activity.v /builds/hdl-libraries/bedrock/badger/base_rx_mac.v /builds/hdl-libraries/bedrock/badger/packet_categorize.v /builds/hdl-libraries/bedrock/dsp/data_xdomain.v /builds/hdl-libraries/bedrock/dsp/flag_xdomain.v lb_demo_slave.v /builds/hdl-libraries/bedrock/dsp/freq_count.v /builds/hdl-libraries/bedrock/dsp/freq_gcount.v fake_config_romx.v /builds/hdl-libraries/bedrock/dsp/dpram.v /builds/hdl-libraries/bedrock/homeless/multi_counter.v spi_gate.v; script /builds/hdl-libraries/bedrock/build-tools/cdc_snitch_proc.ys; write_json hw_test_yosys.json"
python3 /builds/hdl-libraries/bedrock/build-tools/cdc_snitch.py hw_test_yosys.json -o hw_test_cdc.txt
json from Yosys 0.23 (git sha1 7ce5011c24b)
modules: hw_test_skin
OK1: 2773  CDC: 15  OKX: 24  BAD: 0

@chili-chips-ba
Copy link
Author

Bedrock seems like a very interesting library project. Have you seen @obruendl 's OpenLogic library? He is also looking into "pro"-grade CI.

BTW, Bedrock Yosys is curiously ancient. Any reason for such a long leg?

@ldoolitt
Copy link
Contributor

Bedrock seems like a very interesting library project.

Thanks!

Have you seen @obruendl 's OpenLogic library?

Maybe? The licenses prevent the projects from merging. He's welcome to borrow code from us.

Bedrock Yosys is curiously ancient. Any reason for such a long leg?

The hint is in bedrock's Dockerfile:

FROM debian:bookworm-slim as testing_base_bookworm
apt-get install -y [...] yosys

If sufficiently motivated (as we were back in the bullseye days), we could build a fresh (pinned) copy of yosys.
But our code base, including tests, does work with stock yosys-0.23. Stability is important to us. I have been testing newer tools outside of CI, so when trixie comes out, we hope to accomplish a smooth transition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants