Fix V3Life eliminating assignments across timing controls (#6593) (#6596)

For both JumpBlock and Loop, record if they contain a timing control and
do not eliminate assignments across them if so.

Fixes #6593
This commit is contained in:
Geza Lore 2025-10-25 21:59:21 +02:00 committed by GitHub
parent 0ead54b17e
commit 7225c902ee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 113 additions and 16 deletions

View File

@ -278,6 +278,7 @@ public:
class LifeVisitor final : public VNVisitor { class LifeVisitor final : public VNVisitor {
// STATE // STATE
LifeState* const m_statep; // Current state LifeState* const m_statep; // Current state
bool m_containsTiming = false; // Statement contains timing control
bool m_sideEffect = false; // Side effects discovered in assign RHS bool m_sideEffect = false; // Side effects discovered in assign RHS
bool m_noopt = false; // Disable optimization of variables in this block bool m_noopt = false; // Disable optimization of variables in this block
bool m_tracingCall = false; // Iterating into a CCall to a CFunc bool m_tracingCall = false; // Iterating into a CCall to a CFunc
@ -308,6 +309,7 @@ class LifeVisitor final : public VNVisitor {
if (nodep->isTimingControl() || VN_IS(nodep, AssignForce)) { if (nodep->isTimingControl() || VN_IS(nodep, AssignForce)) {
// V3Life doesn't understand time sense nor force assigns - don't optimize // V3Life doesn't understand time sense nor force assigns - don't optimize
setNoopt(); setNoopt();
if (nodep->isTimingControl()) m_containsTiming = true;
iterateChildren(nodep); iterateChildren(nodep);
return; return;
} }
@ -345,6 +347,7 @@ class LifeVisitor final : public VNVisitor {
if (nodep->isTimingControl()) { if (nodep->isTimingControl()) {
// Don't optimize // Don't optimize
setNoopt(); setNoopt();
m_containsTiming = true;
} }
// Don't treat as normal assign // Don't treat as normal assign
iterateChildren(nodep); iterateChildren(nodep);
@ -379,37 +382,37 @@ class LifeVisitor final : public VNVisitor {
void visit(AstLoop* nodep) override { void visit(AstLoop* nodep) override {
// Similar problem to AstJumpBlock, don't optimize loop bodies - most are unrolled // Similar problem to AstJumpBlock, don't optimize loop bodies - most are unrolled
UASSERT_OBJ(!nodep->contsp(), nodep, "'contsp' only used before LinkJump"); UASSERT_OBJ(!nodep->contsp(), nodep, "'contsp' only used before LinkJump");
LifeBlock* const prevLifep = m_lifep; VL_RESTORER(m_containsTiming);
LifeBlock* const bodyLifep = new LifeBlock{prevLifep, m_statep};
{ {
VL_RESTORER(m_noopt); VL_RESTORER(m_noopt);
m_lifep = bodyLifep; VL_RESTORER(m_lifep);
m_lifep = new LifeBlock{m_lifep, m_statep};
setNoopt(); setNoopt();
iterateAndNextNull(nodep->stmtsp()); iterateAndNextNull(nodep->stmtsp());
m_lifep = prevLifep;
}
UINFO(4, " joinloop"); UINFO(4, " joinloop");
// For the next assignments, clear any variables that were read or written in the block // For the next assignments, clear any variables that were read or written in the block
bodyLifep->lifeToAbove(); m_lifep->lifeToAbove();
VL_DO_DANGLING(delete bodyLifep, bodyLifep); VL_DO_DANGLING(delete m_lifep, m_lifep);
}
if (m_containsTiming) setNoopt();
} }
void visit(AstJumpBlock* nodep) override { void visit(AstJumpBlock* nodep) override {
// As with Loop's we can't predict if a JumpGo will kill us or not // As with Loop's we can't predict if a JumpGo will kill us or not
// It's worse though as an IF(..., JUMPGO) may change the control flow. // It's worse though as an IF(..., JUMPGO) may change the control flow.
// Just don't optimize blocks with labels; they're rare - so far. // Just don't optimize blocks with labels; they're rare - so far.
LifeBlock* const prevLifep = m_lifep; VL_RESTORER(m_containsTiming);
LifeBlock* const bodyLifep = new LifeBlock{prevLifep, m_statep};
{ {
VL_RESTORER(m_noopt); VL_RESTORER(m_noopt);
m_lifep = bodyLifep; VL_RESTORER(m_lifep);
m_lifep = new LifeBlock{m_lifep, m_statep};
setNoopt(); setNoopt();
iterateAndNextNull(nodep->stmtsp()); iterateAndNextNull(nodep->stmtsp());
m_lifep = prevLifep;
}
UINFO(4, " joinjump"); UINFO(4, " joinjump");
// For the next assignments, clear any variables that were read or written in the block // For the next assignments, clear any variables that were read or written in the block
bodyLifep->lifeToAbove(); m_lifep->lifeToAbove();
VL_DO_DANGLING(delete bodyLifep, bodyLifep); VL_DO_DANGLING(delete m_lifep, m_lifep);
}
if (m_containsTiming) setNoopt();
} }
void visit(AstNodeCCall* nodep) override { void visit(AstNodeCCall* nodep) override {
// UINFO(4, " CCALL " << nodep); // UINFO(4, " CCALL " << nodep);
@ -447,6 +450,7 @@ class LifeVisitor final : public VNVisitor {
if (nodep->isTimingControl()) { if (nodep->isTimingControl()) {
// V3Life doesn't understand time sense - don't optimize // V3Life doesn't understand time sense - don't optimize
setNoopt(); setNoopt();
m_containsTiming = true;
} }
iterateChildren(nodep); iterateChildren(nodep);
} }

View File

@ -0,0 +1,18 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2025 by Wilson Snyder. 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
import vltest_bootstrap
test.scenarios('vlt_all')
test.compile(verilator_flags2=["--timing", "--binary"])
test.execute()
test.passes()

View File

@ -0,0 +1,75 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2025 by Wilson Snyder.
// SPDX-License-Identifier: CC0-1.0
// verilog_format: off
`define stop $stop
`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%t %%Error: %s:%0d: got=%0d exp=%0d\n", $time, `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
// verilog_format: on
module t;
bit clk = 0;
always #10 clk = ~clk;
// Case A
bit [3:0] cnt_A = 0;
task task_A();
$display("%t %m enter", $time);
cnt_A = 0;
repeat (2) @(posedge clk);
for (int i = 0; i < 8; i++) begin : loop
$display("%t %m inc %d", $time, i);
++cnt_A;
repeat (2) @(posedge clk);
end
$display("%t %m inc final", $time);
++cnt_A;
repeat (2) @(posedge clk);
$display("%t %m exit", $time);
endtask
// Case B - Same, with 'repeat' unrolled
bit [3:0] cnt_B = 0;
task task_B();
$display("%t %m enter", $time);
cnt_B = 0;
@(posedge clk);
@(posedge clk);
for (int i = 0; i < 8; i++) begin : loop
++cnt_B;
@(posedge clk);
@(posedge clk);
end
$display("%t %m inc final", $time);
++cnt_B;
@(posedge clk);
@(posedge clk);
$display("%t %m exit", $time);
endtask
initial begin
task_A();
$display("%t taks_A return 1", $time);
task_A();
$display("%t taks_A return 2", $time);
end
initial begin
task_B();
$display("%t taks_B return 1", $time);
task_B();
$display("%t taks_B return 2", $time);
#100;
$write("*-* All Finished *-*\n");
$finish;
end
always_ff @(posedge clk) begin
#1 `checkd(cnt_A, cnt_B);
end
endmodule