From a4fddb3fbee12417257c29b95b2b82258b6c5c21 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 9 Jul 2022 07:55:46 -0400 Subject: [PATCH] Fix table misoptimizing away display (#3488). --- Changes | 2 ++ src/V3Simulate.h | 14 ++++++-- src/V3Table.cpp | 3 ++ test_regress/t/t_opt_table_display.out | 12 +++++++ test_regress/t/t_opt_table_display.pl | 23 ++++++++++++++ test_regress/t/t_opt_table_display.v | 44 ++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 test_regress/t/t_opt_table_display.out create mode 100755 test_regress/t/t_opt_table_display.pl create mode 100644 test_regress/t/t_opt_table_display.v diff --git a/Changes b/Changes index 2e1183c7b..5f68d4a40 100644 --- a/Changes +++ b/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 ========================== diff --git a/src/V3Simulate.h b/src/V3Simulate.h index 28691d478..195139cd2 100644 --- a/src/V3Simulate.h +++ b/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; diff --git a/src/V3Table.cpp b/src/V3Table.cpp index 0944bb589..30a9cb587 100644 --- a/src/V3Table.cpp +++ b/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)=" diff --git a/test_regress/t/t_opt_table_display.out b/test_regress/t/t_opt_table_display.out new file mode 100644 index 000000000..3bbd99b6b --- /dev/null +++ b/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 *-* diff --git a/test_regress/t/t_opt_table_display.pl b/test_regress/t/t_opt_table_display.pl new file mode 100755 index 000000000..e08f4a744 --- /dev/null +++ b/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; diff --git a/test_regress/t/t_opt_table_display.v b/test_regress/t/t_opt_table_display.v new file mode 100644 index 000000000..19fbe03ad --- /dev/null +++ b/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