From caed0865165857cb5d5c4560a35da2865116556e Mon Sep 17 00:00:00 2001 From: Krzysztof Bieganski Date: Thu, 13 Oct 2022 21:04:43 +0200 Subject: [PATCH] Move Postponed logic after the eval loop (#3673) Signed-off-by: Krzysztof Bieganski --- src/V3Active.cpp | 23 ++++++++++++++-------- src/V3Assert.cpp | 2 +- src/V3AstNodeOther.h | 2 +- src/V3Order.cpp | 21 +------------------- src/V3Sched.cpp | 25 +++++++++++++++++++++--- src/V3Sched.h | 1 + test_regress/t/t_past_strobe.out | 1 - test_regress/t/t_past_strobe.v | 2 +- test_regress/t/t_timing_strobe.out | 4 ++++ test_regress/t/t_timing_strobe.pl | 29 ++++++++++++++++++++++++++++ test_regress/t/t_timing_strobe.v | 31 ++++++++++++++++++++++++++++++ 11 files changed, 106 insertions(+), 35 deletions(-) create mode 100644 test_regress/t/t_timing_strobe.out create mode 100755 test_regress/t/t_timing_strobe.pl create mode 100644 test_regress/t/t_timing_strobe.v diff --git a/src/V3Active.cpp b/src/V3Active.cpp index f753bfcd7..0d0b0e3b4 100644 --- a/src/V3Active.cpp +++ b/src/V3Active.cpp @@ -243,16 +243,21 @@ public: // METHODS AstScope* scopep() { return m_scopep; } - // Return an AstActive sensitive to the given special sensitivity class + // Make a new AstActive sensitive to the given special sensitivity class and return it + template + AstActive* makeSpecialActive(FileLine* const fl) { + AstSenTree* const senTreep = new AstSenTree{fl, new AstSenItem{fl, SenItemKind{}}}; + auto* const activep = new AstActive{fl, "", senTreep}; + activep->sensesStorep(activep->sensesp()); + addActive(activep); + return activep; + } + + // Return an AstActive sensitive to the given special sensitivity class (possibly pre-created) template AstActive* getSpecialActive(FileLine* fl) { AstActive*& cachep = getSpecialActive(); - if (!cachep) { - AstSenTree* const senTreep = new AstSenTree{fl, new AstSenItem{fl, SenItemKind{}}}; - cachep = new AstActive{fl, "", senTreep}; - cachep->sensesStorep(cachep->sensesp()); - addActive(cachep); - } + if (!cachep) cachep = makeSpecialActive(fl); return cachep; } @@ -535,7 +540,9 @@ private: // Might be empty with later optimizations, so this assertion can be removed, // but for now it is guaranteed to be not empty. UASSERT_OBJ(nodep->stmtsp(), nodep, "Should not be empty"); - visitAlways(nodep, nullptr, VAlwaysKwd::ALWAYS); + // Make a new active for it, needs to be the only item under the active for V3Sched + AstActive* const activep = m_namer.makeSpecialActive(nodep->fileline()); + activep->addStmtsp(nodep->unlinkFrBack()); } void visit(AstAlwaysPublic* nodep) override { visitAlways(nodep, nodep->sensesp(), VAlwaysKwd::ALWAYS); diff --git a/src/V3Assert.cpp b/src/V3Assert.cpp index 97ced7b6b..a499aaf7b 100644 --- a/src/V3Assert.cpp +++ b/src/V3Assert.cpp @@ -426,7 +426,7 @@ private: newMonitorNumVarRefp(nodep, VAccess::READ)}}, stmtsp}; ifp->branchPred(VBranchPred::BP_UNLIKELY); - AstNode* const newp = new AstAlwaysPostponed{fl, ifp}; + AstNode* const newp = new AstAlways{fl, VAlwaysKwd::ALWAYS, nullptr, ifp}; m_modp->addStmtsp(newp); } else if (nodep->displayType() == VDisplayType::DT_STROBE) { nodep->displayType(VDisplayType::DT_DISPLAY); diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index 3ef73b94a..9a9ea6d4c 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -2649,7 +2649,7 @@ public: ASTGEN_MEMBERS_AstAlwaysPost; }; class AstAlwaysPostponed final : public AstNodeProcedure { - // Like always but postponement scheduling region + // Like always but Postponed scheduling region public: AstAlwaysPostponed(FileLine* fl, AstNode* stmtsp) diff --git a/src/V3Order.cpp b/src/V3Order.cpp index ea2313c48..80c147a40 100644 --- a/src/V3Order.cpp +++ b/src/V3Order.cpp @@ -181,7 +181,6 @@ class OrderBuildVisitor final : public VNVisitor { bool m_inClocked = false; // Underneath clocked AstActive bool m_inPre = false; // Underneath AstAssignPre bool m_inPost = false; // Underneath AstAssignPost/AstAlwaysPost - bool m_inPostponed = false; // Underneath AstAlwaysPostponed std::function m_readTriggersCombLogic; // METHODS @@ -265,19 +264,7 @@ class OrderBuildVisitor final : public VNVisitor { const bool prevCon = varscp->user2() & VU_CON; // Compute whether the variable is produced (written) here - bool gen = false; - if (!prevGen && nodep->access().isWriteOrRW()) { - gen = true; - if (m_inPostponed) { - // IEEE 1800-2017 (4.2.9) forbids any value updates in the postponed region, but - // Verilator generated trigger signals for $strobe are cleared after the - // display is executed. This is both safe to ignore (because their single read - // is in the same AstAlwaysPostponed, just prior to the clear), and is - // necessary to ignore to avoid a circular logic (UNOPTFLAT) warning. - UASSERT_OBJ(prevCon, nodep, "Should have been consumed in same process"); - gen = false; - } - } + bool gen = !prevGen && nodep->access().isWriteOrRW(); // Compute whether the value is consumed (read) here bool con = false; @@ -398,12 +385,6 @@ class OrderBuildVisitor final : public VNVisitor { iterateLogic(nodep); m_inPost = false; } - void visit(AstAlwaysPostponed* nodep) override { - UASSERT_OBJ(!m_inPostponed, nodep, "Should not nest"); - m_inPostponed = true; - iterateLogic(nodep); - m_inPostponed = false; - } void visit(AstFinal* nodep) override { // LCOV_EXCL_START nodep->v3fatalSrc("AstFinal should not need ordering"); } // LCOV_EXCL_STOP diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index ebf0dfea9..b34133c77 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -184,7 +184,11 @@ LogicClasses gatherLogicClasses(AstNetlist* netlistp) { } else if (senTreep->hasCombo()) { UASSERT_OBJ(!senTreep->sensesp()->nextp(), activep, "combinational logic with additional sensitivities"); - result.m_comb.emplace_back(scopep, activep); + if (VN_IS(activep->stmtsp(), AlwaysPostponed)) { + result.m_postponed.emplace_back(scopep, activep); + } else { + result.m_comb.emplace_back(scopep, activep); + } } else { UASSERT_OBJ(senTreep->hasClocked(), activep, "What else could it be?"); result.m_clocked.emplace_back(scopep, activep); @@ -267,6 +271,14 @@ AstCFunc* createInitial(AstNetlist* netlistp, const LogicClasses& logicClasses) return funcp; // Not splitting yet as it is not final } +AstCFunc* createPostponed(AstNetlist* netlistp, const LogicClasses& logicClasses) { + if (logicClasses.m_postponed.empty()) return nullptr; + AstCFunc* const funcp = makeTopFunction(netlistp, "_eval_postponed", /* slow: */ true); + orderSequentially(funcp, logicClasses.m_postponed); + splitCheck(funcp); + return funcp; +} + void createFinal(AstNetlist* netlistp, const LogicClasses& logicClasses) { AstCFunc* const funcp = makeTopFunction(netlistp, "_eval_final", /* slow: */ true); orderSequentially(funcp, logicClasses.m_final); @@ -738,6 +750,7 @@ void createEval(AstNetlist* netlistp, // AstVarScope* nbaTrigsp, // AstCFunc* actFuncp, // AstCFunc* nbaFuncp, // + AstCFunc* postponedFuncp, // TimingKit& timingKit // ) { FileLine* const flp = netlistp->fileline(); @@ -836,6 +849,9 @@ void createEval(AstNetlist* netlistp, // // Add the NBA eval loop funcp->addStmtsp(nbaEvalLoopp); + + // Add the Postponed eval call + if (postponedFuncp) funcp->addStmtsp(new AstCCall{flp, postponedFuncp}); } } // namespace @@ -1025,9 +1041,12 @@ void schedule(AstNetlist* netlistp) { netlistp->evalNbap(nbaFuncp); // Remember for V3LifePost if (v3Global.opt.stats()) V3Stats::statsStage("sched-create-nba"); - // Step 11: Bolt it all together to create the '_eval' function + // Step 11: Create the 'postponed' region evaluation function + auto* const postponedFuncp = createPostponed(netlistp, logicClasses); + + // Step 12: Bolt it all together to create the '_eval' function createEval(netlistp, icoLoopp, actTrig, preTrigVscp, nbaTrigVscp, actFuncp, nbaFuncp, - timingKit); + postponedFuncp, timingKit); transformForks(netlistp); diff --git a/src/V3Sched.h b/src/V3Sched.h index 876b0a510..9846ada1d 100644 --- a/src/V3Sched.h +++ b/src/V3Sched.h @@ -81,6 +81,7 @@ struct LogicClasses final { LogicByScope m_comb; // Combinational logic (logic with implicit sensitivities) LogicByScope m_clocked; // Clocked (or sequential) logic (logic with explictit sensitivities) LogicByScope m_hybrid; // Hybrid logic (combinational logic with some explicit sensitivities) + LogicByScope m_postponed; // Postponed logic ($strobe) LogicClasses() = default; VL_UNCOPYABLE(LogicClasses); diff --git a/test_regress/t/t_past_strobe.out b/test_regress/t/t_past_strobe.out index 9df6ce6ca..1f96befeb 100644 --- a/test_regress/t/t_past_strobe.out +++ b/test_regress/t/t_past_strobe.out @@ -8,4 +8,3 @@ 8 == 8, 7 == 7 9 == 9, 8 == 8 *-* All Finished *-* -10 == 10, 9 == 9 diff --git a/test_regress/t/t_past_strobe.v b/test_regress/t/t_past_strobe.v index b7e58466a..4ccbce987 100644 --- a/test_regress/t/t_past_strobe.v +++ b/test_regress/t/t_past_strobe.v @@ -38,7 +38,7 @@ module Test1( input [3:0] a, b; always @(posedge clk) begin - $strobe("%0d == %0d, %0d == %0d", a, b, $past(a), $past(b)); + if (a < 9) $strobe("%0d == %0d, %0d == %0d", a, b, $past(a), $past(b)); end endmodule diff --git a/test_regress/t/t_timing_strobe.out b/test_regress/t/t_timing_strobe.out new file mode 100644 index 000000000..01a73482a --- /dev/null +++ b/test_regress/t/t_timing_strobe.out @@ -0,0 +1,4 @@ +v = 1 +v = 2 +v = 3 +*-* All Finished *-* diff --git a/test_regress/t/t_timing_strobe.pl b/test_regress/t/t_timing_strobe.pl new file mode 100755 index 000000000..4ab3c9652 --- /dev/null +++ b/test_regress/t/t_timing_strobe.pl @@ -0,0 +1,29 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2022 by Antmicro Ltd. 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); + +if (!$Self->have_coroutines) { + skip("No coroutine support"); +} +else { + compile( + verilator_flags2 => ["--exe --main --timing"], + make_main => 0, + ); + + execute( + check_finished => 1, + expect_filename => $Self->{golden_filename}, + ); +} + +ok(1); +1; diff --git a/test_regress/t/t_timing_strobe.v b/test_regress/t/t_timing_strobe.v new file mode 100644 index 000000000..8c1057fb4 --- /dev/null +++ b/test_regress/t/t_timing_strobe.v @@ -0,0 +1,31 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + event e1; + event e2; + int v = 0; + + initial begin + #1 $strobe("v = %0d", v); ->e1; + @e2 $strobe("v = %0d", v); ->e1; + @e2 $strobe("v = %0d", v); ->e1; + @e2 $write("*-* All Finished *-*\n"); + $finish; + end + + initial begin + @e1 v = 1; #1 ->e2; + @e1 v = 2; #1 ->e2; + @e1 v = 3; #1 ->e2; + end + + initial #5 $stop; // timeout +endmodule