Fix deep traversal of class inheritance timing (#4216)

src/V3Timing.cpp

@@ -57,6 +57,8 @@
 #include "V3SenTree.h"
 #include "V3UniqueNames.h"
 
+#include <queue>
+
 VL_DEFINE_DEBUG_FUNCTIONS;
 
 // ######################################################################
@@ -143,26 +145,40 @@ private:
         if (!m_classp) return;
         // If class method (possibly overrides another method)
         if (!m_classp->user1SetOnce()) m_classp->repairCache();
+
         // Go over overridden functions
-        for (auto* cextp = m_classp->extendsp(); cextp;
+
-             cextp = VN_AS(cextp->nextp(), ClassExtends)) {
+        std::queue<AstClassExtends*> extends;
-            // TODO: It is possible that a methods the same name in the base class is not
+        if (m_classp->extendsp()) extends.push(m_classp->extendsp());
-            // actually overridden by our method. If this causes a problem, traverse to the
+
-            // root of the inheritance hierarchy and check if the original method is virtual or
+        while (!extends.empty()) {
-            // not.
+            AstClassExtends* ext_list = extends.front();
-            if (!cextp->classp()->user1SetOnce()) cextp->classp()->repairCache();
+            extends.pop();
-            if (auto* const overriddenp
+
-                = VN_CAST(cextp->classp()->findMember(nodep->name()), CFunc)) {
+            for (AstClassExtends* cextp = ext_list; cextp;
-                if (overriddenp->user2()) {  // If it's suspendable
+                 cextp = VN_AS(cextp->nextp(), ClassExtends)) {
-                    nodep->user2(true);  // Then we are also suspendable
+                // TODO: It is possible that a methods the same name in the base class is not
-                    // As both are suspendable already, there is no need to add it as our
+                // actually overridden by our method. If this causes a problem, traverse to
-                    // dependency or self to its dependencies
+                // the root of the inheritance hierarchy and check if the original method is
+                // virtual or not.
+                if (!cextp->classp()->user1SetOnce()) cextp->classp()->repairCache();
+                if (AstCFunc* const overriddenp
+                    = VN_CAST(cextp->classp()->findMember(nodep->name()), CFunc)) {
+                    if (overriddenp->user2()) {  // If it's suspendable
+                        nodep->user2(true);  // Then we are also suspendable
+                        // As both are suspendable already, there is no need to add it as our
+                        // dependency or self to its dependencies
+                    } else {
+                        // Make a dependency cycle, as being suspendable should propagate both
+                        // up and down the inheritance tree
+                        TimingDependencyVertex* const overriddenVxp
+                            = getDependencyVertex(overriddenp);
+                        new V3GraphEdge{&m_depGraph, vxp, overriddenVxp, 1};
+                        new V3GraphEdge{&m_depGraph, overriddenVxp, vxp, 1};
+                    }
                 } else {
-                    // Make a dependency cycle, as being suspendable should propagate both up
+                    AstClassExtends* more_extends = cextp->classp()->extendsp();
-                    // and down the inheritance tree
+                    if (more_extends) extends.push(more_extends);
-                    TimingDependencyVertex* const overriddenVxp = getDependencyVertex(overriddenp);
-                    new V3GraphEdge{&m_depGraph, vxp, overriddenVxp, 1};
-                    new V3GraphEdge{&m_depGraph, overriddenVxp, vxp, 1};
                 }
             }
         }

test_regress/t/t_suspendable_deep.pl

@@ -0,0 +1,18 @@
+#!/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(vlt => 1);
+
+compile(
+    verilator_flags2 => ["--timing"],
+    );
+
+ok(1);
+1;

test_regress/t/t_suspendable_deep.v

@@ -0,0 +1,35 @@
+// DESCRIPTION: Verilator: Verilog Test module for specialized type default values
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2023 by Antmicro.
+// SPDX-License-Identifier: CC0-1.0
+
+`timescale 1ns/1ns
+
+event evt;
+
+class Baz;
+  virtual task do_something(); endtask
+endclass
+
+class Foo extends Baz;
+endclass
+
+class Bar extends Foo;
+  virtual task do_something();
+    @evt $display("Hello");
+  endtask
+endclass
+
+module top();
+  initial begin
+    Bar bar;
+    bar = new;
+
+    fork
+      #10 bar.do_something();
+      #20 $display("world!");
+      #10 ->evt;
+    join
+  end
+endmodule