From c79ea88576b57d18f922ec29e27b23c284e774bc Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 9 Apr 2022 20:24:40 +0100 Subject: [PATCH] Fix incorrect localization when encountering non-leaf functions. Fixes #3286. --- Changes | 1 + src/V3Localize.cpp | 28 +++++++-- test_regress/t/t_opt_localize_deep.pl | 22 +++++++ test_regress/t/t_opt_localize_deep.v | 83 +++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 5 deletions(-) create mode 100755 test_regress/t/t_opt_localize_deep.pl create mode 100644 test_regress/t/t_opt_localize_deep.v diff --git a/Changes b/Changes index 25b7d3b41..d605bbe74 100644 --- a/Changes +++ b/Changes @@ -17,6 +17,7 @@ Verilator 4.221 devel * Deprecate 'vluint64_t' and similar types (#3255). * Fix MSVC localtime_s (#3124). * Fix Bison 3.8.2 error (#3366). [elike-ypq] +* Fix rare bug in -Oz (V3Localize) (#3286). [Geza Lore, Shunyao CAD] Verilator 4.220 2022-03-12 diff --git a/src/V3Localize.cpp b/src/V3Localize.cpp index eaf8a5ea6..618b010ab 100644 --- a/src/V3Localize.cpp +++ b/src/V3Localize.cpp @@ -40,15 +40,15 @@ class LocalizeVisitor final : public VNVisitor { private: // NODE STATE // AstVarScope::user1() -> Bool indicating VarScope is not optimizable. + // AstCFunc::user1() -> Bool indicating CFunc is not a leaf function. // AstVarScope::user2() -> Bool indicating VarScope was fully assigned in the current // function. // AstVarScope::user3p() -> Set of CFuncs referencing this VarScope. (via m_accessors) // AstCFunc::user4p() -> Multimap of 'VarScope -> VarRefs that reference that VarScope' // in this function. (via m_references) - const VNUser1InUse m_inuser1; - const VNUser2InUse m_inuser2; - const VNUser3InUse m_inuser3; - const VNUser4InUse m_inuser4; + const VNUser1InUse m_user1InUse; + const VNUser3InUse m_user3InUse; + const VNUser4InUse m_user4InUse; AstUser3Allocator> m_accessors; AstUser4Allocator> @@ -69,6 +69,13 @@ private: && m_accessors(nodep).size() == 1); // .. a block temp used in a single CFunc } + bool existsNonLeaf(const std::unordered_set& funcps) { + for (const AstCFunc* const funcp : funcps) { + if (funcp->user1()) return true; + } + return false; + } + void moveVarScopes() { for (AstVarScope* const nodep : m_varScopeps) { if (!isOptimizable(nodep)) continue; // Not optimizable @@ -76,6 +83,12 @@ private: const std::unordered_set& funcps = m_accessors(nodep); if (funcps.empty()) continue; // No referencing functions at all + // If more than one referencing function, but not all are leaf + // functions, then don't localize, as one of the referencing + // functions might be calling another, which the current analysis + // cannot cope with. This should be rare (introduced by V3Depth). + if (funcps.size() > 1 && existsNonLeaf(funcps)) continue; + UINFO(4, "Localizing " << nodep << endl); ++m_statLocVars; @@ -121,11 +134,16 @@ private: { m_cfuncp = nodep; m_nodeDepth = 0; - AstNode::user2ClearTree(); // Check each function independently + const VNUser2InUse user2InUse; iterateChildrenConst(nodep); } } + virtual void visit(AstCCall* nodep) override { + m_cfuncp->user1(true); // Mark caller as not a leaf function + iterateChildrenConst(nodep); + } + virtual void visit(AstNodeAssign* nodep) override { // Analyze RHS first so "a = a + 1" is detected as a read before write iterate(nodep->rhsp()); diff --git a/test_regress/t/t_opt_localize_deep.pl b/test_regress/t/t_opt_localize_deep.pl new file mode 100755 index 000000000..7d49cd2cd --- /dev/null +++ b/test_regress/t/t_opt_localize_deep.pl @@ -0,0 +1,22 @@ +#!/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 => ["--compiler msvc"], # We have deep expressions we want to test + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_opt_localize_deep.v b/test_regress/t/t_opt_localize_deep.v new file mode 100644 index 000000000..1c8267fc7 --- /dev/null +++ b/test_regress/t/t_opt_localize_deep.v @@ -0,0 +1,83 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Geza Lore. +// SPDX-License-Identifier: CC0-1.0 + +`ifdef verilator + `define dontOptimize $c1("1") +`else + `define dontOptimize 1'b1 +`endif + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + int cyc = 0; + int x = 0; + + always @(posedge clk) begin + cyc <= cyc + 1; + x = 32'hcafe1234; + + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) if (`dontOptimize) + x = cyc; + + $write("[%0t] cyc=%0d x=%x\n", $time, cyc, x); + if (x !== cyc) $stop; + if (cyc == 99) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule