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

Dotted references to type parameters do not have the correct size #1458

Closed
veripoolbot opened this issue Jun 7, 2019 · 7 comments
Closed
Assignees
Labels
area: wrong runtime result Issue involves an incorrect runtine result from Verilated model resolution: fixed Closed; fixed

Comments

@veripoolbot
Copy link
Contributor


Author Name: Todd Strader (@toddstrader)
Original Redmine Issue: 1458 from https://www.veripool.org

Original Assignee: Todd Strader (@toddstrader)


See the commented out portions of t_type_param.v. Perhaps this shouldn't even be allowed since it can cause elaboration-time loops. However, we currently do not get any errors or warnings at compile-time.

See Issue #1456 for background.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-06-08T03:18:57Z


For the second commented section, it refers to baz is a parameter set to a $bits. $bits is by IEEE a 32-bit return value, therefore $bits(baz) should be 32, which is what Verilator says (that is you're saying m != $bits($bits(bar)). I suspect you didn't want the $bits(foo_inst.baz) and instead wanted

               if (m != foo_inst.baz) begin

Similar for first section. When I remove the extra $bits, and fix the instance pointed to, both pass. Or I'm misunderstanding the problem.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2019-06-08T12:25:49Z


Ugh, yup. I think I started with $bits(foo_inst.bar) before I added baz. In any case, this fixes the commented portions of the test:
https://github.com/toddstrader/verilator-dev/tree/samehash-test

Or if you think that is redundant with the checking via bar_size, we can just rip out the comments. Whatever you think is best here.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-06-08T22:01:31Z


Um, now t_type_param.pl looks good but t_type_param_collision.pl fails.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2019-06-08T23:06:39Z


Sigh, this issue isn't working out well for me. So #1456 isn't totally fixed then. I'm getting AstBasicDTypes in paramValueNumber() for this, but the left() and right() are always evaluating to zero even though I see that they should not be zero. I'll have to investigate some more.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2019-06-11T11:01:13Z


OK, so I actually ran all the tests this time:
https://github.com/toddstrader/verilator-dev/tree/samehash-test

The problem was that by the time we got to paramValueNumber() with the type parameter, the type's range had not yet been constified. So when I tried to get left() and right(), I always ended up with "[0:0]". This meant all the keys were the same for any width range of the same type which caused the collision error.

I wasn't sure if there would be any issue with additional levels of param indirection so I added a wrapper module to the test to force some additional elaboration steps.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-06-11T23:00:33Z


Golden, thanks. Looks like one of those sort of patches which requires hours of thought to result in just one patched line ;)

Fixed in git towards 4.015.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-06-16T13:59:27Z


In 4.016.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: wrong runtime result Issue involves an incorrect runtine result from Verilated model resolution: fixed Closed; fixed
Projects
None yet
Development

No branches or pull requests

2 participants