diff --git a/Changes b/Changes index e1e249363..77b84c525 100644 --- a/Changes +++ b/Changes @@ -5,6 +5,8 @@ indicates the contributor was also the author of the fix; Thanks! * Verilator 3.852 devel +*** Support named function and task arguments. [Chris Randall] + * Verilator 3.851 2013-08-15 diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 8d6e1c9a5..c1ed6b917 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -1294,6 +1294,25 @@ public: void svImplicit(bool flag) { m_svImplicit=flag; } }; +struct AstArg : public AstNode { + // An argument to a function/task +private: + string m_name; // Pin name, or "" for number based interconnect +public: + AstArg(FileLine* fl, const string& name, AstNode* exprp) + : AstNode(fl) + ,m_name(name) { + setNOp1p(exprp); + } + ASTNODE_NODE_FUNCS(Arg, ARG) + virtual string name() const { return m_name; } // * = Pin name, ""=go by number + virtual void name(const string& name) { m_name = name; } + virtual V3Hash sameHash() const { return V3Hash(); } + void exprp(AstNode* nodep) { addOp1p(nodep); } + AstNode* exprp() const { return op1p()->castNode(); } // op1 = Expression connected to pin, NULL if unconnected + bool emptyConnectNoNext() const { return !exprp() && name()=="" && !nextp(); } +}; + struct AstModule : public AstNodeModule { // A module declaration AstModule(FileLine* fl, const string& name) diff --git a/src/V3Const.cpp b/src/V3Const.cpp index e69149dd8..dc785f40d 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -1230,6 +1230,8 @@ private: && ((!m_params // Can reduce constant wires into equations && m_doNConst && v3Global.opt.oConst() + && !(nodep->varp()->isFuncLocal() // Default value, not a "known" constant for this usage + && nodep->varp()->isInput()) && !nodep->varp()->isSigPublic()) || nodep->varp()->isParam())) { AstConst* constp = nodep->varp()->valuep()->castConst(); @@ -1643,7 +1645,10 @@ private: replaceWithSimulation(nodep); } } - + virtual void visit(AstArg* nodep, AstNUser*) { + // replaceWithSimulation on the Arg's parent FuncRef replaces these + nodep->iterateChildren(*this); + } virtual void visit(AstWhile* nodep, AstNUser*) { nodep->iterateChildren(*this); if (m_doNConst) { diff --git a/src/V3EmitV.cpp b/src/V3EmitV.cpp index 87b48cf21..78eed17d8 100644 --- a/src/V3EmitV.cpp +++ b/src/V3EmitV.cpp @@ -525,6 +525,9 @@ class EmitVBaseVisitor : public EmitCBaseVisitor { nodep->pinsp()->iterateAndNext(*this); puts(")"); } + virtual void visit(AstArg* nodep, AstNUser*) { + nodep->exprp()->iterateAndNext(*this); + } // Terminals virtual void visit(AstVarRef* nodep, AstNUser*) { if (nodep->varScopep()) diff --git a/src/V3Simulate.h b/src/V3Simulate.h index f46532359..ead3a836a 100644 --- a/src/V3Simulate.h +++ b/src/V3Simulate.h @@ -607,7 +607,7 @@ private: V3TaskConnects tconnects = V3Task::taskConnects(nodep, nodep->taskp()->stmtsp()); for (V3TaskConnects::iterator it=tconnects.begin(); it!=tconnects.end(); ++it) { AstVar* portp = it->first; - AstNode* pinp = it->second; + AstNode* pinp = it->second->exprp(); if (pinp==NULL) { // Too few arguments in function call - ignore it } else { diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 16f72254a..7d487f73f 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -370,7 +370,8 @@ private: V3TaskConnects tconnects = V3Task::taskConnects(refp, beginp); for (V3TaskConnects::iterator it=tconnects.begin(); it!=tconnects.end(); ++it) { AstVar* portp = it->first; - AstNode* pinp = it->second; + AstArg* argp = it->second; + AstNode* pinp = argp->exprp(); portp->unlinkFrBack(); pushDeletep(portp); // Remove it from the clone (not original) if (pinp==NULL) { // Too few arguments in function call @@ -378,6 +379,7 @@ private: UINFO(9, " Port "<unlinkFrBack(); // Relinked to assignment below + argp->unlinkFrBack()->deleteTree(); // Args no longer needed // if ((portp->isInout()||portp->isOutput()) && pinp->castConst()) { pinp->v3error("Function/task output connected to constant instead of variable: "+portp->prettyName()); @@ -476,7 +478,7 @@ private: V3TaskConnects tconnects = V3Task::taskConnects(refp, refp->taskp()->stmtsp()); for (V3TaskConnects::iterator it=tconnects.begin(); it!=tconnects.end(); ++it) { AstVar* portp = it->first; - AstNode* pinp = it->second; + AstNode* pinp = it->second->exprp(); if (!pinp) { // Too few arguments in function call } else { @@ -532,9 +534,10 @@ private: AstNode* nextpinp; for (AstNode* pinp = refp->pinsp(); pinp; pinp=nextpinp) { nextpinp = pinp->nextp(); - // Move pin to the CCall - pinp->unlinkFrBack(); - ccallp->addArgsp(pinp); + // Move pin to the CCall, removing all Arg's + AstNode* exprp = pinp->castArg()->exprp(); + exprp->unlinkFrBack(); + ccallp->addArgsp(exprp); } if (outvscp) { @@ -1177,20 +1180,20 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp) // Missing pin/expr? We return (pinvar, NULL) // Extra pin/expr? We clean it up + typedef map NameToIndex; + NameToIndex nameToIndex; V3TaskConnects tconnects; if (!nodep->taskp()) nodep->v3fatalSrc("unlinked"); // Find ports //map name_to_pinnum; - int tpinnum = 0; // Note grammar starts pin counting at one + int tpinnum = 0; AstVar* sformatp = NULL; for (AstNode* stmtp = taskStmtsp; stmtp; stmtp=stmtp->nextp()) { if (AstVar* portp = stmtp->castVar()) { if (portp->isIO()) { - tconnects.push_back(make_pair(portp, (AstNode*)NULL)); - // Eventually we'll do name based connections - // That'll require a AstTpin or somesuch which will replace the ppinnum counting - //name_to_pinnum.insert(make_pair(portp->name(), tpinnum)); + tconnects.push_back(make_pair(portp, (AstArg*)NULL)); + nameToIndex.insert(make_pair(portp->name(), tpinnum)); // For name based connections tpinnum++; if (portp->attrSFormat()) { sformatp = portp; @@ -1201,27 +1204,90 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp) } } - // Connect pins + // Find pins int ppinnum = 0; - for (AstNode* pinp = nodep->pinsp(); pinp; pinp=pinp->nextp()) { - if (ppinnum >= tpinnum) { - if (sformatp) { - tconnects.push_back(make_pair(sformatp, (AstNode*)NULL)); + bool reorganize = false; + for (AstNode* nextp, *pinp = nodep->pinsp(); pinp; pinp=nextp) { + nextp = pinp->nextp(); + AstArg* argp = pinp->castArg(); if (!argp) pinp->v3fatalSrc("Non-arg under ftask reference"); + if (argp->name() != "") { + // By name + NameToIndex::iterator it = nameToIndex.find(argp->name()); + if (it == nameToIndex.end()) { + pinp->v3error("No such argument '"<prettyName() + <<"' in function call to "<taskp()->prettyTypeName()); + // We'll just delete it; seems less error prone than making a false argument + pinp->unlinkFrBack()->deleteTree(); pinp=NULL; } else { - pinp->v3error("Too many arguments in function call to "<taskp()->prettyTypeName()); - // We'll just delete them; seems less error prone than making a false argument - pinp->unlinkFrBackWithNext()->deleteTree(); pinp=NULL; - break; + if (tconnects[it->second].second) { + pinp->v3error("Duplicate argument '"<prettyName() + <<"' in function call to "<taskp()->prettyTypeName()); + } + argp->name(""); // Can forget name as will add back in pin order + tconnects[it->second].second = argp; + reorganize = true; + } + } else { // By pin number + if (ppinnum >= tpinnum) { + if (sformatp) { + tconnects.push_back(make_pair(sformatp, (AstArg*)NULL)); + tconnects[ppinnum].second = argp; + tpinnum++; + } else { + pinp->v3error("Too many arguments in function call to "<taskp()->prettyTypeName()); + // We'll just delete it; seems less error prone than making a false argument + pinp->unlinkFrBack()->deleteTree(); pinp=NULL; + } + } else { + tconnects[ppinnum].second = argp; } } - tconnects[ppinnum].second = pinp; ppinnum++; } - while (ppinnum < tpinnum) { - nodep->v3error("Too few arguments in function call to "<taskp()->prettyTypeName()); - UINFO(1,"missing argument for '"<prettyName()<<"'"<exprp()) { + AstNode* newvaluep = NULL; + if (!portp->valuep()) { + nodep->v3error("Missing argument on non-defaulted argument '"<prettyName() + <<"' in function call to "<taskp()->prettyTypeName()); + newvaluep = new AstConst(nodep->fileline(), AstConst::Unsized32(), 0); + } else if (!portp->valuep()->castConst()) { + // Problem otherwise is we might have a varref, task call, or something else that only + // makes sense in the domain of the function, not the callee. + nodep->v3error("Unsupported: Non-constant default value in missing argument '"<prettyName() + <<"' in function call to "<taskp()->prettyTypeName()); + newvaluep = new AstConst(nodep->fileline(), AstConst::Unsized32(), 0); + } else { + newvaluep = portp->valuep()->cloneTree(true); + } + // To avoid problems with callee needing to know to deleteTree or not, we make this into a pin + UINFO(9,"Default pin for "<fileline(), portp->name(), newvaluep); + if (tconnects[i].second) { // Have a "NULL" pin already defined for it + tconnects[i].second->unlinkFrBack()->deleteTree(); tconnects[i].second=NULL; + } + tconnects[i].second = newp; + reorganize = true; + } + if (tconnects[i].second) { UINFO(9,"Connect "< "< NONE"<pinsp()) nodep->pinsp()->unlinkFrBack(); // Must unlink each pin, not all pins linked together as one list + for (int i=0; iv3fatalSrc("Lost argument in func conversion"); + nodep->addPinsp(argp); + } + } + + if (debug()>=9) { + nodep->dumpTree(cout,"-ftref-out: "); + for (int i=0; iunlinkFrBackWithNext(&handle); // Format + additional args, if any + argp->unlinkFrBackWithNext(&handle); // Format + additional args, if any AstNode* argsp = NULL; - if (pinp->nextp()) argsp = pinp->nextp()->unlinkFrBackWithNext(); + while (AstArg* nextargp = argp->nextp()->castArg()) { + argsp = argsp->addNext(nextargp->exprp()->unlinkFrBackWithNext()); // Expression goes to SFormatF + nextargp->unlinkFrBack()->deleteTree(); // Remove the call's Arg wrapper + } string format; if (pinp->castConst()) format = pinp->castConst()->num().toString(); else pinp->v3error("Format to $display-like function must have constant format string"); - pushDeletep(pinp); pinp=NULL; + pushDeletep(argp); argp=NULL; AstSFormatF* newp = new AstSFormatF(nodep->fileline(), format, false, argsp); if (!newp->scopeNamep() && newp->formatScopeTracking()) { newp->scopeNamep(new AstScopeName(newp->fileline())); } - handle.relink(newp); + handle.relink(new AstArg(newp->fileline(), "", newp)); // Connection list is now incorrect (has extra args in it). goto reloop; // so exit early; next loop will correct it } diff --git a/src/verilog.y b/src/verilog.y index d99177f90..3663ee035 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -85,6 +85,7 @@ public: } // METHODS + void argWrapList(AstNodeFTaskRef* nodep); AstNodeDType* createArray(AstNodeDType* basep, AstRange* rangep, bool isPacked); AstVar* createVariable(FileLine* fileline, string name, AstRange* arrayp, AstNode* attrsp); AstNode* createSupplyExpr(FileLine* fileline, string name, int value); @@ -2470,6 +2471,7 @@ funcRef: // IEEE: part of tf_call // // property_instance property_identifier property_actual_arg // // sequence_instance sequence_identifier sequence_actual_arg // // let_expression let_identifier let_actual_arg + // id '(' list_of_argumentsE ')' { $$ = new AstFuncRef($2, *$1, $3); } | package_scopeIdFollows id '(' list_of_argumentsE ')' { $$ = AstDot::newIfPkg($2, $1, new AstFuncRef($2,*$2,$4)); } //UNSUP: idDotted is really just id to allow dotted method calls @@ -2497,7 +2499,7 @@ system_t_call: // IEEE: system_tf_call (as task) | yaD_IGNORE '(' exprList ')' { $$ = new AstSysIgnore($1,$3); } // | yaD_DPI parenE { $$ = new AstTaskRef($1,*$1,NULL); } - | yaD_DPI '(' exprList ')' { $$ = new AstTaskRef($2,*$1,$3); } + | yaD_DPI '(' exprList ')' { $$ = new AstTaskRef($2,*$1,$3); GRAMMARP->argWrapList($$->castTaskRef()); } // | yD_C '(' cStrList ')' { $$ = (v3Global.opt.ignc() ? NULL : new AstUCStmt($1,$3)); } | yD_FCLOSE '(' idClassSel ')' { $$ = new AstFClose($1, $3); } @@ -2542,7 +2544,7 @@ system_f_call: // IEEE: system_tf_call (as func) | yaD_IGNORE '(' exprList ')' { $$ = new AstConst($2,V3Number($2,"'b0")); } // Unsized 0 // | yaD_DPI parenE { $$ = new AstFuncRef($1,*$1,NULL); } - | yaD_DPI '(' exprList ')' { $$ = new AstFuncRef($2,*$1,$3); } + | yaD_DPI '(' exprList ')' { $$ = new AstFuncRef($2,*$1,$3); GRAMMARP->argWrapList($$->castFuncRef()); } // | yD_BITS '(' data_type ')' { $$ = new AstAttrOf($1,AstAttrType::DIM_BITS,$3); } | yD_BITS '(' data_type ',' expr ')' { $$ = new AstAttrOf($1,AstAttrType::DIM_BITS,$3,$5); } @@ -2598,9 +2600,10 @@ system_f_call: // IEEE: system_tf_call (as func) ; list_of_argumentsE: // IEEE: [list_of_arguments] - /* empty */ { $$ = NULL; } - | argsExprList { $$ = $1; } - //UNSUP empty arguments with just ,, + argsDottedList { $$ = $1; } + | argsExprListE { if ($1->castArg() && $1->castArg()->emptyConnectNoNext()) { $1->deleteTree(); $$ = NULL; } // Mis-created when have 'func()' + /*cont*/ else $$ = $1; } + | argsExprListE ',' argsDottedList { $$ = $1->addNextNull($3); } ; task_declaration: // ==IEEE: task_declaration @@ -3063,6 +3066,26 @@ argsExprList: // IEEE: part of list_of_arguments (used where ,, isn't le | argsExprList ',' expr { $$ = $1->addNext($3); } ; +argsExprListE: // IEEE: part of list_of_arguments + argsExprOneE { $$ = $1; } + | argsExprListE ',' argsExprOneE { $$ = $1->addNext($3); } + ; + +argsExprOneE: // IEEE: part of list_of_arguments + /*empty*/ { $$ = new AstArg(CRELINE(),"",NULL); } + | expr { $$ = new AstArg(CRELINE(),"",$1); } + ; + +argsDottedList: // IEEE: part of list_of_arguments + argsDotted { $$ = $1; } + | argsDottedList ',' argsDotted { $$ = $1->addNextNull($3); } + ; + +argsDotted: // IEEE: part of list_of_arguments + '.' idAny '(' ')' { $$ = new AstArg($1,*$2,NULL); } + | '.' idAny '(' expr ')' { $$ = new AstArg($1,*$2,$4); } + ; + stream_expression: // ==IEEE: stream_expression // // IEEE: array_range_expression expanded below expr { $$ = $1; } @@ -3590,6 +3613,16 @@ void V3ParseImp::parserClear() { VARDTYPE(NULL); } +void V3ParseGrammar::argWrapList(AstNodeFTaskRef* nodep) { + // Convert list of expressions to list of arguments + AstNode* outp = NULL; + while (nodep->pinsp()) { + AstNode* exprp = nodep->pinsp()->unlinkFrBack(); + outp = outp->addNext(new AstArg(exprp->fileline(), "", exprp)); + } + if (outp) nodep->addPinsp(outp); +} + AstNode* V3ParseGrammar::createSupplyExpr(FileLine* fileline, string name, int value) { FileLine* newfl = new FileLine (fileline); newfl->warnOff(V3ErrorCode::WIDTH, true); diff --git a/test_regress/t/t_func_bad.pl b/test_regress/t/t_func_bad.pl index b698ebfb4..a5650262e 100755 --- a/test_regress/t/t_func_bad.pl +++ b/test_regress/t/t_func_bad.pl @@ -11,9 +11,13 @@ compile ( v_flags2 => ["--lint-only"], fails=>1, expect=> -q{%Error: t/t_func_bad.v:\d+: Too few arguments in function call to FUNC 'add' +q{%Error: t/t_func_bad.v:\d+: Missing argument on non-defaulted argument 'from2' in function call to FUNC 'add' %Error: t/t_func_bad.v:\d+: Too many arguments in function call to FUNC 'add' -%Error: t/t_func_bad.v:\d+: Too few arguments in function call to TASK 'x' +%Error: t/t_func_bad.v:\d+: Missing argument on non-defaulted argument 'y' in function call to TASK 'x' +%Error: t/t_func_bad.v:\d+: Unsupported: Function output argument 'y' requires 1 bits, but connection's CONST '.*' generates 32 bits. +%Error: t/t_func_bad.v:\d+: No such argument 'no_such' in function call to FUNC 'f' +%Error: t/t_func_bad.v:\d+: Duplicate argument 'dup' in function call to FUNC 'f' +%Error: t/t_func_bad.v:\d+: Too many arguments in function call to FUNC 'f' %Error: Exiting due to}, ); diff --git a/test_regress/t/t_func_bad.v b/test_regress/t/t_func_bad.v index 22391e509..7ced7e832 100644 --- a/test_regress/t/t_func_bad.v +++ b/test_regress/t/t_func_bad.v @@ -9,6 +9,10 @@ module t; if (add(3'd1, 3'd2, 3'd3) != 0) $stop; // Too many args x; // Too few args if (hasout(3'd1) != 0) $stop; // outputs + // + f(.j(1), .no_such(2)); // Name mismatch + f(.dup(1), .dup(3)); // Duplicate + f(1,2,3); // Too many end function [2:0] add; @@ -29,4 +33,8 @@ module t; hasout = 0; endfunction + function int f( int j = 1, int dup = 0 ); + return (j<<16) | dup; + endfunction + endmodule diff --git a/test_regress/t/t_func_named.pl b/test_regress/t/t_func_named.pl new file mode 100755 index 000000000..f91289753 --- /dev/null +++ b/test_regress/t/t_func_named.pl @@ -0,0 +1,18 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you can +# redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. + +compile ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_func_named.v b/test_regress/t/t_func_named.v new file mode 100644 index 000000000..f51fd9962 --- /dev/null +++ b/test_regress/t/t_func_named.v @@ -0,0 +1,28 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2013 by Wilson Snyder. + +module t (/*AUTOARG*/); + + function int f( int j = 1, int s = 0 ); + return (j<<16) | s; + endfunction + +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); $stop; end while(0); + + initial begin + `checkh( f(.j(2), .s(1)) , 32'h2_0001 ); + `checkh( f(.s(1)) , 32'h1_0001 ); + `checkh( f(, 1) , 32'h1_0001 ); + `checkh( f(.j(2)) , 32'h2_0000 ); + `checkh( f(.s(1), .j(2)) , 32'h2_0001 ); + `checkh( f(.s(), .j()) , 32'h1_0000 ); + `checkh( f(2) , 32'h2_0000 ); + `checkh( f() , 32'h1_0000 ); + + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule