From 2022bacd388995dedb6abcb8fa82471164cafd95 Mon Sep 17 00:00:00 2001 From: Yilou Wang Date: Mon, 3 Aug 2026 11:15:07 +0200 Subject: [PATCH] Fix timing controls in interface tasks called via virtual interface (#7959) --- src/V3Task.cpp | 44 ++++ src/V3Timing.cpp | 9 +- .../t/t_interface_virtual_task_timing.py | 18 ++ .../t/t_interface_virtual_task_timing.v | 198 ++++++++++++++++++ .../t_interface_virtual_task_timing_noinl.py | 20 ++ .../t_interface_virtual_task_timing_unsup.out | 6 + .../t_interface_virtual_task_timing_unsup.py | 16 ++ .../t/t_interface_virtual_task_timing_unsup.v | 27 +++ 8 files changed, 335 insertions(+), 3 deletions(-) create mode 100755 test_regress/t/t_interface_virtual_task_timing.py create mode 100644 test_regress/t/t_interface_virtual_task_timing.v create mode 100755 test_regress/t/t_interface_virtual_task_timing_noinl.py create mode 100644 test_regress/t/t_interface_virtual_task_timing_unsup.out create mode 100755 test_regress/t/t_interface_virtual_task_timing_unsup.py create mode 100644 test_regress/t/t_interface_virtual_task_timing_unsup.v diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 2c3d7c99c..6130066bb 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -1883,6 +1883,49 @@ public: } }; +//###################################################################### +// Mark interface members under timing controls of interface CFuncs as interface-sensed + +class TaskIfaceSensVisitor final : public VNVisitorConst { + // STATE + bool m_underIfaceFunc = false; // Under a CFunc owned by an interface scope + bool m_underSenses = false; // Under a sensitivity expression of such a CFunc + + // METHODS + void markSensesAndIterate(AstNode* nodep) { + if (!m_underIfaceFunc) return; + VL_RESTORER(m_underSenses); + m_underSenses = true; + iterateAndNextConstNull(nodep); + } + // VISITORS + void visit(AstCFunc* nodep) override { + VL_RESTORER(m_underIfaceFunc); + m_underIfaceFunc = VN_IS(nodep->scopep()->modp(), Iface); + iterateChildrenConst(nodep); + } + void visit(AstSenTree* nodep) override { markSensesAndIterate(nodep->sensesp()); } + void visit(AstWait* nodep) override { + markSensesAndIterate(nodep->condp()); + iterateAndNextConstNull(nodep->stmtsp()); + } + void visit(AstVarRef* nodep) override { + if (!m_underSenses) return; + UASSERT_OBJ(nodep->varScopep(), nodep, "No var scope"); + // Keep temps: the clocking event var is a MODULETEMP + if (nodep->varp()->isFuncLocal()) return; + if (AstIface* const ifacep = VN_CAST(nodep->varScopep()->scopep()->modp(), Iface)) { + nodep->varp()->sensIfacep(ifacep); + } + } + void visit(AstNode* nodep) override { iterateChildrenConst(nodep); } + +public: + // CONSTRUCTORS + explicit TaskIfaceSensVisitor(AstNetlist* nodep) { iterateChildrenConst(nodep); } + ~TaskIfaceSensVisitor() override = default; +}; + //###################################################################### // Task class functions @@ -2302,5 +2345,6 @@ void V3Task::taskAll(AstNetlist* nodep) { TaskStateVisitor visitors{nodep}; const TaskVisitor visitor{nodep, &visitors}; } // Destruct before checking + { TaskIfaceSensVisitor{nodep}; } V3Global::dumpCheckGlobalTree("task", 0, dumpTreeEitherLevel() >= 3); } diff --git a/src/V3Timing.cpp b/src/V3Timing.cpp index 385bf8c8a..1d8c844e5 100644 --- a/src/V3Timing.cpp +++ b/src/V3Timing.cpp @@ -482,6 +482,7 @@ class TimingControlVisitor final : public VNVisitor { int m_forkCnt = 0; // Number of forks inside a module bool m_underJumpBlock = false; // True if we are inside of a jump-block bool m_underProcedure = false; // True if we are under an always or initial + bool m_underIfaceCFunc = false; // True if we are under a CFunc owned by an interface scope bool m_hasStaticZeroDelay = false; // True if we have a static #0 delay std::vector m_unknownDelayFlps; // Locations of AstDelay with non-constant value @@ -633,10 +634,10 @@ class TimingControlVisitor final : public VNVisitor { new AstVarRef{flp, m_netlistp->nbaEventTriggerp(), VAccess::WRITE}, new AstConst{flp, AstConst::BitTrue{}}}; } - // Returns true if we are under a class or the given tree has any references to locals. These - // are cases where static, globally-evaluated triggers are not suitable. + // Returns true if we are under a class or interface function, or the tree references locals. + // These are cases where static, globally-evaluated triggers are not suitable. bool needDynamicTrigger(AstNode* const nodep) const { - return m_classp || nodep->exists([](AstNode* const nodep) { + return m_classp || m_underIfaceCFunc || nodep->exists([](AstNode* const nodep) { if (AstNodeVarRef* varp = VN_CAST(nodep, NodeVarRef)) { return varp->varp()->isFuncLocal(); } @@ -959,8 +960,10 @@ class TimingControlVisitor final : public VNVisitor { void visit(AstCFunc* nodep) override { VL_RESTORER(m_procp); VL_RESTORER(m_hasProcess); + VL_RESTORER(m_underIfaceCFunc); m_procp = nodep; m_hasProcess = hasFlags(nodep, T_HAS_PROC); + m_underIfaceCFunc = VN_IS(m_scopep->modp(), Iface); iterateChildren(nodep); if (hasFlags(nodep, T_HAS_PROC)) nodep->setNeedProcess(); if (!(hasFlags(nodep, T_SUSPENDEE))) return; diff --git a/test_regress/t/t_interface_virtual_task_timing.py b/test_regress/t/t_interface_virtual_task_timing.py new file mode 100755 index 000000000..6fe7d000c --- /dev/null +++ b/test_regress/t/t_interface_virtual_task_timing.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile(verilator_flags2=["--binary"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_virtual_task_timing.v b/test_regress/t/t_interface_virtual_task_timing.v new file mode 100644 index 000000000..f3842a0c5 --- /dev/null +++ b/test_regress/t/t_interface_virtual_task_timing.v @@ -0,0 +1,198 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 PlanV GmbH +// SPDX-License-Identifier: CC0-1.0 + +package t_pkg; + bit go = 0; +endpackage + +interface clkif ( + input logic clk +); + int id = 0; + clocking cb @(posedge clk); + endclocking + task automatic wait_clks_cb(int n, output int seen_id); + repeat (n) @cb; + seen_id = id; + endtask + task automatic wait_clks_raw(int n, output int seen_id); + repeat (n) @(posedge clk); + seen_id = id; + endtask + task automatic wait_clks_lvl(int n, output int seen_id); + repeat (n) begin + wait (clk == 1'b0); + wait (clk == 1'b1); + end + seen_id = id; + endtask + task automatic wait_intra(output int seen_id); + seen_id = @cb id; + endtask + task automatic wait_until_id(int target, output int seen_id); + wait (id == target); + seen_id = id; + endtask + task automatic wait_pkg_go(output int seen_id); + wait (t_pkg::go); + seen_id = id; + endtask +endinterface + +interface wrapif ( + input logic clk +); + clkif nested_if (.clk(clk)); +endinterface + +class Waiter; + virtual clkif m_vif; + function new(virtual clkif vif); + m_vif = vif; + endfunction + task wait_clks(int n, output int seen_id); + m_vif.wait_clks_cb(n, seen_id); + endtask +endclass + +module t; + logic slow_clk = 0, fast_clk = 0; + always #50 slow_clk = ~slow_clk; // 100 time-unit period + always #7 fast_clk = ~fast_clk; // 14 time-unit period + + clkif main_s (.clk(slow_clk)); // id=1, the vif target + clkif sib_f (.clk(fast_clk)); // id=2, sibling decoy + wrapif wrap_f (.clk(fast_clk)); // id=3, nested decoy + + int mod_cnt = 0; + task automatic wait_mod_cnt(int target); + wait (mod_cnt == target); + endtask + + task automatic check(string name, realtime dt, realtime lo, realtime hi, int seen_id, int exp_id); + if (dt < lo || dt > hi || seen_id != exp_id) begin + $display("%%Error: %s: dt=%0t seen_id=%0d expected id=%0d dt=[%0t:%0t]", name, dt, seen_id, + exp_id, lo, hi); + $stop; + end + endtask + + initial begin + virtual clkif vs, vf; + int seen_id; + realtime t0; + Waiter w; + vs = main_s; + vf = sib_f; + main_s.id = 1; + sib_f.id = 2; + wrap_f.nested_if.id = 3; + w = new(main_s); + #1000; + + // Task body waits on the clocking block event of the slow instance + t0 = $realtime; + seen_id = -1; + vs.wait_clks_cb(2, seen_id); + check("cb_task", $realtime - t0, 100, 200, seen_id, 1); + + // Task body waits on a raw posedge of the slow instance's clock + t0 = $realtime; + seen_id = -1; + vs.wait_clks_raw(2, seen_id); + check("raw_task", $realtime - t0, 100, 200, seen_id, 1); + + // Task body uses level waits on the slow instance's clock + t0 = $realtime; + seen_id = -1; + vs.wait_clks_lvl(2, seen_id); + check("lvl_task", $realtime - t0, 100, 200, seen_id, 1); + + // Intra-assignment event control; aligned so the next cb event is 50 away + @(negedge slow_clk); + t0 = $realtime; + seen_id = -1; + vs.wait_intra(seen_id); + check("intra_task", $realtime - t0, 40, 60, seen_id, 1); + + // Call-site event control through the handle + t0 = $realtime; + seen_id = -1; + repeat (2) @vs.cb; + seen_id = vs.id; + check("call_site", $realtime - t0, 100, 200, seen_id, 1); + + // Class method wrapping the virtual interface call (UVM shape) + t0 = $realtime; + seen_id = -1; + w.wait_clks(2, seen_id); + check("class_vif", $realtime - t0, 100, 200, seen_id, 1); + + // Second handle to the fast instance: per-call dispatch both ways + t0 = $realtime; + seen_id = -1; + vf.wait_clks_cb(2, seen_id); + check("vif_fast", $realtime - t0, 1, 56, seen_id, 2); + + t0 = $realtime; + seen_id = -1; + vs.wait_clks_cb(2, seen_id); + check("vif_slow_again", $realtime - t0, 100, 200, seen_id, 1); + + // Hierarchical (non-virtual) call must keep timing on its own instance + t0 = $realtime; + seen_id = -1; + sib_f.wait_clks_cb(2, seen_id); + check("hier_fast", $realtime - t0, 1, 56, seen_id, 2); + + // Wait condition mixing the member with a task argument + fork + #30 main_s.id = 42; + join_none + t0 = $realtime; + seen_id = -1; + vs.wait_until_id(42, seen_id); + check("wait_arg", $realtime - t0, 25, 35, seen_id, 42); + main_s.id = 1; + + // Wait condition on a package variable inside the interface task + fork + #20 t_pkg::go = 1'b1; + join_none + t0 = $realtime; + seen_id = -1; + vs.wait_pkg_go(seen_id); + check("wait_pkg", $realtime - t0, 15, 25, seen_id, 1); + + // Module-task wait on a task argument stays on the module path + fork + #10 mod_cnt = 5; + join_none + t0 = $realtime; + wait_mod_cnt(5); + check("wait_mod", $realtime - t0, 5, 15, mod_cnt, 5); + + // Wait on an automatic local in a procedure context + begin + automatic int tgt = 42; + fork + #10 mod_cnt = 42; + join_none + t0 = $realtime; + wait (mod_cnt == tgt); + check("wait_auto", $realtime - t0, 5, 15, mod_cnt, 42); + end + + $write("*-* All Finished *-*\n"); + $finish; + end + + initial begin + #100000; + $display("%%Error: timeout"); + $stop; + end +endmodule diff --git a/test_regress/t/t_interface_virtual_task_timing_noinl.py b/test_regress/t/t_interface_virtual_task_timing_noinl.py new file mode 100755 index 000000000..9cc922019 --- /dev/null +++ b/test_regress/t/t_interface_virtual_task_timing_noinl.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.top_filename = "t/t_interface_virtual_task_timing.v" + +test.compile(verilator_flags2=["--binary", "-fno-inline"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_virtual_task_timing_unsup.out b/test_regress/t/t_interface_virtual_task_timing_unsup.out new file mode 100644 index 000000000..e4080b387 --- /dev/null +++ b/test_regress/t/t_interface_virtual_task_timing_unsup.out @@ -0,0 +1,6 @@ +%Error-UNSUPPORTED: t/t_interface_virtual_task_timing_unsup.v:12:5: Unsupported: Event control with implicit sensitivity (@*) in this context; use an explicit sensitivity list instead + : ... note: In instance 't.c' + 12 | @(*) seen_id = id; + | ^ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: Exiting due to diff --git a/test_regress/t/t_interface_virtual_task_timing_unsup.py b/test_regress/t/t_interface_virtual_task_timing_unsup.py new file mode 100755 index 000000000..344a4e20a --- /dev/null +++ b/test_regress/t/t_interface_virtual_task_timing_unsup.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('vlt') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_interface_virtual_task_timing_unsup.v b/test_regress/t/t_interface_virtual_task_timing_unsup.v new file mode 100644 index 000000000..e347eaefd --- /dev/null +++ b/test_regress/t/t_interface_virtual_task_timing_unsup.v @@ -0,0 +1,27 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 PlanV GmbH +// SPDX-License-Identifier: CC0-1.0 + +interface clkif ( + input logic clk +); + int id = 0; + task automatic wait_any(output int seen_id); + @(*) seen_id = id; + endtask +endinterface + +module t; + logic clk = 0; + always #5 clk = ~clk; + clkif c (.clk(clk)); + initial begin + virtual clkif v; + int s; + v = c; + v.wait_any(s); + $finish; + end +endmodule