From 5af271cf3af2a2276530e347dfb1a03493fd8632 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 19 Oct 2023 18:33:58 -0400 Subject: [PATCH] Fix display optimization ignoring side effects (#4585). --- Changes | 1 + src/V3Ast.h | 2 ++ src/V3Const.cpp | 2 ++ test_regress/t/t_display_impure.out | 3 +++ test_regress/t/t_display_impure.pl | 24 ++++++++++++++++++++++++ test_regress/t/t_display_impure.v | 19 +++++++++++++++++++ 6 files changed, 51 insertions(+) create mode 100644 test_regress/t/t_display_impure.out create mode 100755 test_regress/t/t_display_impure.pl create mode 100644 test_regress/t/t_display_impure.v diff --git a/Changes b/Changes index 873f5d1c3..7ec977f93 100644 --- a/Changes +++ b/Changes @@ -43,6 +43,7 @@ Verilator 5.017 devel * Fix shift to remove operation side effects (#4563). * Fix compile warning on unused member function variable (#4567). * Fix method narrowing conversion compiler error (#4568). +* Fix display optimization ignoring side effects (#4585). * Fix preprocessor to show `line 2 on resumed file. diff --git a/src/V3Ast.h b/src/V3Ast.h index 2bc4fe06d..a17cc84e0 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -2132,6 +2132,8 @@ public: virtual bool isPredictOptimizable() const { return !isTimingControl(); } // Else a $display, etc, that must be ordered with other displays virtual bool isPure() { return true; } + // Iff isPure on current node and any nextp() + bool isPureAndNext() { return isPure() && (!nextp() || nextp()->isPure()); } // Else a AstTime etc that can't be substituted out virtual bool isSubstOptimizable() const { return true; } // An event control, delay, wait, etc. diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 1e3685e5b..bb68a436b 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -3149,6 +3149,8 @@ private: // right line numbers, nor scopeNames as might be different scopes (late in process) if (!m_doCpp && pformatp->exprsp()) return false; if (!m_doCpp && nformatp->exprsp()) return false; + if (pformatp->exprsp() && !pformatp->exprsp()->isPureAndNext()) return false; + if (nformatp->exprsp() && !nformatp->exprsp()->isPureAndNext()) return false; // Avoid huge merges static constexpr int DISPLAY_MAX_MERGE_LENGTH = 500; if (pformatp->text().length() + nformatp->text().length() > DISPLAY_MAX_MERGE_LENGTH) diff --git a/test_regress/t/t_display_impure.out b/test_regress/t/t_display_impure.out new file mode 100644 index 000000000..fcf5005f5 --- /dev/null +++ b/test_regress/t/t_display_impure.out @@ -0,0 +1,3 @@ + 1 + 2 +*-* All Finished *-* diff --git a/test_regress/t/t_display_impure.pl b/test_regress/t/t_display_impure.pl new file mode 100755 index 000000000..61f6d3396 --- /dev/null +++ b/test_regress/t/t_display_impure.pl @@ -0,0 +1,24 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2023 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(linter => 1); + +compile( + verilator_flags2 => ["--exe --main --timing"], + make_main => 0, + ); + +execute( + check_finished => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_display_impure.v b/test_regress/t/t_display_impure.v new file mode 100644 index 000000000..ad24975d3 --- /dev/null +++ b/test_regress/t/t_display_impure.v @@ -0,0 +1,19 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +function integer f; + static integer i = 0; + return ++i; +endfunction + +module t; + initial begin + $display("%d", f()); + $display("%d", f()); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule