From dba88bae3c154a406433ab1333aeed68f40f9fba Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 12 Apr 2020 18:57:12 -0400 Subject: [PATCH] Support class new. --- bin/verilator | 4 ++-- src/V3AstNodes.h | 27 +++++++++++++++++----- src/V3CCtors.cpp | 18 +++++++-------- src/V3Clean.cpp | 4 ++++ src/V3EmitC.cpp | 4 +++- src/V3EmitCInlines.cpp | 2 +- src/V3LinkDot.cpp | 24 +++++++++++++++----- src/V3Localize.cpp | 1 + src/V3Name.cpp | 1 + src/V3SplitVar.cpp | 1 + src/V3Task.cpp | 36 ++++++++++++++++++++++++------ src/V3Width.cpp | 21 +++++++++++------ test_regress/t/t_class_new.out | 13 ----------- test_regress/t/t_class_new.pl | 8 +++---- test_regress/t/t_class_new.v | 6 +++++ test_regress/t/t_class_new_bad.out | 23 +++++++++---------- test_regress/t/t_class_new_bad.v | 10 +++++++-- 17 files changed, 132 insertions(+), 71 deletions(-) delete mode 100644 test_regress/t/t_class_new.out diff --git a/bin/verilator b/bin/verilator index d198eb2a7..cd1bfb1e3 100755 --- a/bin/verilator +++ b/bin/verilator @@ -3567,8 +3567,8 @@ path. Verilator class support is very limited and in active development. Verilator supports members, and methods. Verilator doe not support initial -values on class members, class static members, class new methods, class -extend, or class parameters. +values on class members, class static members, class extend, or class +parameters. =head2 Dotted cross-hierarchy references diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 63437c0c5..4eb5676f7 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -4066,24 +4066,22 @@ public: } }; -class AstNew : public AstNodeMath { +class AstNew : public AstNodeFTaskRef { // New as constructor // Don't need the class we are extracting from, as the "fromp()"'s datatype can get us to it // Parents: math|stmt // Children: varref|arraysel, math public: - AstNew(FileLine* fl, AstNode* argsp) - : ASTGEN_SUPER(fl) { - addNOp2p(argsp); - } + AstNew(FileLine* fl, AstNode* pinsp) + : ASTGEN_SUPER(fl, false, "new", pinsp) {} ASTNODE_NODE_FUNCS(New) virtual V3Hash sameHash() const { return V3Hash(); } virtual string emitVerilog() { return "new"; } virtual string emitC() { V3ERROR_NA_RETURN(""); } virtual bool cleanOut() const { return true; } virtual bool same(const AstNode* samep) const { return true; } + virtual bool hasDType() const { return true; } virtual int instrCount() const { return widthInstrs(); } - AstNode* argsp() const { return op2p(); } }; class AstNewCopy : public AstNodeMath { @@ -7040,6 +7038,23 @@ public: void fromp(AstNode* nodep) { setOp1p(nodep); } }; +class AstCNew : public AstNodeCCall { + // C++ new() call + // Parents: Anything above an expression + // Children: Args to the function +public: + AstCNew(FileLine* fl, AstCFunc* funcp, AstNode* argsp = NULL) + : ASTGEN_SUPER(fl, funcp, argsp) { + statement(false); + } + // Replacement form for V3Combine + // Note this removes old attachments from the oldp + AstCNew(AstCCall* oldp, AstCFunc* funcp) + : ASTGEN_SUPER(oldp, funcp) {} + virtual bool hasDType() const { return true; } + ASTNODE_NODE_FUNCS(CNew) +}; + class AstCReturn : public AstNodeStmt { // C++ return from a function // Parents: CFUNC/statement diff --git a/src/V3CCtors.cpp b/src/V3CCtors.cpp index 88821abba..8f114db16 100644 --- a/src/V3CCtors.cpp +++ b/src/V3CCtors.cpp @@ -149,8 +149,13 @@ void V3CCtors::cctorsAll() { // Process each module in turn AstCFunc* varResetFuncp; { - V3CCtorsVisitor var_reset (modp, "_ctor_var_reset"); + V3CCtorsVisitor var_reset( + modp, "_ctor_var_reset", + (VN_IS(modp, Class) ? EmitCBaseVisitor::symClassVar() : ""), + (VN_IS(modp, Class) ? "vlSymsp" : ""), + (VN_IS(modp, Class) ? "if (false && vlSymsp) {} // Prevent unused\n" : "")); varResetFuncp = var_reset.builtFuncp(); + for (AstNode* np = modp->stmtsp(); np; np = np->nextp()) { if (AstVar* varp = VN_CAST(np, Var)) { if (!varp->isIfaceParent() && !varp->isIfaceRef() @@ -164,7 +169,8 @@ void V3CCtors::cctorsAll() { if (v3Global.opt.coverage()) { V3CCtorsVisitor configure_coverage( modp, "_configure_coverage", - EmitCBaseVisitor::symClassVar() + ", bool first", "vlSymsp, first", + EmitCBaseVisitor::symClassVar() + ", bool first", + "vlSymsp, first", "if (false && vlSymsp && first) {} // Prevent unused\n"); for (AstNode* np = modp->stmtsp(); np; np = np->nextp()) { if (AstCoverDecl* coverp = VN_CAST(np, CoverDecl)) { @@ -175,14 +181,6 @@ void V3CCtors::cctorsAll() { } } } - if (VN_IS(modp, Class)) { - AstCFunc* funcp = new AstCFunc(modp->fileline(), "new", NULL, ""); - funcp->isConstructor(true); - funcp->isStatic(false); - funcp->slow(false); - modp->addStmtp(funcp); - funcp->addStmtsp(new AstCCall(varResetFuncp->fileline(), varResetFuncp)); - } if (VN_IS(modp, Class)) { AstCFunc* funcp = new AstCFunc(modp->fileline(), "~", NULL, ""); funcp->isDestructor(true); diff --git a/src/V3Clean.cpp b/src/V3Clean.cpp index 44b3b2457..9ee147aed 100644 --- a/src/V3Clean.cpp +++ b/src/V3Clean.cpp @@ -223,6 +223,10 @@ private: virtual void visit(AstScopeName* nodep) VL_OVERRIDE { setClean(nodep, true); } + virtual void visit(AstCNew* nodep) VL_OVERRIDE { + iterateChildren(nodep); + setClean(nodep, true); + } virtual void visit(AstSel* nodep) VL_OVERRIDE { operandTriop(nodep); setClean(nodep, nodep->cleanOut()); diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index 742ec2f25..46d6bd05e 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -850,8 +850,10 @@ public: puts(cvtToStr(nodep->fileline()->lineno())); puts(")"); } - virtual void visit(AstNew* nodep) VL_OVERRIDE { + virtual void visit(AstCNew* nodep) VL_OVERRIDE { puts("std::make_shared<" + prefixNameProtect(nodep->dtypep()) + ">("); + puts("vlSymsp"); // TODO make this part of argsp, and eliminate when unnecessary + if (nodep->argsp()) puts(", "); iterateAndNextNull(nodep->argsp()); puts(")"); } diff --git a/src/V3EmitCInlines.cpp b/src/V3EmitCInlines.cpp index f8d108f9f..3d6ee4b17 100644 --- a/src/V3EmitCInlines.cpp +++ b/src/V3EmitCInlines.cpp @@ -67,7 +67,7 @@ class EmitCInlines : EmitCBaseVisitor { v3Global.needHeavy(true); iterateChildren(nodep); } - virtual void visit(AstNew* nodep) VL_OVERRIDE { + virtual void visit(AstCNew* nodep) VL_OVERRIDE { if (v3Global.opt.savable()) v3error("Unsupported: --savable with dynamic new"); iterateChildren(nodep); } diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 3737d094b..521d02786 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -643,10 +643,9 @@ public: VSymEnt* foundp = NULL; while (!foundp) { foundp = lookupSymp->findIdFallback(prefix + dotname); // Might be NULL - if (prefix == "") { - break; - } - prefix = removeLastInlineScope(prefix); + if (prefix.empty()) break; + string nextPrefix = removeLastInlineScope(prefix); + if (prefix == nextPrefix) break; } if (!foundp) baddot = dotname; return foundp; @@ -686,7 +685,8 @@ class LinkDotFindVisitor : public AstNVisitor { bool m_inRecursion; // Inside a recursive module int m_paramNum; // Parameter number, for position based connection int m_beginNum; // Begin block number, 0=none seen - int m_modBeginNum; // Begin block number in module, 0=none seen + bool m_explicitNew; // Hit a "new" function + int m_modBeginNum; // Begin block number in module, 0=none seen // METHODS int debug() { return LinkDotState::debug(); } @@ -723,6 +723,13 @@ class LinkDotFindVisitor : public AstNVisitor { } return NULL; } + void makeImplicitNew(AstClass* nodep) { + AstFunc* newp = new AstFunc(nodep->fileline(), "new", NULL, NULL); + newp->isConstructor(true); + nodep->addMembersp(newp); + UINFO(8, "Made implicit new for " << nodep->name() << ": " << nodep << endl); + m_statep->insertBlock(m_curSymp, newp->name(), newp, m_packagep); + } // VISITs virtual void visit(AstNetlist* nodep) VL_OVERRIDE { @@ -842,10 +849,13 @@ class LinkDotFindVisitor : public AstNVisitor { m_paramNum = 0; m_beginNum = 0; m_modBeginNum = 0; + m_explicitNew = false; // m_modSymp/m_curSymp for non-packages set by AstCell above this module // Iterate iterateChildren(nodep); nodep->user4(true); + // Implicit new needed? + if (!m_explicitNew && m_statep->forPrimary()) makeImplicitNew(nodep); } m_scope = oldscope; m_modSymp = oldModSymp; @@ -971,6 +981,7 @@ class LinkDotFindVisitor : public AstNVisitor { // NodeTask: Remember its name for later resolution UINFO(5," "<name() == "new") m_explicitNew = true; // Remember the existing symbol table scope VSymEnt* oldCurSymp = m_curSymp; { @@ -1232,6 +1243,7 @@ public: m_inRecursion = false; m_paramNum = 0; m_beginNum = 0; + m_explicitNew = false; m_modBeginNum = 0; // iterate(rootp); @@ -2423,6 +2435,8 @@ private: <prettyName() <<"'"<<" as a "<nodep()->typeName() <<" but expected a task/function"); + } else if (VN_IS(nodep, New) && m_statep->forPrearray()) { + // Resolved in V3Width } else if (nodep->dotted() == "") { string suggest = m_statep->suggestSymFallback( dotSymp, nodep->name(), LinkNodeMatcherFTask()); diff --git a/src/V3Localize.cpp b/src/V3Localize.cpp index 197a4b2fe..7a001ad43 100644 --- a/src/V3Localize.cpp +++ b/src/V3Localize.cpp @@ -126,6 +126,7 @@ private: if (nodep->valuep()) clearOptimizable(nodep, "HasInitValue"); if (!VarFlags(nodep).m_stdFuncAsn) clearStdOptimizable(nodep, "NoStdAssign"); VarFlags flags (nodep); + if ((nodep->isMovableToBlock() // Blocktemp || !flags.m_notStd) // Or used only in block && !flags.m_notOpt // Optimizable diff --git a/src/V3Name.cpp b/src/V3Name.cpp index 95ba18acc..c2dc0aa39 100644 --- a/src/V3Name.cpp +++ b/src/V3Name.cpp @@ -56,6 +56,7 @@ private: string newname = string("__PVT__")+nodep->name(); nodep->name(newname); nodep->editCountInc(); + } else if (VN_IS(nodep, CFunc) && VN_CAST(nodep, CFunc)->isConstructor()) { } else { string rsvd = m_words.isKeyword(nodep->name()); if (rsvd != "") { diff --git a/src/V3SplitVar.cpp b/src/V3SplitVar.cpp index 729daca0f..fe0f9c057 100644 --- a/src/V3SplitVar.cpp +++ b/src/V3SplitVar.cpp @@ -501,6 +501,7 @@ class SplitUnpackedVarVisitor : public AstNVisitor, public SplitVarImpl { { m_contextp = nodep; AstNodeFTask* ftaskp = nodep->taskp(); + UASSERT_OBJ(ftaskp, nodep, "Unlinked"); // Iterate arguments of a function/task. for (AstNode *argp = nodep->pinsp(), *paramp = ftaskp->stmtsp(); argp; argp = argp->nextp(), paramp = paramp ? paramp->nextp() : NULL) { diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 91f02aaef..e92e97b21 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -186,7 +186,7 @@ private: m_assignwp = NULL; } virtual void visit(AstNodeFTaskRef* nodep) VL_OVERRIDE { - // Includes handling AstMethodCall + // Includes handling AstMethodCall, AstNew if (m_assignwp) { // Wire assigns must become always statements to deal with insertion // of multiple statements. Perhaps someday make all wassigns into always's? @@ -204,6 +204,7 @@ private: m_curVxp = getFTaskVertex(nodep); if (nodep->dpiImport()) m_curVxp->noInline(true); if (nodep->classMethod()) m_curVxp->noInline(true); // Until V3Task supports it + if (nodep->isConstructor()) m_curVxp->noInline(true); iterateChildren(nodep); m_curVxp = lastVxp; } @@ -474,7 +475,7 @@ private: } AstNode* createNonInlinedFTask(AstNodeFTaskRef* refp, const string& namePrefix, - AstVarScope* outvscp) { + AstVarScope* outvscp, AstCNew*& cnewpr) { // outvscp is the variable for functions only, if NULL, it's a task UASSERT_OBJ(refp->taskp(), refp, "Unlinked?"); AstCFunc* cfuncp = m_statep->ftaskCFuncp(refp->taskp()); @@ -483,12 +484,19 @@ private: AstNode* beginp = new AstComment(refp->fileline(), string("Function: ")+refp->name(), true); AstNodeCCall* ccallp; - if (AstMethodCall* mrefp = VN_CAST(refp, MethodCall)) { + if (AstNew* mrefp = VN_CAST(refp, New)) { + AstCNew* cnewp = new AstCNew(refp->fileline(), cfuncp); + cnewp->dtypep(refp->dtypep()); + ccallp = cnewp; + // Parent AstNew will replace with this CNew + cnewpr = cnewp; + } else if (AstMethodCall* mrefp = VN_CAST(refp, MethodCall)) { ccallp = new AstCMethodCall(refp->fileline(), mrefp->fromp()->unlinkFrBack(), cfuncp); + beginp->addNext(ccallp); } else { ccallp = new AstCCall(refp->fileline(), cfuncp); + beginp->addNext(ccallp); } - beginp->addNext(ccallp); // Convert complicated outputs to temp signals V3TaskConnects tconnects = V3Task::taskConnects(refp, refp->taskp()->stmtsp()); @@ -988,8 +996,10 @@ private: // so make it unique now. string suffix; // So, make them unique if (!nodep->taskPublic()) suffix = "_"+m_scopep->nameDotless(); + string name = ((nodep->name() == "new") ? "new" + : prefix + nodep->name() + suffix); AstCFunc* cfuncp = new AstCFunc(nodep->fileline(), - prefix + nodep->name() + suffix, + name, m_scopep, ((nodep->taskPublic() && rtnvarp) ? rtnvarp->cPubArgType(true, true) @@ -1003,6 +1013,7 @@ private: cfuncp->dpiImportWrapper(nodep->dpiImport()); cfuncp->isStatic(!(nodep->dpiImport() || nodep->taskPublic() || nodep->classMethod())); cfuncp->pure(nodep->pure()); + cfuncp->isConstructor(nodep->name() == "new"); //cfuncp->dpiImport // Not set in the wrapper - the called function has it set if (cfuncp->dpiExport()) cfuncp->cname(nodep->cname()); @@ -1017,6 +1028,10 @@ private: } else { // Need symbol table cfuncp->argTypes(EmitCBaseVisitor::symClassVar()); + if (cfuncp->name() == "new") { + cfuncp->addInitsp( + new AstCStmt(nodep->fileline(), "_ctor_var_reset(vlSymsp);\n")); + } } } if (nodep->dpiContext()) { @@ -1157,6 +1172,7 @@ private: m_scopep = NULL; } virtual void visit(AstNodeFTaskRef* nodep) VL_OVERRIDE { + // Includes handling AstMethodCall, AstNew UASSERT_OBJ(nodep->taskp(), nodep, "Unlinked?"); iterateIntoFTask(nodep->taskp()); // First, do hierarchical funcs UINFO(4," FTask REF "<ftaskNoInline(nodep->taskp())) { // This may share VarScope's with a public task, if any. Yuk. - beginp = createNonInlinedFTask(nodep, namePrefix, outvscp); + beginp = createNonInlinedFTask(nodep, namePrefix, outvscp, cnewp /*ref*/); } else { beginp = createInlinedFTask(nodep, namePrefix, outvscp); } // Replace the ref AstNode* visitp = NULL; - if (!nodep->isStatement()) { + if (VN_IS(nodep, New)) { + UASSERT_OBJ(!nodep->isStatement(), nodep, "new is non-stmt"); + UASSERT_OBJ(cnewp, nodep, "didn't create cnew for new"); + nodep->replaceWith(cnewp); + visitp = insertBeforeStmt(nodep, beginp); + } else if (!nodep->isStatement()) { UASSERT_OBJ(nodep->taskp()->isFunction(), nodep, "func reference to non-function"); AstVarRef* outrefp = new AstVarRef(nodep->fileline(), outvscp, false); nodep->replaceWith(outrefp); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 8550c3c6c..4f30f2c50 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1895,7 +1895,7 @@ private: } virtual void visit(AstMethodCall* nodep) VL_OVERRIDE { - UINFO(5," METHODSEL "<didWidth()) return; if (debug()>=9) nodep->dumpTree("-mts-in: "); // Should check types the method requires, but at present we don't do much @@ -2431,12 +2431,20 @@ private: return; } nodep->dtypep(refp); - if (nodep->argsp()) { - nodep->v3error("Unsupported: new with arguments"); - pushDeletep(nodep->argsp()->unlinkFrBackWithNext()); - //TODO code similar to AstNodeFTaskRef - //userIterateChildren(nodep, WidthVP(SELF, BOTH).p()); + + AstClass* classp = refp->classp(); + UASSERT_OBJ(classp, nodep, "Unlinked"); + if (AstNodeFTask* ftaskp = VN_CAST(classp->findMember("new"), Func)) { + nodep->taskp(ftaskp); + nodep->packagep(classp); + } else { + // Either made explicitly or V3LinkDot made implicitly + classp->v3fatalSrc("Can't find class's new"); } + + userIterate(nodep->taskp(), NULL); + userIterateChildren(nodep, NULL); + processFTaskRefArgs(nodep); } virtual void visit(AstNewCopy* nodep) VL_OVERRIDE { if (nodep->didWidthAndSet()) return; @@ -3406,7 +3414,6 @@ private: userIterateChildren(nodep, NULL); if (nodep->isConstructor()) { // Pretend it's void so less special casing needed when look at dtypes - nodep->v3error("Unsupported: new constructor"); nodep->dtypeSetVoid(); } else if (nodep->fvarp()) { m_funcp = VN_CAST(nodep, Func); diff --git a/test_regress/t/t_class_new.out b/test_regress/t/t_class_new.out deleted file mode 100644 index 6ddfd5636..000000000 --- a/test_regress/t/t_class_new.out +++ /dev/null @@ -1,13 +0,0 @@ -%Error: t/t_class_new.v:9:13: Unsupported: new constructor - : ... In instance t - 9 | function new(); - | ^~~ -%Error: t/t_class_new.v:16:13: Unsupported: new constructor - : ... In instance t - 16 | function new(int i); - | ^~~ -%Error: t/t_class_new.v:27:12: Unsupported: new with arguments - : ... In instance t - 27 | c2 = new(2); - | ^~~ -%Error: Exiting due to diff --git a/test_regress/t/t_class_new.pl b/test_regress/t/t_class_new.pl index 64b3d2dde..27b3049d2 100755 --- a/test_regress/t/t_class_new.pl +++ b/test_regress/t/t_class_new.pl @@ -11,13 +11,11 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(simulator => 1); compile( - fails => $Self->{vlt_all}, - expect_filename => $Self->{golden_filename}, ); -#execute( -# check_finished => 1, -# ); +execute( + check_finished => 1, + ); ok(1); 1; diff --git a/test_regress/t/t_class_new.v b/test_regress/t/t_class_new.v index 27a806465..e370e2e49 100644 --- a/test_regress/t/t_class_new.v +++ b/test_regress/t/t_class_new.v @@ -16,16 +16,22 @@ class ClsArg; function new(int i); imembera = i + 1; endfunction + function int geta; + return imembera; + endfunction endclass module t (/*AUTOARG*/); initial begin ClsNoArg c1; ClsArg c2; + c1 = new; if (c1.imembera != 5) $stop; + c2 = new(2); if (c2.imembera != 3) $stop; + if (c2.geta() != 3) $stop; $write("*-* All Finished *-*\n"); $finish; diff --git a/test_regress/t/t_class_new_bad.out b/test_regress/t/t_class_new_bad.out index 293a4cd3f..ac1b98bf5 100644 --- a/test_regress/t/t_class_new_bad.out +++ b/test_regress/t/t_class_new_bad.out @@ -1,13 +1,12 @@ -%Error: t/t_class_new_bad.v:10:13: Unsupported: new constructor - : ... In instance t - 10 | function new(); - | ^~~ -%Error: t/t_class_new_bad.v:17:13: Unsupported: new constructor - : ... In instance t - 17 | function new(int i); - | ^~~ -%Error: t/t_class_new_bad.v:26:12: Unsupported: new with arguments - : ... In instance t - 26 | c1 = new(3); +%Error: t/t_class_new_bad.v:31:16: Too many arguments in function call to FUNC 'new' + 31 | c1 = new(3); + | ^ +%Error: t/t_class_new_bad.v:32:16: Too many arguments in function call to FUNC 'new' + 32 | c2 = new(3); + | ^ +%Error: t/t_class_new_bad.v:33:12: Missing argument on non-defaulted argument 'i' in function call to FUNC 'new' + 33 | c3 = new(); + | ^~~ +%Error: Internal Error: t/t_class_new_bad.v:33:12: ../V3Broken.cpp:#: Width != WidthMin + 33 | c3 = new(); | ^~~ -%Error: Exiting due to diff --git a/test_regress/t/t_class_new_bad.v b/test_regress/t/t_class_new_bad.v index 4e721bd61..5029ff0fd 100644 --- a/test_regress/t/t_class_new_bad.v +++ b/test_regress/t/t_class_new_bad.v @@ -12,6 +12,10 @@ class ClsNoArg; endfunction endclass +class ClsNoNew; + int imembera; +endclass + class ClsArg; int imembera; function new(int i); @@ -22,9 +26,11 @@ endclass module t (/*AUTOARG*/); initial begin ClsNoArg c1; - ClsArg c2; + ClsNoNew c2; + ClsArg c3; c1 = new(3); // Bad, called with arg - c2 = new(); // Bad, called without arg + c2 = new(3); // Bad, called with arg + c3 = new(); // Bad, called without arg $stop; end endmodule