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

Improperly width'ed default function argument shouldn't error out #984

Closed
veripoolbot opened this issue Oct 28, 2015 · 6 comments
Closed
Labels
resolution: fixed Closed; fixed

Comments

@veripoolbot
Copy link
Contributor


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


https://github.com/toddstrader/verilator-dev/tree/default_arg

I would expect a WIDTH warning instead. Also, the current error message is a bit confusing:

%Error: t/t_default_arg_warn.v:20: Unsupported: Non-constant default value in missing argument 'x' in function call to FUNC 'foo'

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-10-30T01:16:34Z


Probably missing a call to constify somewhere. Would you like to try a patch?

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Todd Strader (@toddstrader)
Original Date: 2015-10-30T01:52:20Z


Yeah, sure thing. It may take me a little longer to get to this one, but it's on the list.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Johan Bjork
Original Date: 2017-03-17T20:19:52Z


Took a quick look at this. It seems the problem stems from that V3Width converts the constant to SEL\CONST\CONST\CONST, then V3Width calls V3Task::taskConnects which barfs on the SEL (Expecting const). The quick fix of attempting to call V3Const results in an infinte loop as V3Const calls into V3Width. There might still be a quick fix but not as obvious as expected from the ticket.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Johan Bjork
Original Date: 2017-03-20T14:03:48Z


Trivial fix it seems:

V3Width.cpp:2786
-       if (constp && !nodep->isSigned()) {
+       if (constp && !constp->num().isNegative()) {
V3Width:2793
-           num.isSigned(expDTypep->isSigned());
+           num.isSigned(false);


passes all tests including the one attached to this ticket.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-03-21T23:27:51Z


Thanks for the patch!

Fixed in git towards 3.902.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-04-02T12:51:30Z


In 3.902.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution: fixed Closed; fixed
Projects
None yet
Development

No branches or pull requests

1 participant