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

$value$plusargs requires a string instead of an expression #1165

Closed
veripoolbot opened this issue May 18, 2017 · 8 comments
Closed

$value$plusargs requires a string instead of an expression #1165

veripoolbot opened this issue May 18, 2017 · 8 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented May 18, 2017


Author Name: Wesley Terpstra (@terpstra)
Original Redmine Issue: 1165 from https://www.veripool.org


In src/verilog.y there is this:
| yD_VALUEPLUSARGS '(' str ',' expr ')' { $$ = new AstValuePlusArgs($1,*$3,$5); }

This makes it impossible to obtain anything other than hard-coded command-line arguments. vcs/incisive/questa all support expressions like {NAME, "=%d"} or similar for the first argument.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 18, 2017


Original Redmine Comment
Author Name: Wesley Terpstra (@terpstra)
Original Date: 2017-05-18T19:14:25Z


I've tried to implement this myself, but I got lost in the various output forms that use an AstNode. It does not seem too hard to do for someone more familiar with the code base. The format parsing stuff in EmitC.visit just needs to be moved to VL_VALUEPLUSARGS_IW, where it almost already exists anyway.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 18, 2017


Original Redmine Comment
Author Name: Wesley Terpstra (@terpstra)
Original Date: 2017-05-18T20:05:38Z


I've set up an issue in the rocketchip repository that links to this issue. chipsalliance/rocket-chip#752

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 19, 2017


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-05-19T02:43:57Z


Basically, yes, but lots of details involved, as this dates back to before Verilator had true string support.

Anyhow, fixed in git towards 3.903.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 19, 2017


Original Redmine Comment
Author Name: Wesley Terpstra (@terpstra)
Original Date: 2017-05-19T03:28:18Z


Firstly, thanks a lot for taking a look into fixing this! I noticed that $value$plusargs is returning a 32-bit value and not a boolean.

I get this warning:
%Warning-WIDTH: /scratch/terpstra/federation/rocket-chip/vsrc/plusarg_reader.v:15: Logical Operator LOGNOT expects 1 bit on the LHS, but LHS's VALUEPLUSARGS generates 32 bits.

When compiling this line:
if (!$value$plusargs(FORMAT, myplus)) myplus = DEFAULT;

I can work around this easily, but it is behaviour that differs from the other tools I use.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 19, 2017


Original Redmine Comment
Author Name: Wesley Terpstra (@terpstra)
Original Date: 2017-05-19T04:07:19Z


I've tried verilator commit 7fb2962... it does not seem to update myplus. I'm investigating, but something still seems a bit wrong.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 19, 2017


Original Redmine Comment
Author Name: Wesley Terpstra (@terpstra)
Original Date: 2017-05-19T05:13:52Z


Ok. I've figured it out. The return value of $value$plusargs MUST be inspected, otherwise the call is optimized away to nothing. Presumably it should be marked as having side-effects, since it modifies it's second argument.

So there are two problems remaining:
1- Return result is 32-bit wide instead of 1-bit wide
2- The call is optimized away if the result is unused

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 19, 2017


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-05-19T11:20:52Z


The spec says it's an integer return, so the warning is technically correct, but silly. I committed to suppress it when comparing it as one bit.

The internal attributes to not optimize look correct, please modify the test_regress/t/t_sys_plusargs.v example as needed to show the other problem.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 31, 2017


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-05-31T02:06:46Z


In 3.904.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.