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

Restore tracing in Snitch DMA #52

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Restore tracing in Snitch DMA #52

merged 1 commit into from
Dec 2, 2024

Conversation

colluca
Copy link
Contributor

@colluca colluca commented Jul 12, 2024

Contributions

  • Restore DMA tracing in Snitch, to the extent this feature had before the old DMA was replaced in hw: Update iDMA and remove future source directory snitch_cluster#108. Note this does not restore all previously logged signals as significant changes were made to the backend.
  • Correct a potentially dangerous typo in the tracer RTL (SYNTHESIS being misspelled).
  • Enable tracing multiple DMA channels, when present.
  • Add backend signals required to parse the DMA traces in Snitch, as used in trace: Add functionality to parse DMA trace snitch_cluster#71. These are equivalent substitutes to some of the previously logged signals in the Snitch DMA.
  • Tentative support for tracing in Verilator (see below).

Tracing in Verilator

Before the iDMA integration in Snitch I was able to enable DMA tracing also in Verilator (version 4.110). With the iDMA this fails with the following error:

%Error-UNSUPPORTED: /scratch/colluca/workspace/snitch_cluster/traces-dma/working_dir/idma/src/frontend/inst64/idma_inst64_top.sv:530:60: Unsupported: Non-single-bit pos/negedge clock statement under some complicated block
  530 |             @(posedge gen_backend[c].i_idma_backend_rw_axi.clk_i); 
      |                                                            ^~~~~

Upgrading to a newer version of Verilator (tested on 5.006) would solve this, but the --timing flag needs to be provided (given the @ timing block, whereas in the legacy DMA the tracer used an always_ff block).

This still fails on the zero-delay statements:

%Error-ZERODLY: /scratch/colluca/workspace/snitch_cluster/traces-dma/working_dir/idma/src/frontend/inst64/idma_inst64_top.sv:530:9: Unsupported: #0 delays do not schedule process resumption in the Inactive region
  530 |         #0; 
      |         ^

but if we guard these with `ifndef VERILATOR, thereby omitting the zero-delays altogether for Verilator, all works as expected.

The question is if we can accept to support only Verilator >5.0, or if we would need to implement version-based guarding. AFAIK, Verilator does not define a VERILATOR_5 macro itself, so we would need to implement something like that ourselves.

@colluca colluca marked this pull request as ready for review July 17, 2024 11:32
@colluca colluca requested a review from thommythomaso as a code owner July 17, 2024 11:32
* util/mario: Correct potentially dangerous typo in tracer RTL

* src: Restore DMA tracing in Snitch DMA

* Correct SV linting errors

* src: Enable tracing multiple channels

* util/mario: Trace signals required for parsing in Snitch

* treewide: Enable tracing in Verilator

* src: Guard tracing logic during synthesis (#58)
@thommythomaso thommythomaso merged commit 110fd06 into master Dec 2, 2024
24 of 26 checks passed
@thommythomaso thommythomaso deleted the snitch-tracing branch December 2, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants