diff --git a/src/V3Begin.cpp b/src/V3Begin.cpp index 9c1e3674a..5bee0a7f4 100644 --- a/src/V3Begin.cpp +++ b/src/V3Begin.cpp @@ -62,6 +62,7 @@ private: BeginState* m_statep; // Current global state AstNodeModule* m_modp = nullptr; // Current module AstNodeFTask* m_ftaskp = nullptr; // Current function/task + AstNode* m_liftedp = nullptr; // Local nodes we are lifting into m_ftaskp string m_namedScope; // Name of begin blocks above us string m_unnamedScope; // Name of begin blocks, including unnamed blocks int m_ifDepth = 0; // Current if depth @@ -74,6 +75,21 @@ private: return a + "__DOT__" + b; } + void liftNode(AstNode* nodep) { + nodep->unlinkFrBack(); + if (m_ftaskp) { + // AstBegin under ftask, just move into the ftask + if (!m_liftedp) { + m_liftedp = nodep; + } else { + m_liftedp->addNext(nodep); + } + } else { + // Move to module + m_modp->addStmtp(nodep); + } + } + // VISITORS virtual void visit(AstNodeModule* nodep) override { VL_RESTORER(m_modp); @@ -101,7 +117,17 @@ private: m_namedScope = ""; m_unnamedScope = ""; m_ftaskp = nodep; + m_liftedp = nullptr; iterateChildren(nodep); + if (m_liftedp) { + // Place lifted nodes at beginning of stmtsp, so Var nodes appear before referenced + if (AstNode* const stmtsp = nodep->stmtsp()) { + stmtsp->unlinkFrBackWithNext(); + m_liftedp->addNext(stmtsp); + } + nodep->addStmtsp(m_liftedp); + m_liftedp = nullptr; + } m_ftaskp = nullptr; } } @@ -157,13 +183,8 @@ private: // Rename it nodep->name(dot(m_unnamedScope, nodep->name())); m_statep->userMarkChanged(nodep); - // Move to module - nodep->unlinkFrBack(); - if (m_ftaskp) { - m_ftaskp->addStmtsp(nodep); // Begins under funcs just move into the func - } else { - m_modp->addStmtp(nodep); - } + // Move it under enclosing tree + liftNode(nodep); } } virtual void visit(AstTypedef* nodep) override { @@ -171,14 +192,8 @@ private: // Rename it nodep->name(dot(m_unnamedScope, nodep->name())); m_statep->userMarkChanged(nodep); - // Move to module - nodep->unlinkFrBack(); - // Begins under funcs just move into the func - if (m_ftaskp) { - m_ftaskp->addStmtsp(nodep); - } else { - m_modp->addStmtp(nodep); - } + // Move it under enclosing tree + liftNode(nodep); } } virtual void visit(AstCell* nodep) override { diff --git a/src/V3Broken.cpp b/src/V3Broken.cpp index 3205ce912..b9bae7707 100644 --- a/src/V3Broken.cpp +++ b/src/V3Broken.cpp @@ -18,6 +18,7 @@ // Entire netlist // Mark all nodes // Check all links point to marked nodes +// Check local variables in CFuncs appear before they are referenced // //************************************************************************* @@ -33,6 +34,7 @@ #include #include +#include //###################################################################### @@ -240,13 +242,23 @@ public: class BrokenCheckVisitor final : public AstNVisitor { bool m_inScope = false; // Under AstScope + // Current CFunc, if any + const AstCFunc* m_cfuncp = nullptr; + // All local variables declared in current function + std::unordered_set m_localVars; + // Variable references in current function that do not reference an in-scope local + std::unordered_map m_suspectRefs; + // Local variables declared in the scope of the current statement + std::vector> m_localsStack; + private: static void checkWidthMin(const AstNode* nodep) { UASSERT_OBJ(nodep->width() == nodep->widthMin() || v3Global.widthMinUsage() != VWidthMinUsage::MATCHES_WIDTH, nodep, "Width != WidthMin"); } - void processAndIterate(AstNode* nodep) { + + void processEnter(AstNode* nodep) { BrokenTable::setUnder(nodep, true); const char* whyp = nodep->broken(); UASSERT_OBJ(!whyp, nodep, @@ -270,8 +282,30 @@ private: if (const AstNodeDType* dnodep = VN_CAST(nodep, NodeDType)) checkWidthMin(dnodep); } checkWidthMin(nodep); + } + void processExit(AstNode* nodep) { BrokenTable::setUnder(nodep, false); } + void processAndIterate(AstNode* nodep) { + processEnter(nodep); iterateChildrenConst(nodep); - BrokenTable::setUnder(nodep, false); + processExit(nodep); + } + void processAndIterateList(AstNode* nodep) { + while (nodep) { + processAndIterate(nodep); + nodep = nodep->nextp(); + }; + } + void pushLocalScope() { + if (m_cfuncp) m_localsStack.emplace_back(); + } + void popLocalScope() { + if (m_cfuncp) m_localsStack.pop_back(); + } + bool isInScopeLocal(const AstVar* varp) const { + for (const auto& set : m_localsStack) { + if (set.count(varp)) return true; + } + return false; } virtual void visit(AstNodeAssign* nodep) override { processAndIterate(nodep); @@ -295,6 +329,65 @@ private: UASSERT_OBJ( !(v3Global.assertScoped() && m_inScope && nodep->varp() && !nodep->varScopep()), nodep, "VarRef missing VarScope pointer"); + if (m_cfuncp) { + // Check if variable is an in-scope local, otherwise mark as suspect + if (const AstVar* const varp = nodep->varp()) { + if (!isInScopeLocal(varp)) { + // This only stores the first ref for each Var, which is what we want + m_suspectRefs.emplace(varp, nodep); + } + } + } + } + virtual void visit(AstCFunc* nodep) override { + UASSERT_OBJ(!m_cfuncp, nodep, "Nested AstCFunc"); + m_cfuncp = nodep; + m_localVars.clear(); + m_suspectRefs.clear(); + m_localsStack.clear(); + pushLocalScope(); + + processAndIterate(nodep); + + // Check suspect references are all to non-locals + for (const auto& pair : m_suspectRefs) { + UASSERT_OBJ(m_localVars.count(pair.first) == 0, pair.second, + "Local variable not in scope where referenced: " << pair.first); + } + + m_cfuncp = nullptr; + } + virtual void visit(AstNodeIf* nodep) override { + // Each branch is a separate local variable scope + pushLocalScope(); + processEnter(nodep); + processAndIterate(nodep->condp()); + if (AstNode* const ifsp = nodep->ifsp()) { + pushLocalScope(); + processAndIterateList(ifsp); + popLocalScope(); + } + if (AstNode* const elsesp = nodep->elsesp()) { + pushLocalScope(); + processAndIterateList(elsesp); + popLocalScope(); + } + processExit(nodep); + popLocalScope(); + } + virtual void visit(AstNodeStmt* nodep) override { + // For local variable checking act as if any statement introduces a new scope. + // This is aggressive but conservatively correct. + pushLocalScope(); + processAndIterate(nodep); + popLocalScope(); + } + virtual void visit(AstVar* nodep) override { + processAndIterate(nodep); + if (m_cfuncp) { + m_localVars.insert(nodep); + m_localsStack.back().insert(nodep); + } } virtual void visit(AstNode* nodep) override { // Process not just iterate