github/verilator
1
0
Fork 1
mirror of https://github.com/verilator/verilator.git synced 2025-07-31 07:56:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

Dfg: resolve multi-driven signal ranges

Browse Source 
In order to avoid unexpected breakage on multi-driven variables, we
resolve in DFG construction by using only the first driver encountered.
Also issues the MULTIDRIVEN error for these signals.
This commit is contained in:
Geza Lore 2022-11-12 14:14:32 +00:00
parent 8291d8bcc1
commit eaf09ba0e7
8 changed files with 211 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

142
src/V3DfgAstToDfg.cpp
View File

@ -79,6 +79,18 @@ class AstToDfgVisitor final : public VNVisitor {
// AstNode::user1p // DfgVertex for this AstNode
const VNUser1InUse m_user1InUse;
// TYPES
// Represents a driver during canonicalization
struct Driver {
FileLine* m_fileline;
DfgVertex* m_vtxp;
uint32_t m_lsb;
Driver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp)
: m_fileline{flp}
, m_vtxp{vtxp}
, m_lsb{lsb} {}
};
// STATE
DfgGraph* const m_dfgp; // The graph being built
@ -259,6 +271,21 @@ class AstToDfgVisitor final : public VNVisitor {
return true;
}
// Sometime assignment ranges are coalesced by V3Const,
// so we unpack concatenations for better error reporting.
void addDriver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp,
std::vector<Driver>& drivers) const {
if (DfgConcat* const concatp = vtxp->cast<DfgConcat>()) {
DfgVertex* const rhsp = concatp->rhsp();
addDriver(rhsp->fileline(), lsb, rhsp, drivers);
DfgVertex* const lhsp = concatp->lhsp();
addDriver(lhsp->fileline(), lsb + rhsp->width(), lhsp, drivers);
concatp->unlinkDelete(*m_dfgp);
} else {
drivers.emplace_back(flp, lsb, vtxp);
}
}
// Canonicalize packed variables
void canonicalizePacked() {
for (DfgVarPacked* const varp : m_varPackedps) {
@ -270,29 +297,71 @@ class AstToDfgVisitor final : public VNVisitor {
}
// Gather (and unlink) all drivers
struct Driver {
FileLine* flp;
uint32_t lsb;
DfgVertex* vtxp;
Driver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp)
: flp{flp}
, lsb{lsb}
, vtxp{vtxp} {}
};
std::vector<Driver> drivers;
drivers.reserve(varp->arity());
varp->forEachSourceEdge([varp, &drivers](DfgEdge& edge, size_t idx) {
UASSERT(edge.sourcep(), "Should not have created undriven sources");
drivers.emplace_back(varp->driverFileLine(idx), varp->driverLsb(idx),
edge.sourcep());
varp->forEachSourceEdge([this, varp, &drivers](DfgEdge& edge, size_t idx) {
DfgVertex* const driverp = edge.sourcep();
UASSERT(driverp, "Should not have created undriven sources");
addDriver(varp->driverFileLine(idx), varp->driverLsb(idx), driverp, drivers);
edge.unlinkSource();
});
// Sort drivers by LSB
std::stable_sort(drivers.begin(), drivers.end(),
[](const Driver& a, const Driver& b) { return a.lsb < b.lsb; });
const auto cmp = [](const Driver& a, const Driver& b) {
if (a.m_lsb != b.m_lsb) return a.m_lsb < b.m_lsb;
return a.m_fileline->operatorCompare(*b.m_fileline) < 0;
};
// TODO: bail on multidriver
// Sort drivers by LSB
std::stable_sort(drivers.begin(), drivers.end(), cmp);
// Vertices that might have become unused due to multiple driver resolution. Having
// multiple drivers is an error and is hence assumed to be rare, so performance is
// not very important, set will suffice.
std::set<DfgVertex*> prune;
// Fix multiply driven ranges
for (auto it = drivers.begin(); it != drivers.end();) {
Driver& a = *it++;
const uint32_t aWidth = a.m_vtxp->width();
const uint32_t aEnd = a.m_lsb + aWidth;
while (it != drivers.end()) {
Driver& b = *it;
// If no overlap, then nothing to do
if (b.m_lsb >= aEnd) break;
const uint32_t bWidth = b.m_vtxp->width();
const uint32_t bEnd = b.m_lsb + bWidth;
const uint32_t overlapEnd = std::min(aEnd, bEnd) - 1;
varp->varp()->v3warn( //
MULTIDRIVEN,
"Bits [" //
<< overlapEnd << ":" << b.m_lsb << "] of signal "
<< varp->varp()->prettyNameQ()
<< " have multiple combinational drivers\n"
<< a.m_fileline->warnOther() << "... Location of first driver\n"
<< a.m_fileline->warnContextPrimary() << '\n'
<< b.m_fileline->warnOther() << "... Location of other driver\n"
<< b.m_fileline->warnContextSecondary() << varp->varp()->warnOther()
<< "... Only the first driver will be respected");
// If the first driver completely covers the range of the second driver,
// we can just delete the second driver completely, otherwise adjust the
// second driver to apply from the end of the range of the first driver.
if (aEnd >= bEnd) {
prune.emplace(b.m_vtxp);
it = drivers.erase(it);
} else {
const auto dtypep = DfgVertex::dtypeForWidth(bEnd - aEnd);
DfgSel* const selp = new DfgSel{*m_dfgp, b.m_vtxp->fileline(), dtypep};
selp->fromp(b.m_vtxp);
selp->lsb(aEnd - b.m_lsb);
b.m_lsb = aEnd;
b.m_vtxp = selp;
std::stable_sort(it, drivers.end(), cmp);
}
}
}
// Coalesce adjacent ranges
for (size_t i = 0, j = 1; j < drivers.size(); ++j) {
@ -300,15 +369,15 @@ class AstToDfgVisitor final : public VNVisitor {
Driver& b = drivers[j];
// Coalesce adjacent range
const uint32_t aWidth = a.vtxp->width();
const uint32_t bWidth = b.vtxp->width();
if (a.lsb + aWidth == b.lsb) {
const uint32_t aWidth = a.m_vtxp->width();
const uint32_t bWidth = b.m_vtxp->width();
if (a.m_lsb + aWidth == b.m_lsb) {
const auto dtypep = DfgVertex::dtypeForWidth(aWidth + bWidth);
DfgConcat* const concatp = new DfgConcat{*m_dfgp, a.flp, dtypep};
concatp->rhsp(a.vtxp);
concatp->lhsp(b.vtxp);
a.vtxp = concatp;
b.vtxp = nullptr; // Mark as moved
DfgConcat* const concatp = new DfgConcat{*m_dfgp, a.m_fileline, dtypep};
concatp->rhsp(a.m_vtxp);
concatp->lhsp(b.m_vtxp);
a.m_vtxp = concatp;
b.m_vtxp = nullptr; // Mark as moved
++m_ctx.m_coalescedAssignments;
continue;
}
@ -318,17 +387,30 @@ class AstToDfgVisitor final : public VNVisitor {
// Compact non-adjacent ranges within the vector
if (j != i) {
Driver& c = drivers[i];
UASSERT_OBJ(!c.vtxp, c.flp, "Should have been marked moved");
UASSERT_OBJ(!c.m_vtxp, c.m_fileline, "Should have been marked moved");
c = b;
b.vtxp = nullptr; // Mark as moved
b.m_vtxp = nullptr; // Mark as moved
}
}
// Reinsert sources in order
// Reinsert drivers in order
varp->resetSources();
for (const Driver& driver : drivers) {
if (!driver.vtxp) break; // Stop at end of cmpacted list
varp->addDriver(driver.flp, driver.lsb, driver.vtxp);
if (!driver.m_vtxp) break; // Stop at end of compacted list
varp->addDriver(driver.m_fileline, driver.m_lsb, driver.m_vtxp);
}
// Prune vertices potentially unused due to resolving multiple drivers.
while (!prune.empty()) {
// Pop last vertex
const auto it = prune.begin();
DfgVertex* const vtxp = *it;
prune.erase(it);
// If used (or a variable), then done
if (vtxp->hasSinks() || vtxp->is<DfgVertexVar>()) continue;
// If unused, then add sources to work list and delete
vtxp->forEachSource([&](DfgVertex& src) { prune.emplace(&src); });
vtxp->unlinkDelete(*m_dfgp);
}
}
}

39
test_regress/t/t_dfg_multidriver_dfg_bad.out Normal file
View File

@ -0,0 +1,39 @@
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:13:18: Bits [3:1] of signal 'a' have multiple combinational drivers
: ... In instance t
t/t_dfg_multidriver_dfg_bad.v:14:19: ... Location of first driver
14 | assign a[3:0] = i[3:0];
| ^
t/t_dfg_multidriver_dfg_bad.v:15:19: ... Location of other driver
15 | assign a[4:1] = ~i[4:1];
| ^
t/t_dfg_multidriver_dfg_bad.v:13:18: ... Only the first driver will be respected
... For warning description see https://verilator.org/warn/MULTIDRIVEN?v=latest
... Use "/* verilator lint_off MULTIDRIVEN */" and lint_on around source to disable this message.
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:13:18: Bits [3:3] of signal 'a' have multiple combinational drivers
: ... In instance t
t/t_dfg_multidriver_dfg_bad.v:14:19: ... Location of first driver
14 | assign a[3:0] = i[3:0];
| ^
t/t_dfg_multidriver_dfg_bad.v:16:17: ... Location of other driver
16 | assign a[3] = ~i[3];
| ^
t/t_dfg_multidriver_dfg_bad.v:13:18: ... Only the first driver will be respected
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:13:18: Bits [7:6] of signal 'a' have multiple combinational drivers
: ... In instance t
t/t_dfg_multidriver_dfg_bad.v:17:19: ... Location of first driver
17 | assign a[8:5] = i[8:5];
| ^
t/t_dfg_multidriver_dfg_bad.v:18:19: ... Location of other driver
18 | assign a[7:6] = ~i[7:6];
| ^
t/t_dfg_multidriver_dfg_bad.v:13:18: ... Only the first driver will be respected
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:13:18: Bits [9:9] of signal 'a' have multiple combinational drivers
: ... In instance t
t/t_dfg_multidriver_dfg_bad.v:19:17: ... Location of first driver
19 | assign a[9] = i[9];
| ^
t/t_dfg_multidriver_dfg_bad.v:20:19: ... Location of other driver
20 | assign a[9] = ~i[9];
| ^
t/t_dfg_multidriver_dfg_bad.v:13:18: ... Only the first driver will be respected
%Error: Exiting due to

19
test_regress/t/t_dfg_multidriver_dfg_bad.pl Executable file
View File

@ -0,0 +1,19 @@
#!/usr/bin/env perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2022 by Geza Lore. 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
scenarios(vlt => 1);
compile(
fails => 1,
expect_filename => $Self->{golden_filename},
);
ok(1);
1;

23
test_regress/t/t_dfg_multidriver_dfg_bad.v Normal file
View File

@ -0,0 +1,23 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2022 by Geza Lore.
// SPDX-License-Identifier: CC0-1.0
`default_nettype none
module t(
input wire [10:0] i,
output wire [10:0] o
);
logic [10:0] a;
assign a[3:0] = i[3:0];
assign a[4:1] = ~i[4:1];
assign a[3] = ~i[3];
assign a[8:5] = i[8:5];
assign a[7:6] = ~i[7:6];
assign a[9] = i[9];
assign a[9] = ~i[9];
assign a[10] = i[10];
assign o = a;
endmodule

2
test_regress/t/t_mem_multi_io3.v
View File

@ -40,7 +40,9 @@ module testio
input logic signed [3:0] [3:0] [8:0] arr3d_in,
output logic signed [3:0] [35:0] arr2d_out
);
/* verilator lint_off MULTIDRIVEN */
logic signed [3:0] [35:0] ar2d_out_pre;
/* verilator lint_on MULTIDRIVEN */
always_comb ar2d_out_pre[0][35:0] = {arr3d_in[0][0][8:0], arr3d_in[0][1][8:0], arr3d_in[0][2][8:0], arr3d_in[0][3][8:0]};
always_comb ar2d_out_pre[0][35:0] = {arr3d_in[0][0][8:0], arr3d_in[0][1][8:0], arr3d_in[0][2][8:0], arr3d_in[0][3][8:0]};

15
test_regress/t/t_param_type3.v
View File

@ -8,26 +8,25 @@ typedef logic T_t;
module t (/*AUTOARG*/
// Outputs
o, o2,
o, ob, o2, o2b,
// Inputs
i
);
input T_t i;
output T_t o;
output T_t o2;
output T_t o, ob, o2, o2b;
sub1 #(.T_t(T_t), .CHECK(1))
sub1 (.i, .o);
sub1 (.i, .o(o));
sub2 #(.T_t(T_t), .CHECK(2))
sub2 (.i, .o(o2));
sub1 #(T_t, 1)
sub1b (i, o);
sub1b (i, ob);
sub2 #(T_t, 2)
sub2b (i, o2);
sub2b (i, o2b);
endmodule
@ -35,8 +34,8 @@ module sub1 (i,o);
parameter type T_t = real;
localparam type T2_t = T_t;
parameter int CHECK = 0;
input T_t i;
output T2_t o;
input T_t i;
output T2_t o;
assign o = i;
if (CHECK != 1) $error;
endmodule

2
test_regress/t/t_unoptflat_simple_2.v
View File

@ -12,7 +12,9 @@ module t (/*AUTOARG*/
);
input clk;
/* verilator lint_off MULTIDRIVEN */
wire [2:0] x;
/* verilator lint_on MULTIDRIVEN */
assign x[1:0] = { x[0], clk };
assign x[2:1] = x[1:0];

14
test_regress/t/t_unoptflat_simple_2_bad.out
View File

@ -1,14 +1,14 @@
%Warning-UNOPTFLAT: t/t_unoptflat_simple_2.v:15:15: Signal unoptimizable: Circular combinational logic: 't.x'
15 | wire [2:0] x;
%Warning-UNOPTFLAT: t/t_unoptflat_simple_2.v:16:15: Signal unoptimizable: Circular combinational logic: 't.x'
16 | wire [2:0] x;
| ^
... For warning description see https://verilator.org/warn/UNOPTFLAT?v=latest
... Use "/* verilator lint_off UNOPTFLAT */" and lint_on around source to disable this message.
t/t_unoptflat_simple_2.v:15:15: Example path: t.x
t/t_unoptflat_simple_2.v:17:18: Example path: ASSIGNW
t/t_unoptflat_simple_2.v:15:15: Example path: t.x
t/t_unoptflat_simple_2.v:16:15: Example path: t.x
t/t_unoptflat_simple_2.v:13:10: Example path: ASSIGNW
t/t_unoptflat_simple_2.v:16:15: Example path: t.x
... Widest variables candidate to splitting:
t/t_unoptflat_simple_2.v:15:15: t.x, width 3, circular fanout 2, can split_var
t/t_unoptflat_simple_2.v:16:15: t.x, width 3, circular fanout 1, can split_var
... Candidates with the highest fanout:
t/t_unoptflat_simple_2.v:15:15: t.x, width 3, circular fanout 2, can split_var
t/t_unoptflat_simple_2.v:16:15: t.x, width 3, circular fanout 1, can split_var
... Suggest add /*verilator split_var*/ to appropriate variables above.
%Error: Exiting due to