luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix disable-by-name process kill semantics for named blocks/tasks and add regression coverage for forked named-block disables

This commit is contained in:
Nick Brereton 2026-03-01 23:10:52 -05:00
parent 835cc63d23
commit 346f0ead2f
3 changed files with 156 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
include/verilated_std.sv
View File

@ -181,8 +181,9 @@ package std;
static task killQueue(ref process processQueue[$]);
`ifdef VERILATOR_TIMING
while (processQueue.size() > 0) begin
processQueue.pop_back().kill();
int initialSize = processQueue.size();
for (int i = 0; i < initialSize; ++i) begin
processQueue.pop_front().kill();
end
`endif
endtask

130
src/V3LinkJump.cpp
View File

@ -67,8 +67,11 @@ class LinkJumpVisitor final : public VNVisitor {
V3UniqueNames m_queueNames{
"__VprocessQueue"}; // Names for queues needed for 'disable' handling
std::unordered_map<const AstTask*, AstVar*> m_taskDisableQueues; // Per-task process queues
std::unordered_map<const AstBegin*, AstVar*> m_beginDisableQueues; // Per-begin process queues
std::unordered_map<const AstTask*, AstBegin*>
m_taskDisableBegins; // Per-task process wrappers
std::unordered_map<const AstBegin*, AstBegin*>
m_beginDisableBegins; // Per-begin process wrappers
// METHODS
// Get (and create if necessary) the JumpBlock for this statement
@ -181,6 +184,12 @@ class LinkJumpVisitor final : public VNVisitor {
return new AstStmtExpr{
fl, new AstMethodCall{fl, queueRefp, "push_back", new AstArg{fl, "", processSelfp}}};
}
static AstStmtExpr* getQueuePushProcessSelfp(FileLine* const fl, AstVar* const processQueuep) {
AstPackage* const topPkgp = v3Global.rootp()->dollarUnitPkgAddp();
AstVarRef* const queueWriteRefp
= new AstVarRef{fl, topPkgp, processQueuep, VAccess::WRITE};
return getQueuePushProcessSelfp(queueWriteRefp);
}
static AstStmtExpr* getQueueKillStmtp(FileLine* const fl, AstVar* const processQueuep) {
AstPackage* const topPkgp = v3Global.rootp()->dollarUnitPkgAddp();
AstClass* const processClassp
@ -192,6 +201,24 @@ class LinkJumpVisitor final : public VNVisitor {
killQueueCall->classOrPackagep(processClassp);
return new AstStmtExpr{fl, killQueueCall};
}
static void prependStmtsp(AstNodeFTask* 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);
}
nodep->addStmtsp(stmtp);
}
static bool directlyUnderFork(const AstNode* const nodep) {
if (nodep->backp()->nextp() == nodep) return directlyUnderFork(nodep->backp());
return VN_IS(nodep->backp(), Fork);
}
AstBegin* getOrCreateTaskDisableBeginp(AstTask* const taskp, FileLine* const fl) {
const auto it = m_taskDisableBegins.find(taskp);
if (it != m_taskDisableBegins.end()) return it->second;
@ -220,18 +247,57 @@ class LinkJumpVisitor final : public VNVisitor {
processQueuep->lifetime(VLifetime::STATIC_EXPLICIT);
topPkgp->addStmtsp(processQueuep);
AstVarRef* const queueWriteRefp
= new AstVarRef{fl, topPkgp, processQueuep, VAccess::WRITE};
AstStmtExpr* const pushCurrentProcessp = getQueuePushProcessSelfp(queueWriteRefp);
AstStmtExpr* const pushCurrentProcessp = getQueuePushProcessSelfp(fl, processQueuep);
AstBegin* const taskBodyp = getOrCreateTaskDisableBeginp(taskp, fl);
if (taskBodyp->stmtsp()) {
taskBodyp->stmtsp()->addHereThisAsNext(pushCurrentProcessp);
} else {
taskBodyp->addStmtsp(pushCurrentProcessp);
}
prependStmtsp(taskBodyp, pushCurrentProcessp);
m_taskDisableQueues.emplace(taskp, processQueuep);
return processQueuep;
}
AstBegin* getOrCreateBeginDisableBeginp(AstBegin* const beginp, FileLine* const fl) {
const auto it = m_beginDisableBegins.find(beginp);
if (it != m_beginDisableBegins.end()) return it->second;
AstBegin* const beginBodyp = new AstBegin{fl, "", nullptr, false};
if (beginp->stmtsp()) beginBodyp->addStmtsp(beginp->stmtsp()->unlinkFrBackWithNext());
AstFork* const forkp = new AstFork{fl, VJoinType::JOIN};
forkp->addForksp(beginBodyp);
beginp->addStmtsp(forkp);
m_beginDisableBegins.emplace(beginp, beginBodyp);
return beginBodyp;
}
AstVar* getOrCreateBeginDisableQueuep(AstBegin* const beginp, FileLine* const fl) {
const auto it = m_beginDisableQueues.find(beginp);
if (it != m_beginDisableQueues.end()) return it->second;
AstPackage* const topPkgp = v3Global.rootp()->dollarUnitPkgAddp();
AstClass* const processClassp
= VN_AS(getMemberp(v3Global.rootp()->stdPackagep(), "process"), Class);
AstVar* const processQueuep = new AstVar{
fl, VVarType::VAR, m_queueNames.get(beginp->name()), VFlagChildDType{},
new AstQueueDType{fl, VFlagChildDType{},
new AstClassRefDType{fl, processClassp, nullptr}, nullptr}};
processQueuep->lifetime(VLifetime::STATIC_EXPLICIT);
topPkgp->addStmtsp(processQueuep);
AstStmtExpr* const pushCurrentProcessp = getQueuePushProcessSelfp(fl, processQueuep);
AstBegin* const beginBodyp = getOrCreateBeginDisableBeginp(beginp, fl);
prependStmtsp(beginBodyp, pushCurrentProcessp);
// 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) {
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;
}
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
@ -265,10 +331,7 @@ class LinkJumpVisitor final : public VNVisitor {
if (pushCurrentProcessp->backp()) {
pushCurrentProcessp = pushCurrentProcessp->cloneTree(false);
}
if (beginp->stmtsp()) {
// There is no need to add it to empty block
beginp->stmtsp()->addHereThisAsNext(pushCurrentProcessp);
}
prependStmtsp(beginp, pushCurrentProcessp);
}
AstStmtExpr* const killStmtp = getQueueKillStmtp(fl, processQueuep);
nodep->addNextHere(killStmtp);
@ -289,12 +352,6 @@ class LinkJumpVisitor final : public VNVisitor {
}
}
}
static bool directlyUnderFork(const AstNode* const nodep) {
if (nodep->backp()->nextp() == nodep) return directlyUnderFork(nodep->backp());
if (VN_IS(nodep->backp(), Fork)) return true;
return false;
}
// VISITORS
void visit(AstNodeModule* nodep) override {
if (nodep->dead()) return;
@ -495,22 +552,37 @@ class LinkJumpVisitor final : public VNVisitor {
handleDisableOnFork(nodep, forks);
} else if (AstBegin* const beginp = VN_CAST(targetp, Begin)) {
if (existsBlockAbove(beginp->name())) {
if (beginp->user3()) {
nodep->v3warn(E_UNSUPPORTED,
"Unsupported: disabling block that contains a fork");
} else {
if (!beginp->user3()) {
// Jump to the end of the named block
AstJumpBlock* const blockp = getJumpBlock(beginp, false);
nodep->addNextHere(new AstJumpGo{nodep->fileline(), blockp});
} else {
AstVar* const processQueuep
= getOrCreateBeginDisableQueuep(beginp, nodep->fileline());
AstStmtExpr* const killStmtp
= 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
// end of that branch to prevent statements after disable from executing.
AstBegin* currentBeginp = nullptr;
for (AstNodeBlock* const blockp : vlstd::reverse_view(m_blockStack)) {
if (VN_IS(blockp, Begin)) {
currentBeginp = VN_AS(blockp, Begin);
break;
}
}
if (currentBeginp && directlyUnderFork(currentBeginp)) {
AstJumpBlock* const blockp = getJumpBlock(currentBeginp, false);
killStmtp->addNextHere(new AstJumpGo{nodep->fileline(), blockp});
}
}
} else {
if (directlyUnderFork(beginp)) {
std::vector<AstBegin*> forks{beginp};
handleDisableOnFork(nodep, forks);
} else {
nodep->v3warn(E_UNSUPPORTED, "disable isn't underneath a begin with name: '"
<< beginp->name() << "'");
}
AstVar* const processQueuep
= getOrCreateBeginDisableQueuep(beginp, nodep->fileline());
AstStmtExpr* const killStmtp = getQueueKillStmtp(nodep->fileline(), processQueuep);
nodep->addNextHere(killStmtp);
}
} else {
nodep->v3fatalSrc("Disable linked with node of unhandled type "

52
test_regress/t/t_disable_task_by_name.v
View File

@ -17,6 +17,10 @@ int self_after_disable = 0;
int par_started = 0;
int par_finished = 0;
int par_parent_continued = 0;
int named_block_fork_fail = 0;
int task_named_block_fork_fail = 0;
event named_block_fork_ev;
event task_named_block_fork_ev;
class StaticCls;
static int ticks = 0;
@ -75,6 +79,23 @@ task parent_disables_worker;
par_parent_continued = 1;
endtask
task task_named_block_fork_disable;
begin : t_named_block
fork
begin
#5;
task_named_block_fork_fail = 1;
end
begin
@task_named_block_fork_ev;
disable t_named_block;
task_named_block_fork_fail = 2;
end
join
task_named_block_fork_fail = 3;
end
endtask
class Cls;
int m_time = 0;
@ -249,6 +270,37 @@ module t;
#10;
if (y != 1) $stop;
// Disable named block containing fork from inside a fork branch
fork
begin : named_block_under_test
fork
begin
#5;
named_block_fork_fail = 1;
end
begin
@named_block_fork_ev;
disable named_block_under_test;
named_block_fork_fail = 2;
end
join
named_block_fork_fail = 3;
end
join_none
#2;
->named_block_fork_ev;
#10;
if (named_block_fork_fail != 0) $stop;
// Same case as above, but with the named block inside a task
fork
task_named_block_fork_disable();
join_none
#2;
->task_named_block_fork_ev;
#10;
if (task_named_block_fork_fail != 0) $stop;
// Disabling a task after it already finished is a no-op
finish_z();
if (z != 1) $stop;