Fix incorrect localization when encountering non-leaf functions.

Fixes #3286.

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

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 VNUser1InUse m_user1InUse;
-    const VNUser2InUse m_inuser2;
+    const VNUser3InUse m_user3InUse;
-    const VNUser3InUse m_inuser3;
+    const VNUser4InUse m_user4InUse;
-    const VNUser4InUse m_inuser4;
 
     AstUser3Allocator<AstVarScope, std::unordered_set<AstCFunc*>> m_accessors;
     AstUser4Allocator<AstCFunc, std::unordered_multimap<const AstVarScope*, AstVarRef*>>
@@ -69,6 +69,13 @@ private:
                 && m_accessors(nodep).size() == 1);  // .. a block temp used in a single CFunc
     }
 
+    bool existsNonLeaf(const std::unordered_set<AstCFunc*>& 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<AstCFunc*>& 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());

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;

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