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 <wsnyder@wsnyder.org>

src/V3Split.cpp

@@ -791,21 +791,55 @@ private:
 };
 
 class RemovePlaceholdersVisitor final : public VNVisitor {
-    std::unordered_set<AstNode*> m_removeSet;  // placeholders to be removed
+    // MEMBERS
-public:
+    bool m_isPure = true;
-    explicit RemovePlaceholdersVisitor(AstNode* nodep) {
+    int m_emptyAlways = 0;
-        iterate(nodep);
+
-        for (AstNode* np : m_removeSet) {
+    // CONSTRUCTORS
-            np->unlinkFrBack();  // Without next
+    RemovePlaceholdersVisitor() = default;
-            VL_DO_DANGLING(np->deleteTree(), np);
+    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(AstNode* nodep) override {
-    virtual void visit(AstSplitPlaceholder* nodep) override { m_removeSet.insert(nodep); }
+        m_isPure &= nodep->isPure();
-    virtual void visit(AstNode* nodep) override { iterateChildren(nodep); }
+        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:

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;
+   function logic[15:0]  sideeffect_func(logic [15:0] v);
-   always @ (posedge clk) begin
+      /*verilator no_inline_task */
-      l_split_2 <= l_split_1;
+      $display(" sideeffect_func() is called %t", $time);
-      l_split_1 <= l_split_2 | m_din;
+      return ~v;
-   end
+   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

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(

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

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(

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(