From c0c8d7d5ab78e034e17da884da7fe64c3cc44503 Mon Sep 17 00:00:00 2001 From: Krzysztof Bieganski Date: Mon, 16 Oct 2023 14:10:53 +0200 Subject: [PATCH] Support NBAs in non-inlined tasks/functions Signed-off-by: Krzysztof Bieganski --- src/V3AstNodeOther.h | 6 + src/V3Delayed.cpp | 8 +- src/V3Sched.cpp | 137 ++++++++++-------- src/V3Timing.cpp | 69 +++++++-- test_regress/t/t_assigndly_dynamic.pl | 32 ++++ test_regress/t/t_assigndly_dynamic.v | 55 +++++++ .../t/t_assigndly_dynamic_notiming.out | 6 + .../t/t_assigndly_dynamic_notiming.pl | 20 +++ test_regress/t/t_assigndly_dynamic_notiming.v | 12 ++ test_regress/t/t_uvm_pkg_todo.vh | 4 +- 10 files changed, 272 insertions(+), 77 deletions(-) create mode 100755 test_regress/t/t_assigndly_dynamic.pl create mode 100644 test_regress/t/t_assigndly_dynamic.v create mode 100644 test_regress/t/t_assigndly_dynamic_notiming.out create mode 100755 test_regress/t/t_assigndly_dynamic_notiming.pl create mode 100644 test_regress/t/t_assigndly_dynamic_notiming.v diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 2fea7ff71a1..45129de44cf 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1191,6 +1191,8 @@ class AstNetlist final : public AstNode { AstCFunc* m_evalNbap = nullptr; // The '_eval__nba' function AstVarScope* m_dpiExportTriggerp = nullptr; // The DPI export trigger variable AstVar* m_delaySchedulerp = nullptr; // The delay scheduler variable + AstVarScope* m_nbaEventp = nullptr; // The NBA event variable + AstVarScope* m_nbaEventTriggerp = nullptr; // If set to 1, the NBA event should get triggered AstTopScope* m_topScopep = nullptr; // The singleton AstTopScope under the top module VTimescale m_timeunit; // Global time unit VTimescale m_timeprecision; // Global time precision @@ -1220,6 +1222,10 @@ class AstNetlist final : public AstNode { void dpiExportTriggerp(AstVarScope* varScopep) { m_dpiExportTriggerp = varScopep; } AstVar* delaySchedulerp() const { return m_delaySchedulerp; } void delaySchedulerp(AstVar* const varScopep) { m_delaySchedulerp = varScopep; } + AstVarScope* nbaEventp() const { return m_nbaEventp; } + void nbaEventp(AstVarScope* const varScopep) { m_nbaEventp = varScopep; } + AstVarScope* nbaEventTriggerp() const { return m_nbaEventTriggerp; } + void nbaEventTriggerp(AstVarScope* const varScopep) { m_nbaEventTriggerp = varScopep; } void stdPackagep(AstPackage* const packagep) { m_stdPackagep = packagep; } AstPackage* stdPackagep() const { return m_stdPackagep; } AstTopScope* topScopep() const { return m_topScopep; } diff --git a/src/V3Delayed.cpp b/src/V3Delayed.cpp index eba736d4864..12f74ac8d8b 100644 --- a/src/V3Delayed.cpp +++ b/src/V3Delayed.cpp @@ -527,8 +527,12 @@ class DelayedVisitor final : public VNVisitor { m_nextDlyp = VN_CAST(nodep->nextp(), AssignDly); // Next assignment in same block, maybe nullptr. if (m_cfuncp) { - nodep->v3warn(E_UNSUPPORTED, - "Unsupported: Delayed assignment inside public function/task"); + if (!v3Global.rootp()->nbaEventp()) { + nodep->v3warn( + E_NOTIMING, + "Delayed assignment in a non-inlined function/task requires --timing"); + } + return; } UASSERT_OBJ(m_procp, nodep, "Delayed assignment not under process"); const bool isArray = VN_IS(nodep->lhsp(), ArraySel) diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index a6453435cbf..62b4bb97888 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -899,72 +899,83 @@ void createEval(AstNetlist* netlistp, // if (icoLoop) funcp->addStmtsp(icoLoop); // Create the active eval loop - AstNodeStmt* const activeEvalLoopp - = makeEvalLoop( - netlistp, "act", "Active", actKit.m_vscp, actKit.m_dumpp, - [&]() { // Trigger - AstNodeStmt* resultp = nullptr; - - // Compute the current triggers - { - AstCCall* const trigsp = new AstCCall{flp, actKit.m_triggerComputep}; - trigsp->dtypeSetVoid(); - resultp = AstNode::addNext(resultp, trigsp->makeStmt()); - } - - // Commit trigger awaits from the previous iteration - if (AstCCall* const commitp = timingKit.createCommit(netlistp)) { - resultp = AstNode::addNext(resultp, commitp->makeStmt()); - } - - return resultp; - }, - [&]() { // Body - // Compute the pre triggers - AstNodeStmt* resultp - = createTriggerAndNotCall(flp, preTrigsp, actKit.m_vscp, nbaKit.m_vscp); - // Latch the active trigger flags under the NBA trigger flags - resultp = AstNode::addNext( - resultp, createTriggerSetCall(flp, nbaKit.m_vscp, actKit.m_vscp)); - // Resume triggered timing schedulers - if (AstCCall* const resumep = timingKit.createResume(netlistp)) { - resultp = AstNode::addNext(resultp, resumep->makeStmt()); - } - // Invoke body function - { - AstCCall* const callp = new AstCCall{flp, actKit.m_funcp}; - callp->dtypeSetVoid(); - resultp = AstNode::addNext(resultp, callp->makeStmt()); - } + const auto& activeEvalLoop = makeEvalLoop( + netlistp, "act", "Active", actKit.m_vscp, actKit.m_dumpp, + [&]() { // Trigger + AstNodeStmt* resultp = nullptr; + + // Compute the current triggers + { + AstCCall* const trigsp = new AstCCall{flp, actKit.m_triggerComputep}; + trigsp->dtypeSetVoid(); + resultp = AstNode::addNext(resultp, trigsp->makeStmt()); + } + + // Commit trigger awaits from the previous iteration + if (AstCCall* const commitp = timingKit.createCommit(netlistp)) { + resultp = AstNode::addNext(resultp, commitp->makeStmt()); + } - return resultp; - }) - .stmtsp; + return resultp; + }, + [&]() { // Body + // Compute the pre triggers + AstNodeStmt* resultp + = createTriggerAndNotCall(flp, preTrigsp, actKit.m_vscp, nbaKit.m_vscp); + // Latch the active trigger flags under the NBA trigger flags + resultp = AstNode::addNext(resultp, + createTriggerSetCall(flp, nbaKit.m_vscp, actKit.m_vscp)); + // Resume triggered timing schedulers + if (AstCCall* const resumep = timingKit.createResume(netlistp)) { + resultp = AstNode::addNext(resultp, resumep->makeStmt()); + } + // Invoke body function + { + AstCCall* const callp = new AstCCall{flp, actKit.m_funcp}; + callp->dtypeSetVoid(); + resultp = AstNode::addNext(resultp, callp->makeStmt()); + } + + return resultp; + }); // Create the NBA eval loop. This uses the Active eval loop in the trigger section. - AstNodeStmt* topEvalLoopp - = makeEvalLoop( - netlistp, "nba", "NBA", nbaKit.m_vscp, nbaKit.m_dumpp, - [&]() { // Trigger - // Reset NBA triggers - AstNodeStmt* resultp = createTriggerClearCall(flp, nbaKit.m_vscp); - // Run the Active eval loop - resultp = AstNode::addNext(resultp, activeEvalLoopp); - return resultp; - }, - [&]() { // Body - AstCCall* const callp = new AstCCall{flp, nbaKit.m_funcp}; - callp->dtypeSetVoid(); - AstNodeStmt* resultp = callp->makeStmt(); - // Latch the NBA trigger flags under the following region's trigger flags - AstVarScope* const nextVscp = obsKit.m_vscp ? obsKit.m_vscp : reactKit.m_vscp; - if (nextVscp) { - resultp = AstNode::addNext( - resultp, createTriggerSetCall(flp, nextVscp, nbaKit.m_vscp)); - } - return resultp; - }) - .stmtsp; + const auto& nbaEvalLoop = makeEvalLoop( + netlistp, "nba", "NBA", nbaKit.m_vscp, nbaKit.m_dumpp, + [&]() { // Trigger + // Reset NBA triggers + AstNodeStmt* resultp = createTriggerClearCall(flp, nbaKit.m_vscp); + // Run the Active eval loop + resultp = AstNode::addNext(resultp, activeEvalLoop.stmtsp); + return resultp; + }, + [&]() { // Body + AstCCall* const callp = new AstCCall{flp, nbaKit.m_funcp}; + callp->dtypeSetVoid(); + AstNodeStmt* resultp = callp->makeStmt(); + // Latch the NBA trigger flags under the following region's trigger flags + AstVarScope* const nextVscp = obsKit.m_vscp ? obsKit.m_vscp : reactKit.m_vscp; + if (nextVscp) { + resultp = AstNode::addNext(resultp, + createTriggerSetCall(flp, nextVscp, nbaKit.m_vscp)); + } + return resultp; + }); + + // If the NBA event exists, trigger it in 'nba' + if (netlistp->nbaEventp()) { + AstIf* const ifp + = new AstIf{flp, new AstVarRef{flp, netlistp->nbaEventTriggerp(), VAccess::READ}}; + ifp->addThensp(setVar(nbaEvalLoop.continuep, 1)); + ifp->addThensp(setVar(netlistp->nbaEventTriggerp(), 0)); + AstCMethodHard* const firep = new AstCMethodHard{ + flp, new AstVarRef{flp, netlistp->nbaEventp(), VAccess::WRITE}, "fire"}; + firep->dtypeSetVoid(); + ifp->addThensp(firep->makeStmt()); + activeEvalLoop.stmtsp->addNext(ifp); + } + + AstNodeStmt* topEvalLoopp = nbaEvalLoop.stmtsp; if (obsKit.m_funcp) { // Create the Observed eval loop. This uses the NBA eval loop in the trigger section. diff --git a/src/V3Timing.cpp b/src/V3Timing.cpp index 132d3d6bf95..7b602e1282f 100644 --- a/src/V3Timing.cpp +++ b/src/V3Timing.cpp @@ -366,6 +366,10 @@ class TimingSuspendableVisitor final : public VNVisitor { m_underFork |= F_MIGHT_NEED_PROC; iterateChildren(nodep); } + void visit(AstAssignDly* nodep) override { + if (!VN_IS(m_procp, NodeProcedure)) v3Global.setUsesTiming(); + visit(static_cast(nodep)); + } void visit(AstNode* nodep) override { if (nodep->isTimingControl()) { v3Global.setUsesTiming(); @@ -452,8 +456,10 @@ class TimingControlVisitor final : public VNVisitor { double m_timescaleFactor = 1.0; // Factor to scale delays by 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 // Unique names + V3UniqueNames m_dlyforkNames{"__Vdlyfork"}; // Names for temp AssignW vars V3UniqueNames m_contAssignVarNames{"__VassignWtmp"}; // Names for temp AssignW vars V3UniqueNames m_intraValueNames{"__Vintraval"}; // Intra assign delay value var names V3UniqueNames m_intraIndexNames{"__Vintraidx"}; // Intra assign delay index var names @@ -575,6 +581,31 @@ class TimingControlVisitor final : public VNVisitor { m_netlistp->topScopep()->addSenTreesp(m_dynamicSensesp); return m_dynamicSensesp; } + // Creates the event variable to trigger in NBA region + AstEventControl* createNbaEventControl(FileLine* flp) { + if (!m_netlistp->nbaEventp()) { + auto* const nbaEventDtp = new AstBasicDType{m_scopeTopp->fileline(), + VBasicDTypeKwd::EVENT, VSigning::UNSIGNED}; + m_netlistp->typeTablep()->addTypesp(nbaEventDtp); + m_netlistp->nbaEventp(m_scopeTopp->createTemp("__VnbaEvent", nbaEventDtp)); + v3Global.setHasEvents(); + } + return new AstEventControl{ + flp, + new AstSenTree{ + flp, new AstSenItem{flp, VEdgeType::ET_EVENT, + new AstVarRef{flp, m_netlistp->nbaEventp(), VAccess::READ}}}, + nullptr}; + } + // Creates the variable that, if set, causes the NBA event to be triggered + AstAssign* createNbaEventTriggerAssignment(FileLine* flp) { + if (!m_netlistp->nbaEventTriggerp()) { + m_netlistp->nbaEventTriggerp(m_scopeTopp->createTemp("__VnbaEventTrigger", 1)); + } + return new AstAssign{flp, + 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. bool needDynamicTrigger(AstNode* const nodep) const { @@ -753,6 +784,8 @@ class TimingControlVisitor final : public VNVisitor { void visit(AstNodeProcedure* nodep) override { VL_RESTORER(m_procp); m_procp = nodep; + VL_RESTORER(m_underProcedure); + m_underProcedure = true; iterateChildren(nodep); if (hasFlags(nodep, T_SUSPENDEE)) nodep->setSuspendable(); if (hasFlags(nodep, T_HAS_PROC)) nodep->setNeedProcess(); @@ -773,7 +806,8 @@ class TimingControlVisitor final : public VNVisitor { if (nodep->user1SetOnce()) return; VL_RESTORER(m_procp); m_procp = nodep; - + VL_RESTORER(m_underProcedure); + m_underProcedure = true; // Workaround for killing `always` processes (doing that is pretty much UB) // TODO: Disallow killing `always` at runtime (throw an error) if (hasFlags(nodep, T_HAS_PROC)) addFlags(nodep, T_SUSPENDEE); @@ -979,20 +1013,37 @@ class TimingControlVisitor final : public VNVisitor { void visit(AstNodeAssign* nodep) override { // Only process once to avoid infinite loops (due to the net delay) if (nodep->user1SetOnce()) return; - AstNode* const controlp = factorOutTimingControl(nodep); - if (!controlp) return; - // Handle the intra assignment timing control FileLine* const flp = nodep->fileline(); - if (VN_IS(nodep, AssignDly)) { - // If it's an NBA with an intra assignment delay, put it in a fork + AstNode* controlp = factorOutTimingControl(nodep); + const bool inAssignDly = VN_IS(nodep, AssignDly); + // Handle the intra assignment timing control + // Transform if: + // * there's a timing control in the assignment + // * the assignment is an AssignDly and it's in a non-inlined function + if (!controlp && (!inAssignDly || m_underProcedure)) return; + // Insert new vars before the timing control if we're in a function; in a process we can't + // do that. These intra-assignment vars will later be passed to forked processes by value. + AstNode* insertBeforep = m_underProcedure ? nullptr : controlp; + // Special case for NBA + if (inAssignDly) { + // Put it in a fork so it doesn't block auto* const forkp = new AstFork{flp, "", nullptr}; forkp->joinType(VJoinType::JOIN_NONE); + if (!m_underProcedure) { + // If it's in a function, it won't be handled by V3Delayed + // Put it behind an additional named event that gets triggered in the NBA region + AstEventControl* const nbaEventControlp = createNbaEventControl(flp); + AstAssign* const trigAssignp = createNbaEventTriggerAssignment(flp); + nodep->replaceWith(trigAssignp); + trigAssignp->addNextHere(nbaEventControlp); + nbaEventControlp->addStmtsp(nodep); + insertBeforep = forkp; + if (!controlp) controlp = nbaEventControlp; + } controlp->replaceWith(forkp); forkp->addStmtsp(controlp); } - // Insert new vars before the timing control if we're in a function; in a process we can't - // do that. These intra-assignment vars will later be passed to forked processes by value. - AstNode* const insertBeforep = m_classp ? controlp : nullptr; + UASSERT_OBJ(nodep, controlp, "Assignment should have timing control"); addCLocalScope(flp, insertBeforep); // Function for replacing values with intermediate variables const auto replaceWithIntermediate = [&](AstNodeExpr* const valuep, diff --git a/test_regress/t/t_assigndly_dynamic.pl b/test_regress/t/t_assigndly_dynamic.pl new file mode 100755 index 00000000000..3576e3ffb55 --- /dev/null +++ b/test_regress/t/t_assigndly_dynamic.pl @@ -0,0 +1,32 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2019 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 => ["--exe --main --timing"], + make_main => 0, + ); + +execute( + check_finished => 1, + ); + +compile( + verilator_flags2 => ["--exe --main --timing +define+WITH_DELAY"], + make_main => 0, + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_assigndly_dynamic.v b/test_regress/t/t_assigndly_dynamic.v new file mode 100644 index 00000000000..9183eb767ff --- /dev/null +++ b/test_regress/t/t_assigndly_dynamic.v @@ -0,0 +1,55 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +`ifdef WITH_DELAY +`define DELAY #1 +`define TIME_AFTER_FIRST_WAIT 2 +`define TIME_AFTER_SECOND_WAIT 3 +`else +`define DELAY +`define TIME_AFTER_FIRST_WAIT 1 +`define TIME_AFTER_SECOND_WAIT 1 +`endif + +class nba_waiter; + // Task taken from UVM + task wait_for_nba_region; + int nba; + int next_nba; + next_nba++; + nba <= `DELAY next_nba; + @(nba); + endtask +endclass + +module t; + nba_waiter waiter = new; + event e; + int cnt = 0; + + initial begin + #1 ->e; + if (cnt != 0) $stop; + cnt++; + waiter.wait_for_nba_region; + ->e; + if (cnt != 2) $stop; + if ($time != `TIME_AFTER_FIRST_WAIT) $stop; + cnt++; + waiter.wait_for_nba_region; + if (cnt != 4) $stop; + if ($time != `TIME_AFTER_SECOND_WAIT) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + + initial begin + @e if (cnt != 1) $stop; + cnt++; + @e if (cnt != 3) $stop; + cnt++; + end +endmodule diff --git a/test_regress/t/t_assigndly_dynamic_notiming.out b/test_regress/t/t_assigndly_dynamic_notiming.out new file mode 100644 index 00000000000..7fe77946a67 --- /dev/null +++ b/test_regress/t/t_assigndly_dynamic_notiming.out @@ -0,0 +1,6 @@ +%Error-NOTIMING: t/t_assigndly_dynamic_notiming.v:10:13: Delayed assignment in a non-inlined function/task requires --timing + : ... note: In instance '$unit::foo' + 10 | qux <= '1; + | ^~ + ... For error description see https://verilator.org/warn/NOTIMING?v=latest +%Error: Exiting due to diff --git a/test_regress/t/t_assigndly_dynamic_notiming.pl b/test_regress/t/t_assigndly_dynamic_notiming.pl new file mode 100755 index 00000000000..1e658d93726 --- /dev/null +++ b/test_regress/t/t_assigndly_dynamic_notiming.pl @@ -0,0 +1,20 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2019 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 => ["--no-timing"], + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_assigndly_dynamic_notiming.v b/test_regress/t/t_assigndly_dynamic_notiming.v new file mode 100644 index 00000000000..f4b96c4de58 --- /dev/null +++ b/test_regress/t/t_assigndly_dynamic_notiming.v @@ -0,0 +1,12 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +class foo; + task bar; + int qux; + qux <= '1; + endtask +endclass diff --git a/test_regress/t/t_uvm_pkg_todo.vh b/test_regress/t/t_uvm_pkg_todo.vh index 30f708ef383..a99dd64260e 100644 --- a/test_regress/t/t_uvm_pkg_todo.vh +++ b/test_regress/t/t_uvm_pkg_todo.vh @@ -866,9 +866,7 @@ task uvm_wait_for_nba_region; int nba; int next_nba; next_nba++; -//TODO issue #4496 - Delayed assignment inside public function/task -//TODO %Error-UNSUPPORTED: t/t_uvm_pkg_todo.vh:875:7: Unsupported: Delayed assignment inside public function/task -//TODO nba <= next_nba; + nba <= next_nba; @(nba); endtask function automatic void uvm_split_string (string str, byte sep, ref string values[$]);