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

[lint] Width Warning due to Comparison of String Parameter in Generate Statement #1621

Closed
veripoolbot opened this issue Dec 5, 2019 · 2 comments
Labels
area: lint Issue involves SystemVerilog lint checking resolution: no fix needed Closed; no fix required (not a bug)

Comments

@veripoolbot
Copy link
Contributor


Author Name: Michael Schaffner
Original Redmine Issue: 1621 from https://www.veripool.org


I recently ran verilator --lint-only on a module that has a string parameter which is used in a generate block later on in the body of the module.
Lint throws a %Warning-WIDTH warning due to a width mismatch between LHS and RHS of the EQ operator used in that generate statement, and I would like to ask whether this is the intended behavior of lint in this case.
I feel like this is the normal way a string parameter is typically used in a design, and was a bit surprised to see this trigger lint warnings.

This issue can be reproduced by downloading prim_lfsr.sv and prim_assert.sv from

https://github.com/lowRISC/opentitan/blob/8711e21c902bed11b09f156579145c247cf88b49/hw/ip/prim/rtl/prim_lfsr.sv
https://github.com/lowRISC/opentitan/blob/8711e21c902bed11b09f156579145c247cf88b49/hw/ip/prim/rtl/prim_assert.sv

and running

verilator --lint-only prim_assert.sv prim_lfsr.sv

using Verilator 4.020 2019-10-06 rev v4.018-62-gdfcd412e.

The warning I am referencing above is the third one that appears:

%Warning-WIDTH: prim_lfsr.sv:397: Operator EQ expects 64 bits on the LHS, but LHS's VARREF 'LfsrType' generates 56 bits.
                                 : ... In instance prim_lfsr
     end else if (LfsrType == "FIB_XNOR") begin
                           ^~
@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-12-05T02:02:06Z


Thanks for the report.

Due to the language origins "quotedconstants" are really packed number arrays. So the warning is tecnnically correct. Unfortunately Verilator because of these rules doesn't even know that it was a string, so it doesn't really have a way to suppress it. Furthermore, width mismatches insert spaces, which often cause other problems, so I'd probably argue the message should be there anyhow.

BTW the Veriliog code ideally should be changed to this:

parameter string LfsrType = "GAL_XOR",

then indeed the warning goes away.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Michael Schaffner
Original Date: 2019-12-05T02:09:13Z


Thanks for the quick reply!

I agree that this is not ideal, but unfortunately some synthesis tools have problems with declaring the parameter explicitly as a string type, hence we resorted to this "old school" string parameter style.

Anyhow, we'll work around with length casts or explicit waivers, thanks!

@veripoolbot veripoolbot added area: lint Issue involves SystemVerilog lint checking resolution: nofixneeded resolution: no fix needed Closed; no fix required (not a bug) and removed resolution: nofixneeded labels Dec 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: lint Issue involves SystemVerilog lint checking resolution: no fix needed Closed; no fix required (not a bug)
Projects
None yet
Development

No branches or pull requests

1 participant