From 41a563bdc8e219c0fffdfcf1e989a3d30344b73f Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 4 Jan 2022 20:19:58 -0500 Subject: [PATCH] Internal cleanups towards recursive functions (#3267) --- src/V3Hasher.cpp | 16 ++++++++++++---- src/V3Task.cpp | 47 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/V3Hasher.cpp b/src/V3Hasher.cpp index 09dc09e39..22a8181db 100644 --- a/src/V3Hasher.cpp +++ b/src/V3Hasher.cpp @@ -39,6 +39,7 @@ private: V3Hash hashNodeAndIterate(AstNode* nodep, bool hashDType, bool hashChildren, std::function&& f) { + // See comments in visit(AstCFunc) about this breaking recursion if (m_cacheInUser4 && nodep->user4()) { return V3Hash(nodep->user4()); } else { @@ -393,6 +394,12 @@ private: } virtual void visit(AstCFunc* nodep) override { m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [=]() { // + // We might be in a recursive function, if so on *second* call + // here we need to break what would be an infinite loop. + nodep->user4(V3Hash(1).value()); // Set this "first" call + // So that a second call will then exit hashNodeAndIterate + // Having a constant in the hash just means the recursion will + // end, it shouldn't change the CFunc having a unique hash itself. m_hash += nodep->isLoose(); }); } @@ -488,11 +495,12 @@ private: public: // CONSTRUCTORS - explicit HasherVisitor(AstNode* nodep) + HasherVisitor(AstNode* nodep) : m_cacheInUser4{true} { iterate(nodep); } - explicit HasherVisitor(const AstNode* nodep) + class Uncached {}; + HasherVisitor(const AstNode* nodep, Uncached) : m_cacheInUser4{false} { iterate(const_cast(nodep)); } @@ -504,11 +512,11 @@ public: // V3Hasher methods V3Hash V3Hasher::operator()(AstNode* nodep) const { - if (!nodep->user4()) { HasherVisitor{nodep}; } + if (!nodep->user4()) HasherVisitor{nodep}; return V3Hash(nodep->user4()); } V3Hash V3Hasher::uncachedHash(const AstNode* nodep) { - const HasherVisitor visitor{nodep}; + const HasherVisitor visitor{nodep, HasherVisitor::Uncached{}}; return visitor.finalHash(); } diff --git a/src/V3Task.cpp b/src/V3Task.cpp index a2613b51a..b80cd2d44 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -140,7 +140,10 @@ public: getFTaskVertex(nodep)->cFuncp(cfuncp); } void checkPurity(AstNodeFTask* nodep) { checkPurity(nodep, getFTaskVertex(nodep)); } + +private: void checkPurity(AstNodeFTask* nodep, TaskBaseVertex* vxp) { + if (nodep->recursive()) return; // Impure, but no warning if (!vxp->pure()) { nodep->v3warn( IMPURE, "Unsupported: External variable referenced by non-inlined function/task: " @@ -156,8 +159,6 @@ public: checkPurity(nodep, static_cast(edgep->top())); } } - -private: TaskFTaskVertex* getFTaskVertex(AstNodeFTask* nodep) { if (!nodep->user4p()) nodep->user4p(new TaskFTaskVertex(&m_callGraph, nodep)); return static_cast(nodep->user4u().toGraphVertex()); @@ -209,6 +210,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->recursive()) m_curVxp->noInline(true); if (nodep->isConstructor()) { m_curVxp->noInline(true); m_ctorp = nodep; @@ -726,6 +728,24 @@ private: return new AstCStmt(portp->fileline(), stmt); } + void unlinkAndClone(AstNodeFTask* funcp, AstNode* nodep, bool withNext) { + UASSERT_OBJ(nodep, funcp, "null in function object clone"); + VNRelinker relinkHandle; + if (withNext) { + nodep->unlinkFrBackWithNext(&relinkHandle); + } else { + nodep->unlinkFrBack(&relinkHandle); + } + if (funcp->recursive()) { + // Recursive functions require the original argument list to + // still be live for linking purposes. + // The old function gets clone, so that node pointers are mostly + // retained through the V3Task transformations + AstNode* const newp = nodep->cloneTree(withNext); + relinkHandle.relink(newp); + } + } + static AstNode* createAssignInternalToDpi(AstVar* portp, bool isPtr, const string& frSuffix, const string& toSuffix) { const string stmt = V3Task::assignInternalToDpi(portp, isPtr, frSuffix, toSuffix); @@ -1149,7 +1169,7 @@ private: if (ftaskNoInline || nodep->dpiExport()) { portp->funcReturn(false); // Converting return to 'outputs' } - portp->unlinkFrBack(); + unlinkAndClone(nodep, portp, false); rtnvarp = portp; rtnvarp->funcLocal(true); rtnvarp->name(rtnvarp->name() @@ -1260,7 +1280,7 @@ private: } else { if (portp->isIO()) { // Move it to new function - portp->unlinkFrBack(); + unlinkAndClone(nodep, portp, false); portp->funcLocal(true); cfuncp->addArgsp(portp); } else { @@ -1282,10 +1302,21 @@ private: if (rtnvarp) cfuncp->addArgsp(rtnvarp); // Move body - AstNode* const bodysp = nodep->stmtsp(); + AstNode* bodysp = nodep->stmtsp(); if (bodysp) { - bodysp->unlinkFrBackWithNext(); - cfuncp->addStmtsp(bodysp); + unlinkAndClone(nodep, bodysp, true); + AstBegin* const tempp = new AstBegin{nodep->fileline(), "[EditWrapper]", bodysp}; + VL_DANGLING(bodysp); + // If we cloned due to recursion, now need to rip out the ports + // (that remained in place) then got cloned + for (AstNode *nextp, *stmtp = tempp->stmtsp(); stmtp; stmtp = nextp) { + nextp = stmtp->nextp(); + if (AstVar* const portp = VN_CAST(stmtp, Var)) { + if (portp->isIO()) portp->unlinkFrBack(); + } + } + if (tempp->stmtsp()) cfuncp->addStmtsp(tempp->stmtsp()->unlinkFrBackWithNext()); + VL_DO_DANGLING(tempp->deleteTree(), tempp); } if (nodep->dpiImport()) bodyDpiImportFunc(nodep, rtnvscp, cfuncp, dpiFuncp); @@ -1455,7 +1486,7 @@ private: if (visitp) iterateAndNextNull(visitp); } virtual void visit(AstNodeFTask* nodep) override { - UINFO(4, " Inline " << nodep << endl); + UINFO(4, " visitFTask " << nodep << endl); VL_RESTORER(m_insMode); VL_RESTORER(m_insStmtp); m_insMode = IM_BEFORE;