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

$value_plusargs compile error for [1..16]-bit signals #1592

Closed
veripoolbot opened this issue Nov 4, 2019 · 6 comments
Closed

$value_plusargs compile error for [1..16]-bit signals #1592

veripoolbot opened this issue Nov 4, 2019 · 6 comments

Comments

@veripoolbot
Copy link
Contributor

@veripoolbot veripoolbot commented Nov 4, 2019


Author Name: Garrett Smith
Original Redmine Issue: 1592 from https://www.veripool.org

Original Assignee: Garrett Smith


Passing signals of length <= 16 bits to $value$plusargs results in compiler errors due to missing overloads for CData and SData. The variable of type vluint8_t or vluint16_t will not bind to a vluint32_t reference:

VsimTop.cpp: In static member function _static void VsimTop::_initial__TOP__5(VsimTop__Syms*)_:
VsimTop.cpp:3537:16: error: invalid initialization of non-const reference of type _IData& {aka unsigned int&}_ from an rvalue of type _IData {aka unsigned int}_
         vlTOPp->simTop__DOT__test);}
                 ^
In file included from VsimTop.h:11:0,
                  from VsimTop.cpp:5,
                  from VsimTop__ALLcls.cpp:3:
/path/to/verilator-4.012/share/verilator/include/verilated_heavy.h:77:14: note:   initializing argument 3 of _IData VL_VALUEPLUSARGS_INI(int, const string&, IData&)_
 inline IData VL_VALUEPLUSARGS_INI(int rbits, const std::string& ld, IData& rdr) VL_MT_SAFE {

This was resolved by overloading VL_VALUEPLUSARGS_INI for CData and SData:
gcsmith@b1ad0b2

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Nov 5, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-11-05T00:00:10Z


Great, love getting bugs with fixes included!

Two minor things to add to your patch so I can merge it and make sure it stays fixed. First, please add your name to docs/CONTRIBUTORS, see docs/CONTRIBUTING.adoc for more info. 2. Please update one of the tests, e.g. t_sys_plusargs.v to show the problem.

Thanks again.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Dec 3, 2019


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


Would you be willing to make the changes suggested earlier (test & add your name to contributors) so this can get in the next release? Thanks.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Dec 3, 2019


Original Redmine Comment
Author Name: Garrett Smith
Original Date: 2019-12-03T19:45:24Z


Sorry for the delay. I was waiting to hear back from my company's legal department regarding open source contributions.

I'll submit the requested changes shortly.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Dec 3, 2019


Original Redmine Comment
Author Name: Garrett Smith
Original Date: 2019-12-03T20:12:29Z


Pull request: #6

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Dec 3, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-03T23:27:56Z


Fixed in git towards eventual 4.024 release.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Dec 8, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-08T13:11:35Z


In 4.024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.