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
Set parameters as plusarg at compile time #1045
Comments
Original Redmine Comment Hi, I managed to change the AstNode properly (?): wallento@acaae5a#diff-2679e807dce49f07773ee6e413f40432R880 It now supports setting a parameter with a plusarg at compile time: +parameter+name=value What I want to add now is support for non-decimals and for hierarchical names. I am not sure about the latter, probably I have to move to another Visitor then. Best, |
Original Redmine Comment Great, when you have all that you want please make a patch with everything in it. You create a float like this:
I'm OK with adding a command line parameter for the top level; that makes sense since you may be building a model to instantiate. However I don't understand why you would hierarchically override parameters; does any simulator allow that? Seems very dangerous and confusing. |
Original Redmine Comment Yes, I agree now after putting some more thoughts into it that hierachical names are a bad idea. I will keep it simple for the moment and also restrict the parameter value to be of basic data types. This feature is probably most powerful for simple automated parameter swapping. Cheers, |
Original Redmine Comment Hi Wilson, attached please find my proposal for the patch. There are some things I am not entirely sure about:
Looking forward to your comments! Cheers, |
Original Redmine Comment
I looked at three simulators. NCVerilog doesn't support this. VCS supports:
Modelsim supports:
So I would remove the +I... support, and add an alias to -pvalue+.
Please move this block a parseParamLiteral or something function that
Nit, use !strncmp (sw, "+parameter+", strlen("+parameter+")). I know I Please add a t/t_flag_parameter test for each flavor of switch (e.g. can be |
Original Redmine Comment Hi Wilson, thanks for your input. I have updated the patch accordingly and the regression test tests all cases. As probably expected by you writing the regression test revealed a few problems, so that I extended it a bit. As it does not parse the literal to determine its type, it is pretty much trial-and-error now, with falling back to parsing it as Verilog number. The last errors out then. Attached please find it split into the patch to the lexer's parseDouble and the parameters patch. Cheers, |
Original Redmine Comment Oh, and one remaining problem is that it cannot use double quotes strings in parameter files (.vc). I think there are three options:
How would you like it solved? |
Original Redmine Comment Patch looks good. Yes, please fix .vc files either way that works, as they are quite commonly used and expected to work. |
Original Redmine Comment Pushed your patch 0001 to trunk, thanks. |
Original Redmine Comment In V3Options.cpp:1082 it says:
I think we can stick with this and the patch is fine then. Best, |
Original Redmine Comment Good stuff. Pushed to git towards 3.883. |
Original Redmine Comment In 3.884. |
Author Name: Stefan Wallentowitz (@wallento)
Original Redmine Issue: 1045 from https://www.veripool.org
Original Date: 2016-03-13
Original Assignee: Stefan Wallentowitz (@wallento)
Hi Wilson,
I am planning to add the capability to set toplevel parameters on the command line. For that I have been playing around a bit: https://github.com/wallento/verilator/tree/parameters
Here is what I did:
Now I am trying to replace the existing value (must be AstConst, right?) with a new one, but cannot use valuep(AstNode *) as it already assigned (here: https://github.com/wallento/verilator/blob/master/src/V3Ast.cpp#L329).
Can you tell me if I am entirely off the track or give me a hint how to get one from there?
Thanks!
Best,
Stefan
The text was updated successfully, but these errors were encountered: