Skip to content

Commit

Permalink
Fix DFG removing forceable signals (verilator#4942)
Browse files Browse the repository at this point in the history 
DFG could remove forceable signals by replacing them with their
in-design driver. This is a bit of a pain to prevent, and ideally the
forcing transform should happen before DFG, but implementing it there is
a pain due to having to rewrite ports based on direction.  This is an
attempted fix in DFG. More cases might remain.
  • Loading branch information
@gezalore
gezalore authored Mar 3, 2024
1 parent 0fbd431 commit 745605e
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 61 deletions.
13 changes: 8 additions & 5 deletions src/V3DfgAstToDfg.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,11 +439,14 @@ class AstToDfgVisitor final : public VNVisitor {
if (nodep->isSc()) return;
// No need to (and in fact cannot) handle variables with unsupported dtypes
if (!DfgVertex::isSupportedDType(nodep->dtypep())) return;
// Mark ports as having external references
if (nodep->isIO()) getNet(nodep)->setHasExtRefs();
// Mark variables that are the target of a hierarchical reference
// (these flags were set up in DataflowPrepVisitor)
if (nodep->user2()) getNet(nodep)->setHasExtRefs();

// Mark variables with external references
if (nodep->isIO() // Ports
|| nodep->user2() // Target of a hierarchical reference
|| nodep->isForceable() // Forceable
) {
getNet(nodep)->setHasExtRefs();
}
}

void visit(AstAssignW* nodep) override {
Expand Down
9 changes: 7 additions & 2 deletions src/V3DfgPasses.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,14 @@ void V3DfgPasses::inlineVars(const DfgGraph& dfg) {
if (varp->hasSinks() && varp->isDrivenFullyByDfg() && !varp->varp()->isSc()) {
DfgVertex* const driverp = varp->source(0);

// If driven from a SystemC variable, don't inline this variable
// We must keep the original driver in certain cases, when swapping them would
// not be functionally or technically (implementation reasons) equivalent
if (DfgVertexVar* const driverVarp = driverp->cast<DfgVarPacked>()) {
if (driverVarp->varp()->isSc()) continue;
const AstVar* const varp = driverVarp->varp();
// If driven from a SystemC variable
if (varp->isSc()) continue;
// If the variable is forceable
if (varp->isForceable()) continue;
}

varp->forEachSinkEdge([=](DfgEdge& edge) {
Expand Down
4 changes: 3 additions & 1 deletion src/V3DfgVertices.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ class DfgVarPacked final : public DfgVertexVar {
: DfgVertexVar{dfg, dfgType(), varp, 1u} {}
ASTGEN_MEMBERS_DfgVarPacked;

bool isDrivenFullyByDfg() const { return arity() == 1 && source(0)->dtypep() == dtypep(); }
bool isDrivenFullyByDfg() const {
return arity() == 1 && source(0)->dtypep() == dtypep() && !varp()->isForceable();
}

void addDriver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp) {
m_driverData.emplace_back(flp, lsb);
Expand Down
35 changes: 19 additions & 16 deletions test_regress/t/t_forceable_net.v
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,41 +32,44 @@ module t (
assign net_1 = ~cyc[0];
assign net_8 = ~cyc[1 +: 8];

wire obs_1 = net_1;
wire [7:0] obs_8 = net_8;

always @ (posedge clk) begin
$display("%d: %x %x", cyc, net_8, net_1);
$display("%d: %x %x", cyc, obs_8, obs_1);

if (!rst) begin
case (cyc)
3: begin
`checkh (net_1, 0);
`checkh (net_8, ~cyc[1 +: 8]);
`checkh (obs_1, 0);
`checkh (obs_8, ~cyc[1 +: 8]);
end
4: begin
`checkh (net_1, 0);
`checkh (net_8, 8'h5f);
`checkh (obs_1, 0);
`checkh (obs_8, 8'h5f);
end
5: begin
`checkh (net_1, 1);
`checkh (net_8, 8'h5f);
`checkh (obs_1, 1);
`checkh (obs_8, 8'h5f);
end
6, 7: begin
`checkh (net_1, 1);
`checkh (net_8, 8'hf5);
`checkh (obs_1, 1);
`checkh (obs_8, 8'hf5);
end
8: begin
`checkh (net_1, ~cyc[0]);
`checkh (net_8, 8'hf5);
`checkh (obs_1, ~cyc[0]);
`checkh (obs_8, 8'hf5);
end
10, 11: begin
`checkh (net_1, 1);
`checkh (net_8, 8'h5a);
`checkh (obs_1, 1);
`checkh (obs_8, 8'h5a);
end
12, 13: begin
`checkh (net_1, 0);
`checkh (net_8, 8'ha5);
`checkh (obs_1, 0);
`checkh (obs_8, 8'ha5);
end
default: begin
`checkh ({net_8, net_1}, ~cyc[0 +: 9]);
`checkh ({obs_8, obs_1}, ~cyc[0 +: 9]);
end
endcase
end
Expand Down
3 changes: 2 additions & 1 deletion test_regress/t/t_forceable_net_trace.vcd
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
$version Generated by VerilatedVcd $end
$timescale 1ps $end

$scope module top $end
$var wire 1 # clk $end
$var wire 1 $ rst $end
Expand All @@ -11,6 +10,8 @@ $timescale 1ps $end
$var wire 32 % cyc [31:0] $end
$var wire 1 & net_1 $end
$var wire 8 ' net_8 [7:0] $end
$var wire 1 & obs_1 $end
$var wire 8 ' obs_8 [7:0] $end
$upscope $end
$upscope $end
$enddefinitions $end
Expand Down
66 changes: 39 additions & 27 deletions test_regress/t/t_forceable_var.v
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,19 @@ module t (
end
end

reg tmp_1;
reg [7:0] tmp_8;

always @(posedge clk) begin
if (rst) begin
tmp_1 <= 0;
tmp_8 <= 0;
end else begin
tmp_1 <= cyc[0];
tmp_8 <= cyc[1 +: 8];
end
end

`ifdef CMT
reg var_1 /* verilator forceable */;
reg [7:0] var_8 /* verilator forceable */;
Expand All @@ -29,55 +42,54 @@ module t (
reg [7:0] var_8;
`endif

always @(posedge clk) begin
if (rst) begin
var_1 <= 0;
var_8 <= 0;
end else begin
var_1 <= cyc[0];
var_8 <= cyc[1 +: 8];
end
end
always @* var_1 = tmp_1;
always @* var_8 = tmp_8;

reg obs_1;
reg [7:0] obs_8;

always @* obs_1 = var_1;
always @* obs_8 = var_8;

always @ (posedge clk) begin
$display("%d: %x %x", cyc, var_8, var_1);
$display("%d: %x %x", cyc, obs_8, obs_1);

if (!rst) begin
case (cyc)
0: begin // Reset values
`checkh (var_1, 0);
`checkh (var_8, 0);
`checkh (obs_1, 0);
`checkh (obs_8, 0);
end
13: begin
`checkh (var_1, 1);
`checkh ({1'b0, var_8}, (cyc[0 +: 9] - 1) >> 1);
`checkh (obs_1, 1);
`checkh ({1'b0, obs_8}, (cyc[0 +: 9] - 1) >> 1);
end
14: begin
`checkh (var_1, 1);
`checkh (var_8, 8'hf5);
`checkh (obs_1, 1);
`checkh (obs_8, 8'hf5);
end
15: begin
`checkh (var_1, 0);
`checkh (var_8, 8'hf5);
`checkh (obs_1, 0);
`checkh (obs_8, 8'hf5);
end
16, 17: begin
`checkh (var_1, 0);
`checkh (var_8, 8'h5f);
`checkh (obs_1, 0);
`checkh (obs_8, 8'h5f);
end
18: begin
`checkh (var_1, ~cyc[0]);
`checkh (var_8, 8'h5f);
`checkh (obs_1, ~cyc[0]);
`checkh (obs_8, 8'h5f);
end
20, 21: begin
`checkh (var_1, 1);
`checkh (var_8, 8'h5a);
`checkh (obs_1, 1);
`checkh (obs_8, 8'h5a);
end
22, 23: begin
`checkh (var_1, 0);
`checkh (var_8, 8'ha5);
`checkh (obs_1, 0);
`checkh (obs_8, 8'ha5);
end
default: begin
`checkh ({var_8, var_1}, cyc[0 +: 9] - 1);
`checkh ({obs_8, obs_1}, cyc[0 +: 9] - 1);
end
endcase
end
Expand Down
Loading

0 comments on commit 745605e

Please sign in to comment.