Move Postponed logic after the eval loop (#3673)

Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>

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 <typename SenItemKind>
+    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 <typename SenItemKind>
     AstActive* getSpecialActive(FileLine* fl) {
         AstActive*& cachep = getSpecialActive<SenItemKind>();
-        if (!cachep) {
+        if (!cachep) cachep = makeSpecialActive<SenItemKind>(fl);
-            AstSenTree* const senTreep = new AstSenTree{fl, new AstSenItem{fl, SenItemKind{}}};
-            cachep = new AstActive{fl, "", senTreep};
-            cachep->sensesStorep(cachep->sensesp());
-            addActive(cachep);
-        }
         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<AstSenItem::Combo>(nodep->fileline());
+        activep->addStmtsp(nodep->unlinkFrBack());
     }
     void visit(AstAlwaysPublic* nodep) override {
         visitAlways(nodep, nodep->sensesp(), VAlwaysKwd::ALWAYS);

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);

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)

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<bool(const AstVarScope*)> 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;
+        bool gen = !prevGen && nodep->access().isWriteOrRW();
-        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;
-            }
-        }
 
         // 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

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);
 

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);

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

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

test_regress/t/t_timing_strobe.out

@@ -0,0 +1,4 @@
+v = 1
+v = 2
+v = 3
+*-* All Finished *-*

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;

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