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

Fix: handle VCD variable references with and without whitespace #4620

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RCoeurjoly
Copy link
Contributor

What are the reasons/motivation for this change?

Fixes #4617.

Explain how this is achieved.

We detect if a variable reference in the VCD file contains a space between the signal name and the bit range. If no space is detected, the bit or part-select is parsed and removed from the signal name.

If applicable, please suggest to reviewers how they can test the change.

In addition to the added test case, I processed the neorv32 design with its GHDL-generated VCD file, and the sim pass did not report any issues. It might be helpful to test with other real-world examples, but I believe the current testing is sufficient to verify the fix

Copy link
Member

@mmicko mmicko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just needed to be rebased on vcd2fst integration when that one is merged.
addr variable could use a better name as well.

Would wait till after the release before merging this, so we can try out more designs and check for regressions

@RCoeurjoly RCoeurjoly force-pushed the fix-vcd-parsing-ghdl-var-spacing branch from 79a60a6 to f67227a Compare October 1, 2024 09:48
@RCoeurjoly RCoeurjoly marked this pull request as ready for review October 1, 2024 09:48
@RCoeurjoly RCoeurjoly marked this pull request as draft October 1, 2024 09:49
Co-authored-by: Miodrag Milanović <[email protected]>
Co-authored-by: Roland Coeurjoly <[email protected]>
@RCoeurjoly RCoeurjoly force-pushed the fix-vcd-parsing-ghdl-var-spacing branch from f67227a to 76c615b Compare October 1, 2024 09:51
@RCoeurjoly RCoeurjoly marked this pull request as ready for review October 1, 2024 09:51
@RCoeurjoly
Copy link
Contributor Author

Just needed to be rebased on vcd2fst integration when that one is merged. addr variable could use a better name as well.

Would wait till after the release before merging this, so we can try out more designs and check for regressions

Rebased on main. Changed addr for index_or_range.

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.

VCD file parsing error in sim pass with GHDL-generated VCDs due to whitespace handling
2 participants