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

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

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

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Oct 23, 2017


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Oct 24, 2017


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 #2 perhaps slightly more. Note this cannot be done at compile time.

Preferences/thoughts?

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Oct 26, 2017


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Oct 26, 2017


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 6, 2017


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 6, 2017


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 25, 2017


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


In 3.916.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 20, 2018


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 20, 2018


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.

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
2 participants
You can’t perform that action at this time.