From 3af5e7e8da96984f7041a6d8026ef1673cf8eac8 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Wed, 25 May 2022 20:16:19 +0100 Subject: [PATCH] Remove scope pointer from OrderEitherVertex. For ordering, only the scope of logic vertices should be relevant, so remove the scope pointer from OrderEitherVertex and move it into OrderLogicVertex. This does not change single-threaded scheduling at all. Theoretically, multi-threaded scheduling should not be affected either though due to some implementation quirk depending on vertex order in a graph the MT schedule is perturbed by this change, but the performance effect of this is negligible on all benchmarks I have access to. No functional change intended. Fixes #3442 --- src/V3Order.cpp | 42 ++++++++++++++++------------------------- src/V3OrderGraph.h | 47 ++++++++++++++++++++-------------------------- 2 files changed, 36 insertions(+), 53 deletions(-) diff --git a/src/V3Order.cpp b/src/V3Order.cpp index c1d49dfbf..291cc1a68 100644 --- a/src/V3Order.cpp +++ b/src/V3Order.cpp @@ -138,24 +138,15 @@ private: public: // METHODS - OrderVarVertex* getVarVertex(V3Graph* graphp, AstScope* scopep, AstVarScope* varscp, - VarVertexType type) { + OrderVarVertex* getVarVertex(V3Graph* graphp, AstVarScope* varscp, VarVertexType type) { const unsigned idx = static_cast(type); OrderVarVertex* vertexp = m_vertexps[idx]; if (!vertexp) { switch (type) { - case VarVertexType::STD: - vertexp = new OrderVarStdVertex(graphp, scopep, varscp); - break; - case VarVertexType::PRE: - vertexp = new OrderVarPreVertex(graphp, scopep, varscp); - break; - case VarVertexType::PORD: - vertexp = new OrderVarPordVertex(graphp, scopep, varscp); - break; - case VarVertexType::POST: - vertexp = new OrderVarPostVertex(graphp, scopep, varscp); - break; + case VarVertexType::STD: vertexp = new OrderVarStdVertex{graphp, varscp}; break; + case VarVertexType::PRE: vertexp = new OrderVarPreVertex{graphp, varscp}; break; + case VarVertexType::PORD: vertexp = new OrderVarPordVertex{graphp, varscp}; break; + case VarVertexType::POST: vertexp = new OrderVarPostVertex{graphp, varscp}; break; } m_vertexps[idx] = vertexp; } @@ -228,7 +219,7 @@ class OrderBuildVisitor final : public VNVisitor { } OrderVarVertex* getVarVertex(AstVarScope* varscp, VarVertexType type) { - return m_orderUser(varscp).getVarVertex(m_graphp, m_scopep, varscp, type); + return m_orderUser(varscp).getVarVertex(m_graphp, varscp, type); } // VISITORS @@ -631,9 +622,9 @@ public: // Clients of ProcessMoveBuildGraph must supply MoveVertexMaker // which creates new T_MoveVertex's. Each new vertex wraps lvertexp // (which may be nullptr.) - virtual T_MoveVertex* makeVertexp( // - OrderLogicVertex* lvertexp, const OrderEitherVertex* varVertexp, - const AstScope* scopep, const AstSenTree* domainp) + virtual T_MoveVertex* makeVertexp(OrderLogicVertex* lvertexp, + const OrderEitherVertex* varVertexp, + const AstSenTree* domainp) = 0; virtual void freeVertexp(T_MoveVertex* freeMep) = 0; }; @@ -680,8 +671,8 @@ public: // For each logic node, make a T_MoveVertex for (V3GraphVertex* itp = m_graphp->verticesBeginp(); itp; itp = itp->verticesNextp()) { if (OrderLogicVertex* const lvertexp = dynamic_cast(itp)) { - T_MoveVertex* const moveVxp = m_vxMakerp->makeVertexp( - lvertexp, nullptr, lvertexp->scopep(), lvertexp->domainp()); + T_MoveVertex* const moveVxp + = m_vxMakerp->makeVertexp(lvertexp, nullptr, lvertexp->domainp()); if (moveVxp) { // Cross link so we can find it later m_logic2move[lvertexp] = moveVxp; @@ -782,10 +773,10 @@ private: const V3GraphVertex* nonLogicVxp = edgep->top(); const VxDomPair key(nonLogicVxp, domainp); if (!m_var2move[key]) { - const OrderEitherVertex* const eithp - = dynamic_cast(nonLogicVxp); + const OrderVarVertex* const eithp + = static_cast(nonLogicVxp); T_MoveVertex* const newMoveVxp - = m_vxMakerp->makeVertexp(nullptr, eithp, eithp->scopep(), domainp); + = m_vxMakerp->makeVertexp(nullptr, eithp, domainp); m_var2move[key] = newMoveVxp; // Find downstream logics that depend on (var, domain) @@ -824,9 +815,9 @@ public: , m_pomWaitingp{pomWaitingp} {} // METHODS virtual OrderMoveVertex* makeVertexp(OrderLogicVertex* lvertexp, const OrderEitherVertex*, - const AstScope* scopep, const AstSenTree* domainp) override { OrderMoveVertex* const resultp = new OrderMoveVertex(m_pomGraphp, lvertexp); + AstScope* const scopep = lvertexp ? lvertexp->scopep() : nullptr; resultp->domScopep(OrderMoveDomScope::findCreate(domainp, scopep)); resultp->m_pomWaitingE.pushBack(*m_pomWaitingp, resultp); return resultp; @@ -849,9 +840,8 @@ public: : m_pomGraphp{pomGraphp} {} virtual MTaskMoveVertex* makeVertexp(OrderLogicVertex* lvertexp, const OrderEitherVertex* varVertexp, - const AstScope* scopep, const AstSenTree* domainp) override { - return new MTaskMoveVertex(m_pomGraphp, lvertexp, varVertexp, scopep, domainp); + return new MTaskMoveVertex(m_pomGraphp, lvertexp, varVertexp, domainp); } virtual void freeVertexp(MTaskMoveVertex* freeMep) override { freeMep->unlinkDelete(m_pomGraphp); diff --git a/src/V3OrderGraph.h b/src/V3OrderGraph.h index 33a22284d..070e38c54 100644 --- a/src/V3OrderGraph.h +++ b/src/V3OrderGraph.h @@ -72,17 +72,13 @@ public: // Vertex types class OrderEitherVertex VL_NOT_FINAL : public V3GraphVertex { - AstScope* const m_scopep; // Scope the vertex is in AstSenTree* m_domainp; // Clock domain (nullptr = to be computed as we iterate) protected: // CONSTRUCTOR - OrderEitherVertex(V3Graph* graphp, AstScope* scopep, AstSenTree* domainp) + OrderEitherVertex(V3Graph* graphp, AstSenTree* domainp) : V3GraphVertex{graphp} - , m_scopep{scopep} - , m_domainp{domainp} { - UASSERT(scopep, "Must not be null"); - } + , m_domainp{domainp} {} virtual ~OrderEitherVertex() override = default; public: @@ -92,24 +88,22 @@ public: // ACCESSORS AstSenTree* domainp() const { return m_domainp; } void domainp(AstSenTree* domainp) { m_domainp = domainp; } - AstScope* scopep() const { return m_scopep; } - - // LCOV_EXCL_START // Debug code - virtual string dotName() const override { return cvtToHex(m_scopep) + "_"; } - // LCOV_EXCL_STOP }; class OrderLogicVertex final : public OrderEitherVertex { - AstNode* const m_nodep; + AstNode* const m_nodep; // The logic this vertex represents + AstScope* const m_scopep; // Scope the logic is under AstSenTree* const m_hybridp; public: // CONSTRUCTOR OrderLogicVertex(V3Graph* graphp, AstScope* scopep, AstSenTree* domainp, AstSenTree* hybridp, AstNode* nodep) - : OrderEitherVertex{graphp, scopep, domainp} + : OrderEitherVertex{graphp, domainp} , m_nodep{nodep} + , m_scopep{scopep} , m_hybridp{hybridp} { + UASSERT_OBJ(scopep, nodep, "Must not be null"); UASSERT_OBJ(!(domainp && hybridp), nodep, "Cannot have bot domainp and hybridp set"); } virtual ~OrderLogicVertex() override = default; @@ -119,6 +113,7 @@ public: // ACCESSORS AstNode* nodep() const { return m_nodep; } + AstScope* scopep() const { return m_scopep; } AstSenTree* hybridp() const { return m_hybridp; } // LCOV_EXCL_START // Debug code @@ -136,8 +131,8 @@ class OrderVarVertex VL_NOT_FINAL : public OrderEitherVertex { public: // CONSTRUCTOR - OrderVarVertex(V3Graph* graphp, AstScope* scopep, AstVarScope* vscp) - : OrderEitherVertex{graphp, scopep, nullptr} + OrderVarVertex(V3Graph* graphp, AstVarScope* vscp) + : OrderEitherVertex{graphp, nullptr} , m_vscp{vscp} {} virtual ~OrderVarVertex() override = default; @@ -156,8 +151,8 @@ public: class OrderVarStdVertex final : public OrderVarVertex { public: // CONSTRUCTOR - OrderVarStdVertex(V3Graph* graphp, AstScope* scopep, AstVarScope* varScp) - : OrderVarVertex{graphp, scopep, varScp} {} + OrderVarStdVertex(V3Graph* graphp, AstVarScope* varScp) + : OrderVarVertex{graphp, varScp} {} virtual ~OrderVarStdVertex() override = default; // METHODS @@ -172,8 +167,8 @@ public: class OrderVarPreVertex final : public OrderVarVertex { public: // CONSTRUCTOR - OrderVarPreVertex(V3Graph* graphp, AstScope* scopep, AstVarScope* varScp) - : OrderVarVertex{graphp, scopep, varScp} {} + OrderVarPreVertex(V3Graph* graphp, AstVarScope* varScp) + : OrderVarVertex{graphp, varScp} {} virtual ~OrderVarPreVertex() override = default; // METHODS @@ -188,8 +183,8 @@ public: class OrderVarPostVertex final : public OrderVarVertex { public: // CONSTRUCTOR - OrderVarPostVertex(V3Graph* graphp, AstScope* scopep, AstVarScope* varScp) - : OrderVarVertex{graphp, scopep, varScp} {} + OrderVarPostVertex(V3Graph* graphp, AstVarScope* varScp) + : OrderVarVertex{graphp, varScp} {} virtual ~OrderVarPostVertex() override = default; // METHODS @@ -204,8 +199,8 @@ public: class OrderVarPordVertex final : public OrderVarVertex { public: // CONSTRUCTOR - OrderVarPordVertex(V3Graph* graphp, AstScope* scopep, AstVarScope* varScp) - : OrderVarVertex{graphp, scopep, varScp} {} + OrderVarPordVertex(V3Graph* graphp, AstVarScope* varScp) + : OrderVarVertex{graphp, varScp} {} virtual ~OrderVarPordVertex() override = default; // METHODS @@ -325,16 +320,14 @@ class MTaskMoveVertex final : public V3GraphVertex { // or a var node, it can't be both. OrderLogicVertex* const m_logicp; // Logic represented by this vertex const OrderEitherVertex* const m_varp; // Var represented by this vertex - const AstScope* const m_scopep; const AstSenTree* const m_domainp; public: MTaskMoveVertex(V3Graph* graphp, OrderLogicVertex* logicp, const OrderEitherVertex* varp, - const AstScope* scopep, const AstSenTree* domainp) + const AstSenTree* domainp) : V3GraphVertex{graphp} , m_logicp{logicp} , m_varp{varp} - , m_scopep{scopep} , m_domainp{domainp} { UASSERT(!(logicp && varp), "MTaskMoveVertex: logicp and varp may not both be set!\n"); } @@ -343,7 +336,7 @@ public: // ACCESSORS OrderLogicVertex* logicp() const { return m_logicp; } const OrderEitherVertex* varp() const { return m_varp; } - const AstScope* scopep() const { return m_scopep; } + const AstScope* scopep() const { return m_logicp ? m_logicp->scopep() : nullptr; } const AstSenTree* domainp() const { return m_domainp; } virtual string dotColor() const override {