From df471e87eb115a477602c8eaadf83de537176ba7 Mon Sep 17 00:00:00 2001 From: John Coiner Date: Sat, 28 Oct 2017 13:31:04 -0400 Subject: [PATCH] Internals: Break unnecessary dependencies in V3Order. Signed-off-by: Wilson Snyder --- src/V3Ast.h | 27 ++++++- src/V3Order.cpp | 80 ++++++++++++++++--- test_regress/t/t_case_huge.pl | 2 +- test_regress/t/t_inst_tree_inl0_pub1.pl | 2 +- .../t/t_inst_tree_inl0_pub1_norelcfuncs.pl | 2 +- 5 files changed, 96 insertions(+), 17 deletions(-) diff --git a/src/V3Ast.h b/src/V3Ast.h index 871a8c9c2..32dbb5332 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -208,14 +208,37 @@ public: "COMBO","INITIAL","SETTLE","NEVER" }; return names[m_e]; - }; + } const char* verilogKwd() const { static const char* const names[] = { "%E-edge", "[any]", "edge", "posedge", "negedge", "[high]","[low]", "*","[initial]","[settle]","[never]" }; return names[m_e]; - }; + } + // Return true iff this and the other have mutually exclusive transitions + bool exclusiveEdge(const AstEdgeType& other) const { + switch (m_e) { + case AstEdgeType::ET_POSEDGE: + switch (other.m_e) { + case AstEdgeType::ET_NEGEDGE: // FALLTHRU + case AstEdgeType::ET_LOWEDGE: + return true; + default: {} + } + break; + case AstEdgeType::ET_NEGEDGE: + switch (other.m_e) { + case AstEdgeType::ET_POSEDGE: // FALLTHRU + case AstEdgeType::ET_HIGHEDGE: + return true; + default: {} + } + break; + default: {} + } + return false; + } inline AstEdgeType () : m_e(ET_ILLEGAL) {} // cppcheck-suppress noExplicitConstructor inline AstEdgeType (en _e) : m_e(_e) {} diff --git a/src/V3Order.cpp b/src/V3Order.cpp index a3aa7d98d..5c3308e22 100644 --- a/src/V3Order.cpp +++ b/src/V3Order.cpp @@ -562,6 +562,7 @@ private: void processMoveReadyOne(OrderMoveVertex* vertexp); void processMoveDoneOne(OrderMoveVertex* vertexp); void processMoveOne(OrderMoveVertex* vertexp, OrderMoveDomScope* domScopep, int level); + static bool domainsExclusive(const AstSenTree* fromp, const AstSenTree* top); string cfuncName(AstNodeModule* modp, AstSenTree* domainp, AstScope* scopep, AstNode* forWhatp) { modp->user3Inc(); @@ -1350,21 +1351,76 @@ void OrderVisitor::processMoveBuildGraph() { } } +bool OrderVisitor::domainsExclusive(const AstSenTree* fromp, + const AstSenTree* top) { + // Return 'true' if we can prove that both 'from' and 'to' cannot both + // be active on the same eval pass, or false if we can't prove this. + // + // For now, this only detects the case of 'always @(posedge clk)' + // and 'always @(negedge clk)' being exclusive. + // + // Are there any other cases we need to handle? Maybe not, + // because these are not exclusive: + // always @(posedge A or posedge B) + // always @(negedge A) + // + // ... unless you know more about A and B, which sounds hard. + const AstSenItem* fromSenListp = fromp->sensesp()->castSenItem(); + const AstSenItem* toSenListp = top->sensesp()->castSenItem(); + // If clk gating is ever reenabled, we may need to update this to handle + // AstSenGate also. + if (!fromSenListp) fromp->v3fatalSrc("sensitivity list item is not an AstSenItem"); + if (!toSenListp) top->v3fatalSrc("sensitivity list item is not an AstSenItem"); + + if (fromSenListp->nextp()) return false; + if (toSenListp->nextp()) return false; + + const AstNodeVarRef* fromVarrefp = fromSenListp->varrefp(); + const AstNodeVarRef* toVarrefp = toSenListp->varrefp(); + if (!fromVarrefp || !toVarrefp) return false; + + // We know nothing about the relationship between different clocks here, + // so give up on proving anything. + if (fromVarrefp->varScopep() != toVarrefp->varScopep()) return false; + + return fromSenListp->edgeType().exclusiveEdge(toSenListp->edgeType()); +} + void OrderVisitor::processMoveBuildGraphIterate (OrderMoveVertex* moveVxp, V3GraphVertex* vertexp, int weightmin) { // Search forward from given logic vertex, making new edges based on moveVxp for (V3GraphEdge* edgep = vertexp->outBeginp(); edgep; edgep=edgep->outNextp()) { - if (edgep->weight()!=0) { // was cut - int weight = weightmin; - if (weight==0 || weight>edgep->weight()) weight=edgep->weight(); - if (OrderLogicVertex* toLVertexp = dynamic_cast(edgep->top())) { - // Path from vertexp to a logic vertex; new edge - // Note we use the last edge's weight, not some function of multiple edges - new OrderEdge(&m_pomGraph, moveVxp, toLVertexp->moveVxp(), weight); - } - else { // Keep hunting forward for a logic node - processMoveBuildGraphIterate(moveVxp, edgep->top(), weight); - } - } + if (edgep->weight()!=0) { // was cut + int weight = weightmin; + if (weight==0 || weight>edgep->weight()) weight=edgep->weight(); + if (OrderLogicVertex* toLVertexp = dynamic_cast(edgep->top())) { + // "Initial" and "settle" domains run at time 0 and never + // again. Everything else runs at time >0 and never before. + // Ignore deps that cross the time-zero/nonzero boundary. + // We'll never run both vertices in the same eval() pass + // so there's no need to order them. + bool toInitial = (toLVertexp->domainp()->hasInitial() + || toLVertexp->domainp()->hasSettle()); + bool fromInitial = (moveVxp->logicp()->domainp()->hasInitial() + || moveVxp->logicp()->domainp()->hasSettle()); + if (toInitial != fromInitial) { + continue; + } + // Sometimes we get deps between 'always @(posedge clk)' and + // 'always @(negedge clk)' domains. Discard these. We'll never + // evaluate both logic vertices on the same eval() pass so + // there's no need to order them. + if (domainsExclusive(moveVxp->logicp()->domainp(), + toLVertexp->domainp())) { + continue; + } + // Path from vertexp to a logic vertex; new edge + // Note we use the last edge's weight, not some function of multiple edges + new OrderEdge(&m_pomGraph, moveVxp, toLVertexp->moveVxp(), weight); + } + else { // Keep hunting forward for a logic node + processMoveBuildGraphIterate(moveVxp, edgep->top(), weight); + } + } } } diff --git a/test_regress/t/t_case_huge.pl b/test_regress/t/t_case_huge.pl index e29a6c00c..05c89e922 100755 --- a/test_regress/t/t_case_huge.pl +++ b/test_regress/t/t_case_huge.pl @@ -13,7 +13,7 @@ compile ( if ($Self->{vlt}) { file_grep ($Self->{stats}, qr/Optimizations, Tables created\s+(\d+)/i, 10); - file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 9); + file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 8); } execute ( diff --git a/test_regress/t/t_inst_tree_inl0_pub1.pl b/test_regress/t/t_inst_tree_inl0_pub1.pl index 9dcd55750..c5593b090 100755 --- a/test_regress/t/t_inst_tree_inl0_pub1.pl +++ b/test_regress/t/t_inst_tree_inl0_pub1.pl @@ -36,7 +36,7 @@ sub checkRelativeRefs { if ($Self->{vlt}) { # We expect to combine sequent functions across multiple instances of # l2, l3, l4, l5. If this number drops, please confirm this has not broken. - file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 59); + file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 52); # Expect absolute refs in CFuncs for t (top module) and l1 (because it # has only one instance) diff --git a/test_regress/t/t_inst_tree_inl0_pub1_norelcfuncs.pl b/test_regress/t/t_inst_tree_inl0_pub1_norelcfuncs.pl index ac45684ea..4eec915b0 100755 --- a/test_regress/t/t_inst_tree_inl0_pub1_norelcfuncs.pl +++ b/test_regress/t/t_inst_tree_inl0_pub1_norelcfuncs.pl @@ -16,7 +16,7 @@ compile ( if ($Self->{vlt}) { # Fewer optimizations than t_inst_tree_inl0_pub1 which allows # relative CFuncs: - file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 16); + file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 31); # Should not find any 'this->' except some 'this->__VlSymsp' my @files = `ls $Self->{obj_dir}/*.cpp`;