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
New WIDTH warnings on genvars #1487
Comments
Original Redmine Comment Added two new tests in trunk, t_lint_width_genfor_bad which correctly fails Tracked the version that broke it. Was expecting it would be my fix for Looking at V3Simulate what seems to be going on is the old code kept the The following hack makes things better, but doesn't fix integers (maybe fix
I suspect #� might break again with this stuff fixed, IMO this bug is I also noted that the debug messages in V3Simulate no longer print the
Another thing for that new bug might be to pull user2/3, m_const*, and the All this work will likely collide with any edits towards #�. |
Original Redmine Comment Re: t_lint_width_genfor, I'm thinking the WIDTH warning in the procedural block "ri = i;" may not really be new. I went back to the revision mentioned and the genvar warnings went away, but the procedural warning is still there. I'm going back and have so far found it back as far as v4.002:
Also, the procedural assignment would seem to be a different class of problem. I can definitely see how V3Simulate could tell you if your truncating while verilating. Proving that a behavioral assignment cannot truncate seems like a whole different ballgame. Unless there's an objection, I'm going to assume that the procedural WIDTH warning is legitimate. |
Original Redmine Comment Ok, the new test was written without looking at the old warning rules. My thinking was any const assignment to a unranged thing (int etc) would not warn. But the old behavior seemed to work well, so I'd suggest we can just lint off out that (new) broken violation in the test so it'll pass. |
Original Redmine Comment Here is my proposed fix: It appears the specific issue in the work towards #� was that it was discarding widthSized. As expected, some of the error message formatting has been changed around constant printing (actually, it's just been changed back to what it was before #�). I had to remove an assertion I put in allocConst() which compared dtypes of the recycled node and the new const. This assertion doesn't work anymore because it needs the value to be set for widthMin to agree. But that doesn't happen until outside of allocConst(). The code could be refactored to make this happen, but now that I've dug deeper into this logic, I don't really see the value in the assertion anymore. This doesn't appear to have disturbed #� as t_enum_size is still passing. Also, I checked and this resolved the SweRV test failure in verilator_ext_tests. |
Original Redmine Comment Great, tighter patch than I was expecting. Please squash and push. |
Original Redmine Comment Squashed, pushed and CI is green. Verilator_ext_tests is working too. That CI environment is still a WIP, but is basically working in my branch: |
Original Redmine Comment In 4.018. |
Author Name: Wilson Snyder (@wsnyder)
Original Redmine Issue: 1487 from https://www.veripool.org
Original Assignee: Todd Strader (@toddstrader)
The following in git (unreleased) creates a warning where previously it did not, due to fixing #�. While technically this is a WIDTH warning it is probably not helpful to users (unless the i has a MSB set that isn't representable in r).
The text was updated successfully, but these errors were encountered: