Original Redmine Comment
Author Name: Stefan Wallentowitz (@wallento)
Original Date: 2016-03-14T12:12:57Z

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,
Stefan

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-03-15T01:57:55Z

Great, when you have all that you want please make a patch with everything in it.

You create a float like this:

new AstConst(fileline,AstConst::RealDouble(),string)

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
Author Name: Stefan Wallentowitz (@wallento)
Original Date: 2016-03-15T07:39:21Z

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,
Stefan

Original Redmine Comment
Author Name: Stefan Wallentowitz (@wallento)
Original Date: 2016-03-16T14:37:48Z

Hi Wilson,

attached please find my proposal for the patch. There are some things I am not entirely sure about:

• I needed to extend the switch expansion in the Perl wrapper to allow for Verilog notation (32'h0) on the command line. I assume there is better Perl code to do this.
• Added a test after the LinkDot phase to check if all parameters were actually used. I am not sure if calling v3fatal is the proper reaction.

Looking forward to your comments!

Cheers,
Stefan

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-03-20T15:16:54Z

 +     -G<name>=<value>           Overwrite toplevel parameter
 +     +parameter+<name>=<value>  Overwrite toplevel parameter
 +=item +parameter+I<name>=I<value>
 +=item +parameter+I<name>=I<value>+I<name2>=I<value2>...

I looked at three simulators. NCVerilog doesn't support this. VCS supports:

     -pvalue+<parameter_hierarchical_name>=<value>

Modelsim supports:

     -g<Name>=<Value>
     -G<Name>=<Value>

So I would remove the +I... support, and add an alias to -pvalue+.
Unless some other simulator +parameter+ I would remove that.

+           if (svalue[0] == '"') {
+               // This is a string
+               string v = svalue.substr(1, svalue.find('"', 1) - 1);
+               V3Number n(V3Number::VerilogStringLiteral(),nodep->fileline(),v);
+               value = new AstConst(nodep->fileline(),n);
+           } else if ((svalue.find(".") != string::npos) ||
+                   (svalue.find("e") != string::npos)) {
+               double v = V3ParseImp::parseDouble(svalue.c_str(), svalue.length());
+               value = new AstConst(nodep->fileline(),AstConst::RealDouble(),v);
+           } else {
+               int v = strtol(svalue.c_str(), NULL, 0);
+               if (v != 0) {
+                   V3Number n(fl, 32, v);
+                   value = new AstConst(nodep->fileline(),n);
+                   UINFO(4,value<<endl);
+               } else {
+                   V3Number n(fl, svalue.c_str());
+                   value = new AstConst(nodep->fileline(),n);
+               }
+           }

Please move this block a parseParamLiteral or something function that
returns a AstConst*. Also I think it should check the strtol return pointer for errors
and bail with an error accordingly.

+           else if ( !strncmp (sw, "+parameter+", 11)) {
+                addParameter (string (sw+strlen("+parameter+")), true);
+           }

Nit, use !strncmp (sw, "+parameter+", strlen("+parameter+")). I know I
didn't do that elsewhere but should have.

Please add a t/t_flag_parameter test for each flavor of switch (e.g. can be
just one test using -pvalue -g and -G on three different parameters.)

Original Redmine Comment
Author Name: Stefan Wallentowitz (@wallento)
Original Date: 2016-03-22T09:22:55Z

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,
Stefan

Original Redmine Comment
Author Name: Stefan Wallentowitz (@wallento)
Original Date: 2016-03-22T09:26:05Z

Oh, and one remaining problem is that it cannot use double quotes strings in parameter files (.vc). I think there are three options:

• Live with it
• Remove the error thrown when a double quote is found in the vc file
• Test the vc file mroe detailed, which essentially can mean parsing it

How would you like it solved?

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-03-23T11:45:47Z

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
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-03-23T11:48:53Z

Pushed your patch 0001 to trunk, thanks.

Original Redmine Comment
Author Name: Stefan Wallentowitz (@wallento)
Original Date: 2016-03-23T12:23:39Z

In V3Options.cpp:1082 it says:

 // Split into argument list and process
 // Note we don't respect quotes.  It seems most simulators dont.
 // Woez those that expect it; we'll at least complain.
 if (whole_file.find("\"") != string::npos) {
fl->v3error("Double quotes in -f files cause unspecified behavior.");
 }

I think we can stick with this and the patch is fine then.

Best,
Stefan

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-03-24T23:15:46Z

Good stuff. Pushed to git towards 3.883.

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-05-19T01:17:43Z

In 3.884.