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

Bad parameter width error message references parameter definition instead of reference #1624

Open
veripoolbot opened this issue Dec 6, 2019 · 6 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Dec 6, 2019


Author Name: Driss Hafdi
Original Redmine Issue: 1624 from https://www.veripool.org


If a variable of the wrong witdth is passed in as a parameter, verilator prints out in its warning the parameter definition location, whereas it would be more helpful to get the reference of the value that was passed in, particularly with bigger design with multiple levels of hierarchy. Here is a small testbench that show the issue and what I think would be a better error message: drissos@feca874

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 6, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-06T22:35:42Z


Thanks for the test (again), will take a look.

BTW as for your tests, thanks much for using the standard framework. I didn't see a signoff on your commits; do you agree that that all these are open sourced as described in https://github.com/verilator/verilator/blob/master/docs/CONTRIBUTING.adoc

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 6, 2019


Original Redmine Comment
Author Name: Driss Hafdi
Original Date: 2019-12-06T23:11:53Z


Will do so now. Thanks again for checking it all out so quickly.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 6, 2019


Original Redmine Comment
Author Name: Driss Hafdi
Original Date: 2019-12-06T23:16:02Z


Signed off through this diff: drissos@5d0a200

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 7, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-07T18:49:29Z


Pushed the test_regress/t/t_param_width_loc_bad.v test with an unsupported() tag (so doesn't run).

@forensicgarlic

This comment has been minimized.

Copy link

@forensicgarlic forensicgarlic commented Jan 14, 2020

Trying to upgrade from v4.020 to v4.026 in our system, this error message is now firing on functions where everything seemed to work fine before. We were using localparams as internal constants, and doing some math on the results to select constant bits. This commit 1a8b192 appears to be where the check got put in. So was this code we had before fine, or was this ability an oversight?

@wsnyder

This comment has been minimized.

Copy link
Member

@wsnyder wsnyder commented Jan 15, 2020

Assuming the localparam you mention was inside a function, this was never intended to be supported, you just got lucky. Not all commercial simulators support this ether, so much better to move the localparam to just outside the function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.