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

Symbol table scope map #966

Closed
veripoolbot opened this issue Sep 22, 2015 · 10 comments
Closed

Symbol table scope map #966

veripoolbot opened this issue Sep 22, 2015 · 10 comments
Assignees
Labels
area: tests Issue involves the testing system resolution: fixed Closed; fixed

Comments

@veripoolbot
Copy link
Contributor


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-09-22T12:55:01Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Jie Xu (@jiexu)
Original Date: 2015-09-22T13:24:35Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2015-09-22T23:46:16Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-09-23T01:27:18Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2015-09-23T02:30:56Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-09-23T12:53:44Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2015-09-23T13:06:10Z


OK, great. I'll take a look and see what I can do.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2015-09-24T01:07:34Z


OK, I updated the GitHub repo again. Now it exposes the preexisting map. Some comments about the changes here:

toddstrader/verilator-scope-map@b8dcc83

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-09-25T01:11:42Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-11-01T13:24:20Z


In 3.878.

@veripoolbot veripoolbot added area: tests Issue involves the testing system resolution: fixed Closed; fixed labels Dec 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: tests Issue involves the testing system resolution: fixed Closed; fixed
Projects
None yet
Development

No branches or pull requests

2 participants