From 999751c422e19b49a4df3c8bf262e4002386631a Mon Sep 17 00:00:00 2001 From: Yutetsu TAKATSUKASA Date: Sun, 6 Mar 2022 12:56:34 +0900 Subject: [PATCH] Count non-empty always blocks in V3Split (#3337) "Optimizations, Split always" in stats now means the number of newly added always. Co-authored-by: Wilson Snyder --- src/V3Split.cpp | 61 ++++++++++++++++++++++++------- test_regress/t/t_alw_nosplit.v | 23 +++++++++--- test_regress/t/t_alw_split.pl | 2 +- test_regress/t/t_alw_split.v | 6 +++ test_regress/t/t_alw_split_rst.pl | 2 +- test_regress/t/t_alw_splitord.pl | 2 +- 6 files changed, 75 insertions(+), 21 deletions(-) diff --git a/src/V3Split.cpp b/src/V3Split.cpp index fc9c75ea0..bd3355fb5 100644 --- a/src/V3Split.cpp +++ b/src/V3Split.cpp @@ -791,21 +791,55 @@ private: }; class RemovePlaceholdersVisitor final : public VNVisitor { - std::unordered_set m_removeSet; // placeholders to be removed -public: - explicit RemovePlaceholdersVisitor(AstNode* nodep) { - iterate(nodep); - for (AstNode* np : m_removeSet) { - np->unlinkFrBack(); // Without next - VL_DO_DANGLING(np->deleteTree(), np); + // MEMBERS + bool m_isPure = true; + int m_emptyAlways = 0; + + // CONSTRUCTORS + RemovePlaceholdersVisitor() = default; + virtual ~RemovePlaceholdersVisitor() override = default; + + // VISITORS + virtual void visit(AstSplitPlaceholder* nodep) override { pushDeletep(nodep->unlinkFrBack()); } + virtual void visit(AstNodeIf* nodep) override { + VL_RESTORER(m_isPure); + m_isPure = true; + iterateChildren(nodep); + if (!nodep->ifsp() && !nodep->elsesp() && m_isPure) pushDeletep(nodep->unlinkFrBack()); + } + virtual void visit(AstAlways* nodep) override { + VL_RESTORER(m_isPure); + m_isPure = true; + iterateChildren(nodep); + if (m_isPure) { + bool emptyOrCommentOnly = true; + for (AstNode* bodysp = nodep->bodysp(); bodysp; bodysp = bodysp->nextp()) { + // If this always block contains only AstComment, remove here. + // V3Gate will remove anyway. + if (!VN_IS(bodysp, Comment)) { + emptyOrCommentOnly = false; + break; + } + } + if (emptyOrCommentOnly) { + pushDeletep(nodep->unlinkFrBack()); + ++m_emptyAlways; + } } } - virtual ~RemovePlaceholdersVisitor() override = default; - virtual void visit(AstSplitPlaceholder* nodep) override { m_removeSet.insert(nodep); } - virtual void visit(AstNode* nodep) override { iterateChildren(nodep); } + virtual void visit(AstNode* nodep) override { + m_isPure &= nodep->isPure(); + iterateChildren(nodep); // must visit regardless of m_isPure to remove placeholders + } -private: VL_UNCOPYABLE(RemovePlaceholdersVisitor); + +public: + static int exec(AstAlways* nodep) { + RemovePlaceholdersVisitor visitor; + visitor.iterate(nodep); + return visitor.m_emptyAlways; + } }; class SplitVisitor final : public SplitReorderBaseVisitor { @@ -832,7 +866,8 @@ public: for (AlwaysVec::iterator addme = it->second.begin(); addme != it->second.end(); ++addme) { origp->addNextHere(*addme); - RemovePlaceholdersVisitor{*addme}; + const int numRemoved = RemovePlaceholdersVisitor::exec(*addme); + m_statSplits -= numRemoved; } origp->unlinkFrBack(); // Without next VL_DO_DANGLING(origp->deleteTree(), origp); @@ -944,7 +979,7 @@ protected: // Counting original always blocks rather than newly-split // always blocks makes it a little easier to use this stat to // check the result of the t_alw_split test: - ++m_statSplits; + m_statSplits += ifColor.colors().size() - 1; // -1 for the original always // Visit through the original always block one more time, // and emit the split always blocks into m_replaceBlocks: diff --git a/test_regress/t/t_alw_nosplit.v b/test_regress/t/t_alw_nosplit.v index 53a29461e..2bfb37b6e 100644 --- a/test_regress/t/t_alw_nosplit.v +++ b/test_regress/t/t_alw_nosplit.v @@ -47,11 +47,21 @@ module t (/*AUTOARG*/ f_split_1 = m_din; end - reg [15:0] l_split_1, l_split_2; - always @ (posedge clk) begin - l_split_2 <= l_split_1; - l_split_1 <= l_split_2 | m_din; - end + function logic[15:0] sideeffect_func(logic [15:0] v); + /*verilator no_inline_task */ + $display(" sideeffect_func() is called %t", $time); + return ~v; + endfunction + reg [15:0] m_split_1 = 0; + reg [15:0] m_split_2 = 0; + always @(posedge clk) begin + if (sideeffect_func(m_split_1) != 16'b0) begin + m_split_1 <= m_din; + end else begin + m_split_2 <= m_din; + end + end + reg [15:0] z_split_1, z_split_2; always @ (posedge clk) begin @@ -104,6 +114,7 @@ module t (/*AUTOARG*/ if (!(c_split_1==16'h0112 && c_split_2==16'hfeed)) $stop; if (!(e_split_1==16'hfeed && e_split_2==16'hfeed)) $stop; if (!(f_split_1==16'hfeed && f_split_2==16'hfeed)) $stop; + if (!(m_split_1==16'hfeed && m_split_2==16'h0000)) $stop; if (!(z_split_1==16'h0112 && z_split_2==16'h0112)) $stop; end if (cyc==5) begin @@ -113,6 +124,7 @@ module t (/*AUTOARG*/ // Two valid orderings, as we don't know which posedge clk gets evaled first if (!(e_split_1==16'hfeed && e_split_2==16'hfeed) && !(e_split_1==16'he11e && e_split_2==16'he11e)) $stop; if (!(f_split_1==16'hfeed && f_split_2==16'hfeed) && !(f_split_1==16'he11e && f_split_2==16'hfeed)) $stop; + if (!(m_split_1==16'hfeed && m_split_2==16'h0000)) $stop; if (!(z_split_1==16'h0112 && z_split_2==16'h0112)) $stop; end if (cyc==6) begin @@ -122,6 +134,7 @@ module t (/*AUTOARG*/ // Two valid orderings, as we don't know which posedge clk gets evaled first if (!(e_split_1==16'he11e && e_split_2==16'he11e) && !(e_split_1==16'he22e && e_split_2==16'he22e)) $stop; if (!(f_split_1==16'he11e && f_split_2==16'hfeed) && !(f_split_1==16'he22e && f_split_2==16'he11e)) $stop; + if (!(m_split_1==16'he11e && m_split_2==16'h0000)) $stop; if (!(z_split_1==16'h1ee1 && z_split_2==16'h0112)) $stop; end if (cyc==7) begin diff --git a/test_regress/t/t_alw_split.pl b/test_regress/t/t_alw_split.pl index ba9c8f933..7b60fb6fc 100755 --- a/test_regress/t/t_alw_split.pl +++ b/test_regress/t/t_alw_split.pl @@ -15,7 +15,7 @@ compile( ); if ($Self->{vlt_all}) { - file_grep($Self->{stats}, qr/Optimizations, Split always\s+(\d+)/i, 3); + file_grep($Self->{stats}, qr/Optimizations, Split always\s+(\d+)/i, 4); } execute( diff --git a/test_regress/t/t_alw_split.v b/test_regress/t/t_alw_split.v index 8d5419ac1..b6b1e7efa 100644 --- a/test_regress/t/t_alw_split.v +++ b/test_regress/t/t_alw_split.v @@ -50,6 +50,12 @@ module t (/*AUTOARG*/ end end + reg [15:0] l_split_1, l_split_2; + always @ (posedge clk) begin + l_split_2 <= l_split_1; + l_split_1 <= l_split_2 | m_din; + end + // (The checker block is an exception, it won't split.) always @ (posedge clk) begin if (cyc!=0) begin diff --git a/test_regress/t/t_alw_split_rst.pl b/test_regress/t/t_alw_split_rst.pl index 87d186513..fe393b88e 100755 --- a/test_regress/t/t_alw_split_rst.pl +++ b/test_regress/t/t_alw_split_rst.pl @@ -16,7 +16,7 @@ compile( ); if ($Self->{vlt_all}) { - file_grep($Self->{stats}, qr/Optimizations, Split always\s+(\d+)/i, 0); + file_grep($Self->{stats}, qr/Optimizations, Split always\s+(\d+)/i, 12); } execute( diff --git a/test_regress/t/t_alw_splitord.pl b/test_regress/t/t_alw_splitord.pl index a1d66aaad..972bb3672 100755 --- a/test_regress/t/t_alw_splitord.pl +++ b/test_regress/t/t_alw_splitord.pl @@ -15,7 +15,7 @@ compile( ); if ($Self->{vlt_all}) { - file_grep($Self->{stats}, qr/Optimizations, Split always\s+(\d+)/i, 0); + file_grep($Self->{stats}, qr/Optimizations, Split always\s+(\d+)/i, 5); } execute(