Internals: Refactor disable process queue plumbing (#7919)

src/V3Ast.h

@@ -730,6 +730,7 @@ public:
 
     // ACCESSORS for specific types
     // Alas these can't be virtual or they break when passed a nullptr
+    bool isDisableQueuePushSelfStmt() const;
     inline bool isClassHandleValue() const;
     inline bool isNull() const;
     inline bool isZero() const;

src/V3AstNodes.cpp

@@ -108,6 +108,17 @@ int AstNodeSel::bitConst() const {
     return (constp ? constp->toSInt() : 0);
 }
 
+bool AstNode::isDisableQueuePushSelfStmt() const {
+    // Detect LinkJump-generated registration:
+    // __VprocessQueue_*.push_back(std::process::self())
+    const AstStmtExpr* const stmtExprp = VN_CAST(this, StmtExpr);
+    if (!stmtExprp) return false;
+    const AstCMethodHard* const methodp = VN_CAST(stmtExprp->exprp(), CMethodHard);
+    if (!methodp || methodp->name() != "push_back") return false;
+    const AstVarRef* const queueRefp = VN_CAST(methodp->fromp(), VarRef);
+    return queueRefp && queueRefp->varp()->processQueue();
+}
+
 void AstNodeStmt::dump(std::ostream& str) const { this->AstNode::dump(str); }
 void AstNodeStmt::dumpJson(std::ostream& str) const { dumpJsonGen(str); }
 

src/V3Fork.cpp

@@ -593,22 +593,12 @@ class ForkVisitor final : public VNVisitor {
         const AstConst* const constp = VN_CAST(delayp->lhsp(), Const);
         return constp && (constp->toUQuad() == std::numeric_limits<uint64_t>::max());
     }
-    static bool isDisableQueuePushSelfStmt(const AstNode* const nodep) {
-        // Detect LinkJump-generated registration:
-        // __VprocessQueue_*.push_back(std::process::self())
-        const AstStmtExpr* const stmtExprp = VN_CAST(nodep, StmtExpr);
-        if (!stmtExprp) return false;
-        const AstCMethodHard* const methodp = VN_CAST(stmtExprp->exprp(), CMethodHard);
-        if (!methodp || methodp->name() != "push_back") return false;
-        const AstVarRef* const queueRefp = VN_CAST(methodp->fromp(), VarRef);
-        return queueRefp && queueRefp->varp()->processQueue();
-    }
     static void moveForkSentinelAfterDisableQueuePushes(AstBegin* const beginp) {
         AstNode* const firstStmtp = beginp->stmtsp();
         if (!isForkJoinNoneSentinelDelay(firstStmtp)) return;
 
         AstNode* insertBeforep = firstStmtp->nextp();
-        while (insertBeforep && isDisableQueuePushSelfStmt(insertBeforep)) {
+        while (insertBeforep && insertBeforep->isDisableQueuePushSelfStmt()) {
             insertBeforep = insertBeforep->nextp();
         }
         if (insertBeforep == firstStmtp->nextp()) return;

src/V3LinkJump.cpp

@@ -175,15 +175,17 @@ class LinkJumpVisitor final : public VNVisitor {
             new AstMethodCall{flp, queueRefp, "push_back",
                               new AstArg{flp, "", v3Global.rootp()->stdPackageProcessSelfp(flp)}}};
     }
-    static AstStmtExpr* getQueuePushProcessSelfp(FileLine* const fl, AstVar* const processQueuep) {
+    static AstVarRef* newQueueRefp(FileLine* const fl, AstVar* const processQueuep,
+                                   const VAccess& access) {
         AstPackage* const topPkgp = v3Global.rootp()->dollarUnitPkgAddp();
-        AstVarRef* const queueWriteRefp
+        return new AstVarRef{fl, topPkgp, processQueuep, access};
-            = new AstVarRef{fl, topPkgp, processQueuep, VAccess::WRITE};
+    }
+    static AstStmtExpr* getQueuePushProcessSelfp(FileLine* const fl, AstVar* const processQueuep) {
+        AstVarRef* const queueWriteRefp = newQueueRefp(fl, processQueuep, VAccess::WRITE);
         return getQueuePushProcessSelfp(queueWriteRefp);
     }
     static AstStmtExpr* getQueueKillStmtp(FileLine* const fl, AstVar* const processQueuep) {
-        AstPackage* const topPkgp = v3Global.rootp()->dollarUnitPkgAddp();
+        AstVarRef* const queueRefp = newQueueRefp(fl, processQueuep, VAccess::READWRITE);
-        AstVarRef* const queueRefp = new AstVarRef{fl, topPkgp, processQueuep, VAccess::READWRITE};
         AstTaskRef* killQueueCall = nullptr;
         for (AstNode* itemp = v3Global.rootp()->stdPackageProcessp()->stmtsp(); itemp;
              itemp = itemp->nextp()) {
@@ -197,14 +199,7 @@ class LinkJumpVisitor final : public VNVisitor {
         killQueueCall->classOrPackagep(v3Global.rootp()->stdPackageProcessp());
         return new AstStmtExpr{fl, killQueueCall};
     }
-    static void prependStmtsp(AstNodeFTask* const nodep, AstNode* const stmtp) {
+    static void prependStmtsp(AstBegin* const nodep, AstNode* const stmtp) {
-        if (AstNode* const origStmtsp = nodep->stmtsp()) {
-            origStmtsp->unlinkFrBackWithNext();
-            stmtp->addNext(origStmtsp);
-        }
-        nodep->addStmtsp(stmtp);
-    }
-    static void prependStmtsp(AstNodeBlock* const nodep, AstNode* const stmtp) {
         if (AstNode* const origStmtsp = nodep->stmtsp()) {
             origStmtsp->unlinkFrBackWithNext();
             stmtp->addNext(origStmtsp);
@@ -232,17 +227,21 @@ class LinkJumpVisitor final : public VNVisitor {
         m_taskDisableBegins.emplace(taskp, taskBodyp);
         return taskBodyp;
     }
-    AstVar* getProcessQueuep(AstNode* const nodep, FileLine* const fl) {
+    AstVar* newProcessQueuep(AstNode* const nodep, FileLine* const fl, const VVarType& varType) {
-        AstPackage* const topPkgp = v3Global.rootp()->dollarUnitPkgAddp();
         AstVar* const processQueuep = new AstVar{
-            fl, VVarType::VAR, m_queueNames.get(nodep->name()), VFlagChildDType{},
+            fl, varType, m_queueNames.get(nodep->name()), VFlagChildDType{},
             new AstQueueDType{
                 fl, VFlagChildDType{},
                 new AstClassRefDType{fl, v3Global.rootp()->stdPackageProcessp(), nullptr},
                 nullptr}};
-        processQueuep->lifetime(VLifetime::STATIC_EXPLICIT);
         processQueuep->processQueue(true);
         processQueuep->setIgnoreSchedWrite();
+        return processQueuep;
+    }
+    AstVar* getProcessQueuep(AstNode* const nodep, FileLine* const fl) {
+        AstPackage* const topPkgp = v3Global.rootp()->dollarUnitPkgAddp();
+        AstVar* const processQueuep = newProcessQueuep(nodep, fl, VVarType::VAR);
+        processQueuep->lifetime(VLifetime::STATIC_EXPLICIT);
         topPkgp->addStmtsp(processQueuep);
         return processQueuep;
     }
@@ -274,6 +273,20 @@ class LinkJumpVisitor final : public VNVisitor {
         m_beginDisableBegins.emplace(beginp, beginBodyp);
         return beginBodyp;
     }
+    void prependForkBranchQueuePushes(AstFork* const forkp, AstVar* const processQueuep,
+                                      FileLine* const fl, bool needProcess) {
+        for (AstBegin* branchp = forkp->forksp(); branchp;
+             branchp = VN_AS(branchp->nextp(), Begin)) {
+            if (needProcess) branchp->setNeedProcess();
+            prependStmtsp(branchp, getQueuePushProcessSelfp(fl, processQueuep));
+        }
+    }
+    void prependNestedForkBranchQueuePushes(AstNode* const nodep, AstVar* const processQueuep,
+                                            FileLine* const fl, bool needProcess) {
+        nodep->foreach([&](AstFork* const forkp) {
+            prependForkBranchQueuePushes(forkp, processQueuep, fl, needProcess);
+        });
+    }
     AstVar* getOrCreateBeginDisableQueuep(AstBegin* const beginp, FileLine* const fl) {
         const auto it = m_beginDisableQueues.find(beginp);
         if (it != m_beginDisableQueues.end()) return it->second;
@@ -285,17 +298,19 @@ class LinkJumpVisitor final : public VNVisitor {
 
         // Named-block disable must also terminate detached descendants created by forks
         // under the block, so track each fork branch process in the same queue.
-        beginBodyp->foreach([&](AstFork* const forkp) {
+        prependNestedForkBranchQueuePushes(beginBodyp, processQueuep, fl, false);
-            for (AstBegin* branchp = forkp->forksp(); branchp;
-                 branchp = VN_AS(branchp->nextp(), Begin)) {
-                AstStmtExpr* const pushBranchProcessp
-                    = getQueuePushProcessSelfp(fl, processQueuep);
-                prependStmtsp(branchp, pushBranchProcessp);
-            }
-        });
         m_beginDisableQueues.emplace(beginp, processQueuep);
         return processQueuep;
     }
+    AstStmtExpr* insertKillStmtp(AstDisable* const nodep, AstVar* const processQueuep) {
+        AstStmtExpr* const killStmtp = getQueueKillStmtp(nodep->fileline(), processQueuep);
+        nodep->addNextHere(killStmtp);
+        return killStmtp;
+    }
+    void addJumpAfterKill(AstStmtExpr* const killStmtp, AstNode* const targetp) {
+        AstJumpBlock* const jmpBlockp = getJumpBlock(targetp, false);
+        killStmtp->addNextHere(new AstJumpGo{killStmtp->fileline(), jmpBlockp});
+    }
     void handleDisableOnFork(AstDisable* const nodep, const std::vector<AstBegin*>& forks) {
         // The support utilizes the process::kill()` method. For each `disable` a queue of
         // processes is declared. At the beginning of each fork that can be disabled, its process
@@ -323,8 +338,7 @@ class LinkJumpVisitor final : public VNVisitor {
             }
             prependStmtsp(beginp, pushCurrentProcessp);
         }
-        AstStmtExpr* const killStmtp = getQueueKillStmtp(fl, processQueuep);
+        AstStmtExpr* const killStmtp = insertKillStmtp(nodep, processQueuep);
-        nodep->addNextHere(killStmtp);
 
         // 'process::kill' does not immediately kill the current process
         // executing the disable statement (because it's in the running state).
@@ -334,8 +348,7 @@ class LinkJumpVisitor final : public VNVisitor {
             AstNodeBlock* forkBranchp = nullptr;
             for (AstNodeBlock* const blockp : vlstd::reverse_view(m_blockStack)) {
                 if (blockp == targetp) {
-                    AstJumpBlock* const jmpBlockp = getJumpBlock(VN_AS(forkBranchp, Begin), false);
+                    addJumpAfterKill(killStmtp, VN_AS(forkBranchp, Begin));
-                    killStmtp->addNextHere(new AstJumpGo{fl, jmpBlockp});
                     break;
                 }
                 forkBranchp = blockp;
@@ -522,8 +535,7 @@ class LinkJumpVisitor final : public VNVisitor {
         }
         if (AstTask* const taskp = VN_CAST(targetp, Task)) {
             AstVar* const processQueuep = getOrCreateTaskDisableQueuep(taskp, nodep->fileline());
-            AstStmtExpr* const killStmtp = getQueueKillStmtp(nodep->fileline(), processQueuep);
+            AstStmtExpr* const killStmtp = insertKillStmtp(nodep, processQueuep);
-            nodep->addNextHere(killStmtp);
 
             // process::kill does not terminate the currently running process immediately.
             // If we disable the current task by name from inside itself, jump to its end.
@@ -531,8 +543,7 @@ class LinkJumpVisitor final : public VNVisitor {
                 AstNode* jumpTargetp = taskp;
                 const auto it = m_taskDisableBegins.find(taskp);
                 if (it != m_taskDisableBegins.end()) jumpTargetp = it->second;
-                AstJumpBlock* const blockp = getJumpBlock(jumpTargetp, false);
+                addJumpAfterKill(killStmtp, jumpTargetp);
-                killStmtp->addNextHere(new AstJumpGo{nodep->fileline(), blockp});
             }
         } else if (AstFork* const forkp = VN_CAST(targetp, Fork)) {
             std::vector<AstBegin*> forks;
@@ -549,9 +560,7 @@ class LinkJumpVisitor final : public VNVisitor {
                 } else {
                     AstVar* const processQueuep
                         = getOrCreateBeginDisableQueuep(beginp, nodep->fileline());
-                    AstStmtExpr* const killStmtp
+                    AstStmtExpr* const killStmtp = insertKillStmtp(nodep, processQueuep);
-                        = getQueueKillStmtp(nodep->fileline(), processQueuep);
-                    nodep->addNextHere(killStmtp);
 
                     // process::kill does not terminate the currently running process immediately.
                     // If disable executes inside a fork branch of this named block, jump to the
@@ -564,15 +573,13 @@ class LinkJumpVisitor final : public VNVisitor {
                         }
                     }
                     if (currentBeginp && directlyUnderFork(currentBeginp)) {
-                        AstJumpBlock* const blockp = getJumpBlock(currentBeginp, false);
+                        addJumpAfterKill(killStmtp, currentBeginp);
-                        killStmtp->addNextHere(new AstJumpGo{nodep->fileline(), blockp});
                     }
                 }
             } else {
                 AstVar* const processQueuep
                     = getOrCreateBeginDisableQueuep(beginp, nodep->fileline());
-                AstStmtExpr* const killStmtp = getQueueKillStmtp(nodep->fileline(), processQueuep);
+                insertKillStmtp(nodep, processQueuep);
-                nodep->addNextHere(killStmtp);
             }
         } else {
             nodep->v3fatalSrc("Disable linked with node of unhandled type "

src/V3Timing.cpp

@@ -752,12 +752,7 @@ class TimingControlVisitor final : public VNVisitor {
     static bool hasDisableQueuePushSelfPrefix(const AstBegin* const beginp) {
         // LinkJump prepends disable-by-name registration as:
         //   __VprocessQueue_*.push_back(std::process::self())
-        const AstStmtExpr* const stmtExprp = VN_CAST(beginp->stmtsp(), StmtExpr);
+        return beginp->stmtsp() && beginp->stmtsp()->isDisableQueuePushSelfStmt();
-        if (!stmtExprp) return false;
-        const AstCMethodHard* const methodp = VN_CAST(stmtExprp->exprp(), CMethodHard);
-        if (!methodp || methodp->name() != "push_back") return false;
-        const AstVarRef* const queueRefp = VN_CAST(methodp->fromp(), VarRef);
-        return queueRefp && queueRefp->varp()->processQueue();
     }
     // Register a callback so killing a process-backed fork branch decrements the join counter
     void addForkOnKill(AstBegin* const beginp, AstVarScope* const forkVscp) const {