Fix invalidating variable caches in SenExprBulider (#5834) (#5835)

src/V3Sched.cpp

@@ -737,16 +737,14 @@ const TriggerKit createTriggers(AstNetlist* netlistp, AstCFunc* const initFuncp,
         map[senTreep] = trigpSenp;
     }
 
+    // Get the SenExprBuilder results
+    const SenExprBuilder::Results senResults = senExprBuilder.getAndClearResults();
+
     // Add the init and update statements
-    for (AstNodeStmt* const nodep : senExprBuilder.getAndClearInits()) {
+    for (AstNodeStmt* const nodep : senResults.m_inits) initFuncp->addStmtsp(nodep);
-        initFuncp->addStmtsp(nodep);
+    for (AstNodeStmt* const nodep : senResults.m_postUpdates) funcp->addStmtsp(nodep);
-    }
+    if (!senResults.m_preUpdates.empty()) {
-    for (AstNodeStmt* const nodep : senExprBuilder.getAndClearPostUpdates()) {
+        for (AstNodeStmt* const nodep : vlstd::reverse_view(senResults.m_preUpdates)) {
-        funcp->addStmtsp(nodep);
-    }
-    const auto& preUpdates = senExprBuilder.getAndClearPreUpdates();
-    if (!preUpdates.empty()) {
-        for (AstNodeStmt* const nodep : vlstd::reverse_view(preUpdates)) {
             UASSERT_OBJ(funcp->stmtsp(), funcp,
                         "No statements in trigger eval function, but there are pre updates");
             funcp->stmtsp()->addHereThisAsNext(nodep);

src/V3SenExprBuilder.h

@@ -28,12 +28,19 @@
 // AstSenTree have triggered
 
 class SenExprBuilder final {
+public:
+    // TYPES
+    struct Results final {
+        std::vector<AstNodeStmt*> m_inits;  // Initialization statements for previous values
+        std::vector<AstNodeStmt*> m_preUpdates;  // Pre update assignments
+        std::vector<AstNodeStmt*> m_postUpdates;  // Post update assignments
+    };
+
+private:
     // STATE
     AstScope* const m_scopep;  // The scope
 
-    std::vector<AstNodeStmt*> m_inits;  // Initialization statements for previous values
+    Results m_results;  // The builder result
-    std::vector<AstNodeStmt*> m_preUpdates;  // Pre update assignments
-    std::vector<AstNodeStmt*> m_postUpdates;  // Post update assignments
 
     std::unordered_map<VNRef<AstNode>, AstVarScope*> m_prev;  // The 'previous value' signals
     std::unordered_map<VNRef<AstNode>, AstVarScope*> m_curr;  // The 'current value' signals
@@ -90,8 +97,8 @@ class SenExprBuilder final {
 
         // Add pre update if it does not exist yet in this round
         if (m_hasPreUpdate.emplace(*currp).second) {
-            m_preUpdates.push_back(new AstAssign{flp, new AstVarRef{flp, currp, VAccess::WRITE},
+            m_results.m_preUpdates.push_back(new AstAssign{
-                                                 exprp->cloneTree(false)});
+                flp, new AstVarRef{flp, currp, VAccess::WRITE}, exprp->cloneTree(false)});
         }
         return new AstVarRef{flp, currp, VAccess::READ};
     }
@@ -109,7 +116,7 @@ class SenExprBuilder final {
             // Add the initializer init
             AstAssign* const initp = new AstAssign{flp, new AstVarRef{flp, prevp, VAccess::WRITE},
                                                    exprp->cloneTree(false)};
-            m_inits.push_back(initp);
+            m_results.m_inits.push_back(initp);
         }
 
         AstVarScope* const prevp = pair.first->second;
@@ -128,9 +135,9 @@ class SenExprBuilder final {
             if (VN_IS(exprp->dtypep()->skipRefp(), UnpackArrayDType)) {
                 AstCMethodHard* const cmhp = new AstCMethodHard{flp, wrPrev(), "assign", rdCurr()};
                 cmhp->dtypeSetVoid();
-                m_postUpdates.push_back(cmhp->makeStmt());
+                m_results.m_postUpdates.push_back(cmhp->makeStmt());
             } else {
-                m_postUpdates.push_back(new AstAssign{flp, wrPrev(), rdCurr()});
+                m_results.m_postUpdates.push_back(new AstAssign{flp, wrPrev(), rdCurr()});
             }
         }
 
@@ -172,7 +179,7 @@ class SenExprBuilder final {
                 // No need to check if the event was fired, we need the flag clear regardless
                 AstCMethodHard* const clearp = new AstCMethodHard{flp, currp(), "clearFired"};
                 clearp->dtypeSetVoid();
-                m_postUpdates.push_back(clearp->makeStmt());
+                m_results.m_postUpdates.push_back(clearp->makeStmt());
             }
 
             // Get 'fired' state
@@ -215,16 +222,12 @@ public:
         return {resultp, firedAtInitialization};
     }
 
-    std::vector<AstNodeStmt*> getAndClearInits() { return std::move(m_inits); }
+    Results getAndClearResults() {
-
+        m_curr.clear();
-    std::vector<AstNodeStmt*> getAndClearPreUpdates() {
+        m_prev.clear();
         m_hasPreUpdate.clear();
-        return std::move(m_preUpdates);
-    }
-
-    std::vector<AstNodeStmt*> getAndClearPostUpdates() {
         m_hasPostUpdate.clear();
-        return std::move(m_postUpdates);
+        return std::move(m_results);
     }
 
     // CONSTRUCTOR

src/V3Timing.cpp

@@ -933,8 +933,10 @@ class TimingControlVisitor final : public VNVisitor {
             UASSERT_OBJ(m_senExprBuilderp, nodep, "No SenExprBuilder for this scope");
             auto* const assignp = new AstAssign{flp, new AstVarRef{flp, trigvscp, VAccess::WRITE},
                                                 m_senExprBuilderp->build(sensesp).first};
+            // Get the SenExprBuilder results
+            const SenExprBuilder::Results senResults = m_senExprBuilderp->getAndClearResults();
             // Put all and inits before the trigger eval loop
-            for (AstNodeStmt* const stmtp : m_senExprBuilderp->getAndClearInits()) {
+            for (AstNodeStmt* const stmtp : senResults.m_inits) {
                 nodep->addHereThisAsNext(stmtp);
             }
             // Create the trigger eval loop, which will await the evaluation step and check the
@@ -943,9 +945,7 @@ class TimingControlVisitor final : public VNVisitor {
                 flp, new AstLogNot{flp, new AstVarRef{flp, trigvscp, VAccess::READ}},
                 awaitEvalp->makeStmt()};
             // Put pre updates before the trigger check and assignment
-            for (AstNodeStmt* const stmtp : m_senExprBuilderp->getAndClearPreUpdates()) {
+            for (AstNodeStmt* const stmtp : senResults.m_preUpdates) loopp->addStmtsp(stmtp);
-                loopp->addStmtsp(stmtp);
-            }
             // Then the trigger check and assignment
             loopp->addStmtsp(assignp);
             // Let the dynamic trigger scheduler know if this trigger was set
@@ -963,9 +963,7 @@ class TimingControlVisitor final : public VNVisitor {
                 loopp->addStmtsp(awaitPostUpdatep->makeStmt());
             }
             // Put the post updates at the end of the loop
-            for (AstNodeStmt* const stmtp : m_senExprBuilderp->getAndClearPostUpdates()) {
+            for (AstNodeStmt* const stmtp : senResults.m_postUpdates) loopp->addStmtsp(stmtp);
-                loopp->addStmtsp(stmtp);
-            }
             // Finally, await the resumption step in 'act'
             AstCAwait* const awaitResumep = awaitEvalp->cloneTree(false);
             VN_AS(awaitResumep->exprp(), CMethodHard)->name("resumption");