Support $value$plusargs with variables, bug1165.

Changes

@@ -11,6 +11,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ***   Support arrayed parameter overrides, bug1153. [John Stevenson]
 
+***   Support $value$plusargs with variables, bug1165. [Wesley Terpstra]
+
 ****  Support modport access to un-modport objects, bug1161. [Todd Strader]
 
 ****  Add stack trace when can't optimize function, bug1158. [Todd Strader]

include/verilated.cpp

@@ -302,8 +302,7 @@ void _vl_vsformat(string& output, const char* formatp, va_list ap) {
     bool inPct = false;
     bool widthSet = false;
     int width = 0;
-    const char* pos = formatp;
+    for (const char* pos = formatp; *pos; ++pos) {
-    for (; *pos; ++pos) {
 	if (!inPct && pos[0]=='%') {
 	    pctp = pos;
 	    inPct = true;
@@ -1077,22 +1076,36 @@ IData VL_TESTPLUSARGS_I(const char* formatp) {
     else return 1;
 }
 
-IData VL_VALUEPLUSARGS_IN(int, const char* prefixp, char, string& ldr) {
+IData VL_VALUEPLUSARGS_INW(int rbits, const string& ld, WDataOutP rwp) {
-    const string& match = VerilatedImp::argPlusMatch(prefixp);
+    string prefix;
-    const char* dp = match.c_str() + 1 /*leading + */ + strlen(prefixp);
+    bool inPct = false;
-    if (match == "") return 0;
+    bool done = false;
-    ldr = string(dp);
+    char fmt = ' ';
-    return 1;
+    for (const char* posp = ld.c_str(); !done && *posp; ++posp) {
-}
+	if (!inPct && posp[0]=='%') {
+	    inPct = true;
+	} else if (!inPct) {   // Normal text
+	    prefix += *posp;
+	} else { // Format character
+	    switch (tolower(*posp)) {
+	    case '%':
+		prefix += *posp;
+		inPct = false;
+		break;
+	    default:
+		fmt = *posp;
+		done = true;
+		break;
+	    }
+	}
+    }
 
-IData VL_VALUEPLUSARGS_IW(int rbits, const char* prefixp, char fmt, WDataOutP rwp) {
+    const string& match = VerilatedImp::argPlusMatch(prefix.c_str());
-    const string& match = VerilatedImp::argPlusMatch(prefixp);
+    const char* dp = match.c_str() + 1 /*leading + */ + prefix.length();
-    const char* dp = match.c_str() + 1 /*leading + */ + strlen(prefixp);
     if (match == "") return 0;
+
     VL_ZERO_RESET_W(rbits, rwp);
     switch (tolower(fmt)) {
-    case '%':
-	break;
     case 'd':
 	vlsint64_t ld;
 	sscanf(dp,"%30" VL_PRI64 "d",&ld);
@@ -1108,18 +1121,47 @@ IData VL_VALUEPLUSARGS_IW(int rbits, const char* prefixp, char fmt, WDataOutP rw
     case 'x':
 	_vl_vsss_based(rwp,rbits, 4, dp, 0, (int)strlen(dp));
 	break;
-    case 's':
+    case 's': // string/no conversion
-	for (int i=0, lsb=0, pos=((int)strlen(dp))-1; i<rbits && pos>=0; pos--) {
+	for (int i=0, lsb=0, posp=((int)strlen(dp))-1; i<rbits && posp>=0; posp--) {
-	    _vl_vsss_setbit(rwp,rbits,lsb, 8, dp[pos]); lsb+=8;
+	    _vl_vsss_setbit(rwp,rbits,lsb, 8, dp[posp]); lsb+=8;
 	}
 	break;
-    default:  // Compile time should have found all errors before this
+    case 'e': //FALLTHRU - Unsupported
-	vl_fatal (__FILE__, __LINE__, "", "$value$plusargs format error");
+    case 'f': //FALLTHRU - Unsupported
-	break;
+    case 'g': //FALLTHRU - Unsupported
+    default:  // Other simulators simply return 0 in these cases and don't error out
+	return 0;
     }
     _VL_CLEAN_INPLACE_W(rbits,rwp);
     return 1;
 }
+IData VL_VALUEPLUSARGS_INN(int rbits, const string& ld, string& rdr) {
+    string prefix;
+    bool inPct = false;
+    bool done = false;
+    for (const char* posp = ld.c_str(); !done && *posp; ++posp) {
+	if (!inPct && posp[0]=='%') {
+	    inPct = true;
+	} else if (!inPct) {   // Normal text
+	    prefix += *posp;
+	} else { // Format character
+	    switch (tolower(*posp)) {
+	    case '%':
+		prefix += *posp;
+		inPct = false;
+		break;
+	    default:
+		done = true;
+		break;
+	    }
+	}
+    }
+    const string& match = VerilatedImp::argPlusMatch(prefix.c_str());
+    const char* dp = match.c_str() + 1 /*leading + */ + prefix.length();
+    if (match == "") return 0;
+    rdr = string(dp);
+    return 1;
+}
 
 const char* vl_mc_scan_plusargs(const char* prefixp) {
     const string& match = VerilatedImp::argPlusMatch(prefixp);

include/verilated_heavy.h

@@ -66,6 +66,19 @@ extern void VL_READMEM_N(bool hex, int width, int depth, int array_lsb, int fnwo
 extern IData VL_SSCANF_INX(int lbits, const string& ld, const char* formatp, ...);
 extern void VL_SFORMAT_X(int obits_ignored, string &output, const char* formatp, ...);
 extern string VL_SFORMATF_NX(const char* formatp, ...);
-extern IData VL_VALUEPLUSARGS_IN(int rbits, const char* prefixp, char fmt, string& ldr);
+extern IData VL_VALUEPLUSARGS_INW(int rbits, const string& ld, WDataOutP rdp);
+inline IData VL_VALUEPLUSARGS_INI(int rbits, const string& ld, IData& rdr) {
+    IData rwp[1];
+    IData got = VL_VALUEPLUSARGS_INW(rbits,ld,rwp);
+    if (got) rdr = rwp[0];
+    return got;
+}
+inline IData VL_VALUEPLUSARGS_INQ(int rbits, const string& ld, QData& rdr) {
+    IData rwp[2];
+    IData got = VL_VALUEPLUSARGS_INW(rbits,ld,rwp);
+    if (got) rdr = VL_SET_QW(rwp);
+    return got;
+}
+extern IData VL_VALUEPLUSARGS_INN(int, const string& ld, string& rdr);
 
 #endif // Guard

src/V3AstNodes.h

@@ -2658,28 +2658,24 @@ public:
 class AstValuePlusArgs : public AstNodeMath {
     // Parents: expr
     // Child: variable to set.  If NULL then this is a $test$plusargs instead of $value$plusargs
-private:
-    string	m_text;
 public:
-    AstValuePlusArgs(FileLine* fileline, const string& text, AstNode* exprsp)
+    AstValuePlusArgs(FileLine* fileline, AstNode* searchp, AstNode* outp)
-	: AstNodeMath (fileline), m_text(text) {
+	: AstNodeMath (fileline) {
-	setOp1p(exprsp);
+	setOp1p(searchp); setOp2p(outp);
     }
     ASTNODE_NODE_FUNCS(ValuePlusArgs)
-    virtual string name()	const { return m_text; }
     virtual string verilogKwd() const { return "$value$plusargs"; }
-    virtual string emitVerilog() { return verilogKwd(); }
+    virtual string emitVerilog() { return "%f$value$plusargs(%l, %k%r)"; }
-    virtual string emitC() { return "VL_VALUEPLUSARGS_%nq(%lw, %P, NULL)"; }
+    virtual string emitC() { V3ERROR_NA; return ""; }
     virtual bool isGateOptimizable() const { return false; }
     virtual bool isPredictOptimizable() const { return false; }
     virtual bool cleanOut() { return true; }
-    virtual V3Hash sameHash() const { return V3Hash(text()); }
+    virtual V3Hash sameHash() const { return V3Hash(); }
-    virtual bool same(AstNode* samep) const {
+    virtual bool same(AstNode* samep) const { return true; }
-	return text()==samep->castValuePlusArgs()->text(); }
+    AstNode*	searchp() const { return op1p(); }	// op1 = Search expression
-    AstNode*	exprsp()	const { return op1p(); }	// op1 = Expressions to output
+    void 	searchp(AstNode* nodep)	{ setOp1p(nodep); }
-    void 	exprsp(AstNode* nodep)	{ setOp1p(nodep); }	// op1 = Expressions to output
+    AstNode*	outp() const { return op2p(); }		// op2 = Expressions to output
-    string 	text()		const { return m_text; }	// * = Text to display
+    void 	outp(AstNode* nodep) { setOp2p(nodep); }
-    void 	text(const string& text) { m_text=text; }
 };
 
 class AstTestPlusArgs : public AstNodeMath {

src/V3EmitC.cpp

@@ -277,54 +277,15 @@ public:
 	displayNode(nodep, NULL, nodep->text(), nodep->exprsp(), true);
     }
     virtual void visit(AstValuePlusArgs* nodep) {
-	string prefix;
+	puts("VL_VALUEPLUSARGS_IN");
-	char format = '?';
+	emitIQW(nodep->outp());
-	bool pct=false;
-	int got=0;
-	string txt = nodep->text();
-	for (string::const_iterator it=txt.begin(); it!=txt.end(); ++it) {
-	    char ch = *it;
-	    if (pct) {
-		pct = false;
-		switch (tolower(ch)) {
-		case '%':
-		    prefix += ch;
-		    break;
-		case 'd': // FALLTHRU
-		case 'o': // FALLTHRU
-		case 'h': // FALLTHRU
-		case 'x': // FALLTHRU
-		case 'b': // FALLTHRU
-		case 'v': // FALLTHRU
-		case 's':
-		    got++; format = tolower(ch);
-		    break;
-		case 'e': // FALLTHRU
-		case 'f': // FALLTHRU
-		case 'g':
-		    got++; format = tolower(ch);
-		    nodep->v3error("Unsupported $value$plusargs format qualifier: '"<<ch<<"'"<<endl);
-		    break;
-		default:
-		    got++;
-		    nodep->v3error("Illegal $value$plusargs format qualifier: '"<<ch<<"'"<<endl);
-		    break;
-		}
-	    }
-	    else if (ch == '%') pct = true;
-	    else prefix += ch;
-	}
-	if (got!=1) nodep->v3error("Missing or extra $value$plusargs format qualifier: '"<<nodep->text()<<"'"<<endl);
-	puts("VL_VALUEPLUSARGS_I");
-	emitIQW(nodep->exprsp());
 	puts("(");
-	puts(cvtToStr(nodep->exprsp()->widthMin()));  // Note argument width, not node width (which is always 32)
+	puts(cvtToStr(nodep->outp()->widthMin()));
-	putbs(",");
-	putsQuoted(prefix);
-	putbs(",");
-	puts("'"); puts(cvtToStr(format)); puts("'");
 	puts(",");
-	nodep->exprsp()->iterateAndNext(*this);
+	emitCvtPackStr(nodep->searchp());
+	puts(",");
+	putbs("");
+	nodep->outp()->iterateAndNext(*this);
 	puts(")");
     }
     virtual void visit(AstTestPlusArgs* nodep) {
@@ -618,6 +579,23 @@ public:
 	puts(nodep->hiername());
 	puts(nodep->varp()->name());
     }
+    void emitCvtPackStr(AstNode* nodep) { 
+	if (AstConst* constp = nodep->castConst()) {
+	    putbs("string(");
+	    putsQuoted(constp->num().toString());
+	    puts(")");
+	} else {
+	    putbs("VL_CVT_PACK_STR_N");
+	    emitIQW(nodep);
+	    puts("(");
+	    if (nodep->isWide()) {
+		puts(cvtToStr(nodep->widthWords()));  // Note argument width, not node width (which is always 32)
+		puts(",");
+	    }
+	    nodep->iterateAndNext(*this);
+	    puts(")");
+	}
+    }
     void emitConstant(AstConst* nodep, AstVarRef* assigntop, const string& assignString) {
 	// Put out constant set to the specified variable, or given variable in a string
 	if (nodep->num().isFourState()) {

src/V3EmitV.cpp

@@ -238,10 +238,6 @@ class EmitVBaseVisitor : public EmitCBaseVisitor {
     virtual void visit(AstSFormatF* nodep) {
 	visitNodeDisplay(nodep, NULL, nodep->text(), nodep->exprsp());
     }
-    virtual void visit(AstValuePlusArgs* nodep) {
-	visitNodeDisplay(nodep, NULL, nodep->text(), nodep->exprsp());
-    }
-
     virtual void visit(AstFOpen* nodep) {
 	putfs(nodep,nodep->verilogKwd());
 	putbs(" (");

src/V3LinkLValue.cpp

@@ -174,8 +174,10 @@ private:
     virtual void visit(AstValuePlusArgs* nodep) {
 	bool last_setRefLvalue = m_setRefLvalue;
 	{
+	    m_setRefLvalue = false;
+	    nodep->searchp()->iterateAndNext(*this);
 	    m_setRefLvalue = true;
-	    nodep->exprsp()->iterateAndNext(*this);
+	    nodep->outp()->iterateAndNext(*this);
 	}
 	m_setRefLvalue = last_setRefLvalue;
     }

src/V3Width.cpp

@@ -2094,7 +2094,8 @@ private:
     }
     virtual void visit(AstValuePlusArgs* nodep) {
 	if (m_vup->prelim()) {
-	    userIterateAndNext(nodep->exprsp(), WidthVP(SELF,BOTH).p());
+	    userIterateAndNext(nodep->searchp(), WidthVP(SELF,BOTH).p());
+	    userIterateAndNext(nodep->outp(), WidthVP(SELF,BOTH).p());
 	    nodep->dtypeSetSigned32();  // Spec says integer return
 	}
     }

src/verilog.y

@@ -2710,12 +2710,12 @@ system_f_call<nodep>:		// IEEE: system_tf_call (as func)
 	|	yD_SQRT '(' expr ')'			{ $$ = new AstSqrtD($1,$3); }
 	|	yD_SSCANF '(' expr ',' str commaVRDListE ')'	{ $$ = new AstSScanF($1,*$5,$3,$6); }
 	|	yD_STIME parenE				{ $$ = new AstSel($1,new AstTime($1),0,32); }
-	|	yD_SYSTEM  '(' expr ')'				{ $$ = new AstSystemF($1,$3); }
+	|	yD_SYSTEM  '(' expr ')'			{ $$ = new AstSystemF($1,$3); }
 	|	yD_TESTPLUSARGS '(' str ')'		{ $$ = new AstTestPlusArgs($1,*$3); }
 	|	yD_TIME	parenE				{ $$ = new AstTime($1); }
 	|	yD_UNPACKED_DIMENSIONS '(' expr ')'	{ $$ = new AstAttrOf($1,AstAttrType::DIM_UNPK_DIMENSIONS,$3); }
 	|	yD_UNSIGNED '(' expr ')'		{ $$ = new AstUnsigned($1,$3); }
-	|	yD_VALUEPLUSARGS '(' str ',' expr ')'	{ $$ = new AstValuePlusArgs($1,*$3,$5); }
+	|	yD_VALUEPLUSARGS '(' expr ',' expr ')'	{ $$ = new AstValuePlusArgs($1,$3,$5); }
 	;
 
 exprOrDataType<nodep>:		// expr | data_type: combined to prevent conflicts

test_regress/t/t_sys_plusargs.v

@@ -8,6 +8,8 @@ module t;
    integer p_i;
    reg [7*8:1] p_str;
    string      sv_str;
+   reg [7*8:1] p_in;
+   string      sv_in;
 
    initial begin
       if ($test$plusargs("PLUS")!==1) $stop;
@@ -29,14 +31,34 @@ module t;
       if ($value$plusargs("INT=%o", p_i)!==1) $stop;
       if (p_i !== 32'o1234) $stop;
 
+      p_str = "none";
       if ($value$plusargs("IN%s", p_str)!==1) $stop;
       $display("str='%s'",p_str);
       if (p_str !== "T=1234") $stop;
 
+      sv_str = "none";
       if ($value$plusargs("IN%s", sv_str)!==1) $stop;
       $display("str='%s'",sv_str);
       if (sv_str != "T=1234") $stop;
 
+      p_in = "IN%s";
+`ifdef VERILATOR
+      p_in = $c(p_in); // Prevent constant propagation
+`endif
+      sv_str = "none";
+      if ($value$plusargs(p_in, sv_str)!==1) $stop;
+      $display("str='%s'",sv_str);
+      if (sv_str != "T=1234") $stop;
+
+      sv_in = "INT=%d";
+`ifdef VERILATOR
+      if ($c1(0)) sv_in = "NEVER"; // Prevent constant propagation
+`endif
+      p_i = 0;
+      if ($value$plusargs(sv_in, p_i)!==1) $stop;
+      $display("i='%d'",p_i);
+      if (p_i !== 32'd1234) $stop;
+
       $write("*-* All Finished *-*\n");
       $finish;
    end

test_regress/t/t_sys_plusargs_bad.pl

@@ -8,13 +8,11 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
 # Version 2.0.
 
 compile (
-	 fails=>$Self->{v3},
+    );
-	 expect=>
+
-q{%Error: t/t_sys_plusargs_bad.v:\d+: Missing or extra \$value\$plusargs format qualifier: 'NOTTHERE'
+execute (
-%Error: t/t_sys_plusargs_bad.v:\d+: Illegal \$value\$plusargs format qualifier: 'z'
+    check_finished=>1,
-%Error: t/t_sys_plusargs_bad.v:\d+: Missing or extra \$value\$plusargs format qualifier: 'INT=%x%x'
+    );
-%Error: Exiting due to.*},
-	 );
 
 ok(1);
 1;

test_regress/t/t_sys_plusargs_bad.v

@@ -17,6 +17,7 @@ module t;
       // BAD: Multi letter
       if ($value$plusargs("INT=%x%x", p_i)!==0) $stop;
 
-      $stop;
+      $write("*-* All Finished *-*\n");
+      $finish;
    end
 endmodule