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

Jump to bottom

Problems attempting to redefine VL_PRINTF #1598

Closed
veripoolbot opened this issue Nov 14, 2019 · 7 comments
Closed

Problems attempting to redefine VL_PRINTF #1598

veripoolbot opened this issue Nov 14, 2019 · 7 comments
Labels
area: documentation Issue involves documentation resolution: fixed Closed; fixed

Comments

@veripoolbot
Copy link
Contributor

Author Name: Julien Margetts
Original Redmine Issue: 1598 from https://www.veripool.org

Original Assignee: Julien Margetts

I need my verilated model to call something other than printf to perform output $display, and the intended way to do that (according to verilated.h) appears to be to re-define VL_PRINTF

However doing so seems to cause a number of unexpected warnings. It is also unclear what the proper approach should be to make the verilator generated make file 'aware' of the declaration of my new output function (I.e. how do I tell verilator to put "#include "my_printf.h" into the verilated output code)

I'm sure I'm making a schoolboy error here but any pointers would be helpful.

verilator -cc --exe -O3 -Wno-lint -Wno-UNOPTFLAT -Wno-COMBDLY -LDFLAGS "-fPIC --shared" --top-module xxxx_wrapper --unroll-count 150 -Mdir obj -CFLAGS "-fPIC -DVL_PRINTF=my_printf" +incdir+. -f ../core.flist --clk clk verilated_xxxx_wrapper.cpp -o ../verilated_xxxx_wrapper.so
make -s -j 4 -C obj OPT=-Os -f Vnvme_core_wrapper.mk 
make[2]: Entering directory `xxxxx/verilator/obj'
<command-line>:0:0: warning: "VL_PRINTF" redefined [enabled by default]
<command-line>:0:0: note: this is the location of the previous definition
<command-line>:0:0: warning: "VL_PRINTF" redefined [enabled by default]
<command-line>:0:0: note: this is the location of the previous definition
<command-line>:0:0: warning: "VL_PRINTF" redefined [enabled by default]
<command-line>:0:0: note: this is the location of the previous definition
<command-line>:0:0: warning: "VL_PRINTF" redefined [enabled by default]
<command-line>:0:0: note: this is the location of the previous definition
<command-line>:0:0: warning: "VL_PRINTF" redefined [enabled by default]
<command-line>:0:0: note: this is the location of the previous definition
In file included from /tools/verilator/verilator-4.018/include/verilated.cpp:29:0:
/tools/verilator/verilator-4.018/include/verilated_imp.h: In static member function ‘static void VerilatedImp::userDump()’:
/tools/verilator/verilator-4.018/include/verilated_imp.h:279:54: error: ‘my_printf’ was not declared in this scope
              if (first) { VL_PRINTF_MT("  userDump:\n"); first=false; }

NOTE: I can work around this by overiding printf itself (and not using -DVL_PRINTF=my_printf) by adding my own version of printf as a wrapper for my new output function to the verilator input files, but I would rather use -DVL_PRINTF=xxxx if that was the intended approach.
@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-11-15T08:39:11Z

Originally VL_PRINTF was intended to be overridden in a .mk file which overrode the default verilator.mk file. For this reason VL_PRINTF is also defined in verilated.mk, hence the GCC warning about multiple definitions of it. That overriding also edited the make rules to insert a special #include.

What you are doing is perfectly reasonable, and probably a better way.

Please try removing VL_PRINTF from verilated.mk.in, and reinstall verilator (or just hack verilated.mk yourself.)

Then you have the missing definition problem... For that I'd suggest we add to the top of verilatedos.h

#ifdef VL_VERILATED_INCLUDE
1. include VL_VERILATED_INCLUDE
#endif

then add -DVL_VERILATED_INCLUDE="foo"

If this all works out send a patch and I'll merge it upstream.

Thanks.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Julien Margetts
Original Date: 2019-11-15T09:36:41Z

Thanks for the pointers, I will persevere with that aproach and try and get a patch out ...

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-03T01:49:00Z

Did the redefinition etc work for you? if so, can you send back a patch of what worked out? Thanks.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Julien Margetts
Original Date: 2019-12-03T12:05:57Z

Just got round to this, yes, the attached patch works when combined with adding the following to the Verilator command line:

my_printf.cpp --CFLAGS '-DVL_PRINTF=my_printf -DVL_VERILATED_INCLUDE="my_printf.h" -I..'

Thanks

PS: I included VL_VERILATED_INCLUDE in verilated.h rather than verilatedos.h since it seems a better fit (given it is intended only for verilated output, not the host side tool itself?)

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-03T12:22:39Z

Yes, that looks like a fine place. Do you agree this (and future so don't need to ask again) contributions are open sourced as per the Developer Certificate of Origin (https://developercertificate.org/)?

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Julien Margetts
Original Date: 2019-12-03T15:52:57Z

Yes, Agreed

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-04T00:12:29Z

Thanks for your work. Pushed to git towards eventual 4.024 release.

@veripoolbot veripoolbot added area: documentation Issue involves documentation 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
Assignees
No one assigned
Labels
area: documentation Issue involves documentation resolution: fixed Closed; fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@veripoolbot