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
Symbol table scope map #966
Comments
Original Redmine Comment This is a patch to Jie Xu's vardbg branch, so it needs to be applied there. As to getting vardbg into the trunk when Jie Xu feels it's ready he can create another bug and we'll work towards adding it in. I like the additional debug capabilities, but would prefer it meld better and improve the existing VerilatedScope/VerilatedVar classes as there's otherwise a lot of overlap to maintain. |
Original Redmine Comment Wilson, nice that you like this debugging functionality. I definitely like this to be merged into the mainstream. Will take a look it and submit the vardbg patch in another thread. Todd, I will try to cleanup the vardbg patch first and then look into your patch immediately after that. |
Original Redmine Comment To be clear, I put the changes in master, not on the vardbg branch. Admittedly, I don't fully grok Jie Xu's vardbg changes, so I'm not sure how much overlap there is here. From the way Jie Xu described it, vardbg imposes some performance implications. I would like to avoid that and think that this change shouldn't impact the runtime (other than what declaring signals /verilator public/ already would). Just so I understand, are you saying that this intersects with the vardbg functionality in some way? Do you think that the scope map will need to operate in the context of some vardbg mode? Let me know what you guys think. I'm happy to do any refactoring that needs to be done. |
Original Redmine Comment Maybe I'm confused or your branch is incomplete. The only files I see changed are the addition of test_regress/t/t_scope_map*. This doesn't compile due to there being no scopeMap in verilated.cpp - since nothing was changed there. |
Original Redmine Comment Ugh. It's the latter. Clearly I still don't understand how to use Git. I've updated the github fork and cloned it myself to make sure it's working now. You should find changes to V3EmitCSyms.cpp in addition to the new test. Sorry for the confusion. |
Original Redmine Comment Ah, this makes more sense now. There was already a ScopeNameMap in VerilatedImp, so there's no reason to make another one; instead if your patch could expose it - I guess add a accessor in VerilatedSyms, then in verilated.cpp the accessor can return the verilated_imp.h's m_nameMap. |
Original Redmine Comment OK, great. I'll take a look and see what I can do. |
Original Redmine Comment OK, I updated the GitHub repo again. Now it exposes the preexisting map. Some comments about the changes here: |
Original Redmine Comment Thanks! Forgot to note that verilated.h shouldn't include because that slows down compiles. Just moved your changes there to verilated_syms.h Merged into git towards 3.877. |
Original Redmine Comment In 3.878. |
Author Name: Todd Strader (@toddstrader)
Original Redmine Issue: 966 from https://www.veripool.org
Original Date: 2015-09-22
Original Assignee: Todd Strader (@toddstrader)
As discussed "here":http://www.veripool.org/boards/2/topics/1697-Verilator-Verilator-signal-reflection-/, I was looking for a way to discover and access public signals within my designs. I've posted a change for this and included a regression test showing how it works:
https://github.com/toddstrader/verilator-scope-map
The scope map links the hierarchical names of the scopes to the scopes themselves. Please let me know what you think of the patch. The only major question I had was whether this should require a command line switch or not. It seems to me that the overhead of one map (which should be empty if there are no public signals) is not great and does not warrant a switch. But I have no strong feelings about this and will add the switch if people feel it is better that way.
The text was updated successfully, but these errors were encountered: