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

Verilator concatenation error when passing overflowed value from C++ to verilog input port #1238

Closed
veripoolbot opened this issue Oct 23, 2017 · 8 comments
Closed

Verilator concatenation error when passing overflowed value from C++ to verilog input port #1238

veripoolbot opened this issue Oct 23, 2017 · 8 comments
Assignees
@wsnyder
Labels
area: lint Issue involves SystemVerilog lint checking resolution: fixed Closed; fixed

Comments

@veripoolbot
Copy link
Contributor

Author Name: Junyi Xie
Original Redmine Issue: 1238 from https://www.veripool.org

Original Assignee: Wilson Snyder (@wsnyder)

Hi Verilator team,

I have found something related to memory accessing in Verilator.

When the value from C++ input to verilog port is larger than port width can represent,and we concatenate that port with other wires. The overflowed extra bits from C++ input will appear in the concatenated wires and overwrite what is supposed to be there.

For instance, the module has an input port A[9:0]. If in C++ code, we pass 11'd2047 (11'b111_1111_1111) to A and perform 'assign B = {2'b0, A};' on B[11:0]. Then B's value will not be 1023, but 2047. I have attached a very simple test code to reproduce the error (change the path of verilator in build_and_run.sh and execute the script should be enough).

This error can be caught by valgrind. However, users may not be aware this and get strange 'bugs' in there code. This problem troubled some of my teammates and myself when we are not aware of this.
Thus, is there any possibility that we can have some type of a warning from verilator during compile time, so that users know they are passing too large a value to a port with insufficient bits, and the potential problems of doing so.

Your help and response is much appreciated.

Thanks a lot!

Junyi
A verilator newbie
@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-10-24T22:25:41Z

Yes, that's true. There are several ways to debate handling this.

  1. No change to verilator, just note it in the documentation.

  2. Have verilator trim over-width inputs at runtime. This will have a performance penalty. This could be mandatory (depending what the hit is), opt-in, or opt-out.

  3. Have verilator add optional #ifdef VL_DEBUG's that check if inputs have any MSBs set, and print a warning if so. This will have a performance impact, probably similar to Cmake support for models #2 perhaps slightly more. Note this cannot be done at compile time.

Preferences/thoughts?

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Junyi Xie
Original Date: 2017-10-26T17:02:25Z

3 can be good. We should not let over-width inputs still pass to input port. If this happens, maybe throwing an exception and terminate the program? In a large test that prints tons of output, warnings just flash away and hard to catch.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Junyi Xie
Original Date: 2017-10-26T17:03:19Z

And probably make it opt-in / opt-out so user can do the performance-safety tradeoff

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-11-06T02:50:19Z

Fixed to add assertion when VL_DEBUG is enabled.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Junyi Xie
Original Date: 2017-11-06T14:46:06Z

Thanks Wilson! I will have a try on the new feature.
Junyi

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-11-25T20:47:45Z

In 3.916.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Yu Sheng Lin (@johnjohnlin)
Original Date: 2018-12-20T09:25:27Z

I also bump into this problem, so I reply to this closed issue.
This problem is somewhat common and hard to identify (I just assign a -1 in C++ to set a 5-bit value to all 1.).
However, I find this issue because I have been debugging for several hours, investigating the generated CPP sources, locating the issue and finally trying to search on the forum.
I didn't note that I can use VL_DEBUG.
A simpler solution can be just add some info about this issue to the example in the document because I think almost everyone learns to use Verilator from that example.

Thanks.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-12-20T11:53:11Z

Yu Sheng, I'm not sure exactly where you're suggesting the comments be put, perhaps you could attach a documentation patch? Thanks.

@veripoolbot veripoolbot added area: lint Issue involves SystemVerilog lint checking resolution: fixed Closed; fixed labels Dec 22, 2019
@wsnyder wsnyder mentioned this issue May 3, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wsnyder wsnyder

Labels
area: lint Issue involves SystemVerilog lint checking resolution: fixed Closed; fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@wsnyder @veripoolbot