Fix table misoptimizing away display (#3488).

Changes

@@ -14,6 +14,8 @@ Verilator 4.225 devel
 **Minor:**
 
 * Fix incorrect bit op tree optimization (#3470). [algrobman]
+* Fix table misoptimizing away display (#3488). [Stefan Post]
+
 
 Verilator 4.224 2022-06-19
 ==========================

src/V3Simulate.h

@@ -100,6 +100,7 @@ private:
     bool m_anyAssignDly;  ///< True if found a delayed assignment
     bool m_anyAssignComb;  ///< True if found a non-delayed assignment
     bool m_inDlyAssign;  ///< Under delayed assignment
+    bool m_isOutputter;  // Creates output
     int m_instrCount;  ///< Number of nodes
     int m_dataCount;  ///< Bytes of data
     AstJumpGo* m_jumpp;  ///< Jump label we're branching from
@@ -205,6 +206,7 @@ public:
     AstNode* whyNotNodep() const { return m_whyNotNodep; }
 
     bool isAssignDly() const { return m_anyAssignDly; }
+    bool isOutputter() const { return m_isOutputter; }
     int instrCount() const { return m_instrCount; }
     int dataCount() const { return m_dataCount; }
 
@@ -342,15 +344,16 @@ private:
         nodep->user2p((void*)valuep);
     }
 
-    void checkNodeInfo(AstNode* nodep) {
+    void checkNodeInfo(AstNode* nodep, bool ignorePredict = false) {
         if (m_checkOnly) {
             m_instrCount += nodep->instrCount();
             m_dataCount += nodep->width();
         }
-        if (!nodep->isPredictOptimizable()) {
+        if (!ignorePredict && !nodep->isPredictOptimizable()) {
             // UINFO(9, "     !predictopt " << nodep << endl);
             clearOptimizable(nodep, "Isn't predictable");
         }
+        if (nodep->isOutputter()) m_isOutputter = true;
     }
 
     void badNodeType(AstNode* nodep) {
@@ -756,6 +759,7 @@ private:
     virtual void visit(AstNodeAssign* nodep) override {
         if (jumpingOver(nodep)) return;
         if (!optimizable()) return;  // Accelerate
+        checkNodeInfo(nodep);
         if (VN_IS(nodep, AssignForce)) {
             clearOptimizable(nodep, "Force");
         } else if (VN_IS(nodep, AssignDly)) {
@@ -970,6 +974,7 @@ private:
         if (jumpingOver(nodep)) return;
         if (!optimizable()) return;  // Accelerate
         UINFO(5, "   FUNCREF " << nodep << endl);
+        checkNodeInfo(nodep);
         if (!m_params) {
             badNodeType(nodep);
             return;
@@ -1053,6 +1058,7 @@ private:
     virtual void visit(AstSFormatF* nodep) override {
         if (jumpingOver(nodep)) return;
         if (!optimizable()) return;  // Accelerate
+        checkNodeInfo(nodep);
         iterateChildren(nodep);
         if (m_params) {
             AstNode* nextArgp = nodep->exprsp();
@@ -1106,6 +1112,9 @@ private:
     virtual void visit(AstDisplay* nodep) override {
         if (jumpingOver(nodep)) return;
         if (!optimizable()) return;  // Accelerate
+        // We ignore isPredictOptimizable as $display is often in constant
+        // functions and we want them to work if used with parameters
+        checkNodeInfo(nodep, /*display:*/ true);
         iterateChildren(nodep);
         if (m_params) {
             AstConst* const textp = fetchConst(nodep->fmtp());
@@ -1155,6 +1164,7 @@ public:
         m_anyAssignComb = false;
         m_anyAssignDly = false;
         m_inDlyAssign = false;
+        m_isOutputter = false;
         m_instrCount = 0;
         m_dataCount = 0;
         m_jumpp = nullptr;

src/V3Table.cpp

@@ -225,6 +225,9 @@ private:
         if (!m_outWidthBytes || !m_inWidthBits) {
             chkvis.clearOptimizable(nodep, "Table has no outputs");
         }
+        if (chkvis.isOutputter()) {
+            chkvis.clearOptimizable(nodep, "Table creates display output");
+        }
         UINFO(4, "  Test: Opt=" << (chkvis.optimizable() ? "OK" : "NO") << ", Instrs="
                                 << chkvis.instrCount() << " Data=" << chkvis.dataCount()
                                 << " in width (bits)=" << m_inWidthBits << " out width (bytes)="

test_regress/t/t_opt_table_display.out

@@ -0,0 +1,12 @@
+Clocked
+Clocked
+Clocked
+Clocked
+Clocked
+Clocked
+Clocked
+Clocked
+Clocked
+Clocked
+Clocked
+*-* All Finished *-*

test_regress/t/t_opt_table_display.pl

@@ -0,0 +1,23 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2003 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(simulator => 1);
+
+compile(
+    verilator_flags2 => ["--stats"],
+    );
+
+execute(
+    check_finished => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;

test_regress/t/t_opt_table_display.v

@@ -0,0 +1,44 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2022 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+module t (/*AUTOARG*/
+   // Outputs
+   test,
+   // Inputs
+   clk
+   );
+   input clk;
+
+   output reg [5:0] test;
+   parameter STATE1 = 6'b000001;
+   parameter STATE2 = 6'b000010;
+   parameter STATE3 = 6'b000100;
+   parameter STATE4 = 6'b001000;
+   parameter STATE5 = 6'b010000;
+   parameter STATE6 = 6'b100000;
+
+   always @(posedge clk) begin
+      $display("Clocked");
+      case (test)
+        STATE1: test <= STATE2;
+        STATE2: test <= STATE3;
+        STATE3: test <= STATE4;
+        STATE4: test <= STATE5;
+        STATE5: test <= STATE6;
+        default: test <= STATE1;
+      endcase
+   end
+
+   int cyc;
+   always @(posedge clk) begin
+      cyc <= cyc + 1;
+      if (cyc == 10) begin
+         $write("*-* All Finished *-*\n");
+         $finish;
+      end
+   end
+
+endmodule