Project

General

Profile

[logo] 
 
Home
News
Activity
About/Contact
Major Tools
  Dinotrace
  Verilator
  Verilog-mode
  Verilog-Perl
Other Tools
  BugVise
  CovVise
  Force-Gate-Sim
  Gspice
  IPC::Locker
  Rsvn
  SVN::S4
  Voneline
  WFH
General Info
  Papers

Issue #1479

Feature Request: VerilatedVcd callback on rolloverMB

Added by Marc Jessome 3 months ago. Updated 3 months ago.

Status:
Feature
Priority:
Normal
Assignee:
Category:
Unsupported
% Done:

0%


Description

I have the time to implement this feature, and am looking for input.

This is a feature for use in conjunction with VerilatedVcd's rolloverMB functionality. I would like to be notified when VerilatedVcd rolls over into a new file, as well as the paths to the old and new vcd file.

A few approaches that come to mind from a quick look at VerilatedVcd:
  1. Add a m_filename getter to VerilatedVcd, and register a changecb on VerilatedVcdC->spTrace(). In the changecb, I would need to implement my own rollover detection.
  2. Add a rollovercb callback through VerilatedVcd::addCallback() that gets invoked in OpenNext(). This would still require m_filename access.
  3. Add a new callback (*VerilatedVcdRolloverCb)(VerilatedVcd *vcdp, void *userthis, std::string old_path, std::string new_path) and register it separately and invoke it in OpenNext().

Input from maintainers on a best approach would be appreciated!

Thanks

0001-Allow-polymorphic-VerilatedVcd-and-VerilatedVcdC.patch View (5.04 KB) Marc Jessome, 07/22/2019 05:18 PM

0001-Allow-polymorphic-VerilatedVcd-and-VerilatedVcdC.patch View (5.03 KB) Marc Jessome, 07/22/2019 08:07 PM

History

#1 Updated by Wilson Snyder 3 months ago

  • Category set to Unsupported
  • Status changed from New to Feature

I'm surprised there wasn't an accessor on filename() please add that regardless.

Do you really need a callback? The alternative is to define your own class with whatever features are needed and inherit the existing class, and override open(). Then use that new class when creating the waves in your main(). If you need some existing functions changed to virtual (maybe openNext) that would be fine.

#2 Updated by Marc Jessome 3 months ago

Thanks for the feedback Wilson.

I could definitely make this work by inheriting VerilatedVcd with access to filename + virtual openNext(). However, unless I'm reading things wrong, I believe this will have implications on VerilatedVcdC in order to make use of it for standalone simulations.

#3 Updated by Wilson Snyder 3 months ago

I'm not sure what problem you are forseeing. Perhaps give inheriting a try? We welcome patches that result.

#5 Updated by Marc Jessome 3 months ago

I have rebased the initial patch on top of master, sorry for posting a second patch.

This patch adds a filename() getter and also allows for polymorphic use of VerilatedVcd and VerilatedVcdC. Please let me know if there are any further changes or if you would like me to take a different approach.

Thanks again!

#6 Updated by Wilson Snyder 3 months ago

  • Assignee set to Marc Jessome

Good work, in general it seems fine.

Please also have the patch add your name to docs/CONTRIBUTORS to acknowledge your contribution is open sourced as described there.

I'd like to understand why you need to have VerilatedVcdC have a pointer to VerilatedVcd rather than just be one itself. I'm not opposed to this, but it is a bit slower and if you derive from this class it shouldn't matter.

Also it's optional for this, but perhaps you want to add a test_regress test to check the polymorphic use? Otherwise what isn't tested is likely to break by accident in the future.

Thanks

Also available in: Atom