-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Bump to Verilator 4.028. #2377
Bump to Verilator 4.028. #2377
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry the binary dist approach didn't work out.
Both aspects of the CI flow (the Travis magic and the rocket-chip Makefiles) are impenetrable to me. It would be nice to clean all that up, but it's quite the project...
Thanks, and no problem. I think we can address the Travis magic part at least by trying out a different CI provider. We've had success in other SiFive repositories using GitHub's own GitHub Action, and I at least find their configuration format to be less "magic" since there are fewer special-cases, even if it is a bit more verbose to type out. |
This is failing due to several instances of the following error:
This was actually reported in verilator/verilator#2082 and fixed in verilator/verilator#2128, which made it into 4.028. That PR fix bumped the default max width from 5120 to 65536. I suppose there are two ways of fixing this: either 1) use the new @mikeyangsiv, did you run into anything like this when you were trying out the fix in verilator/verilator#2128 ? |
I do not view these wide signals as erroneous, and still question the wisdom of making them errors by default in Verilator. I have no problem with setting |
No, I did not run into any further issues after they bumped up the default width in 4.028. |
I don't know of you looked up the spec flagged in the error, it says ... IEEE 1800-2017 6.9.1
Note that in this specific case, integer literals can be written as unsized. ( |
I knew the spec explicitly permitted implementations to limit some of these parameters, but wasn't aware it specified lower bounds. Incidentally, C does the same thing, but normal users of C blow past the lower bounds in the spec all the time, because normal implementations of C don't impose such draconian limits. Regardless, using the command-line option to increase the limit is the pragmatic choice for the time being. |
Thanks, I've pushed up another commit (aca2ac7) that sets |
Related issue: #2370
Type of change: other enhancement
Impact: no functional change
Development Phase: implementation
Release Notes
I have given up on trying to switch to SiFive's (my) prebuilt Verilator binaries in #2370, but this at least bumps the compiled-from-source Verilator from 4.008 to 4.028, which represents about a year's worth of updates.