mirror of
https://github.com/verilator/verilator.git
synced 2026-10-06 01:54:01 +02:00
Synthesize successive whole-element array assignments (#8316)
Preserve previous whole-element drivers when synthesizing successive constant-index assignments to unpacked arrays. Retain earlier values through their assignment temporaries and keep the default while some elements remain uncovered. Complete the array-default tracing path exposed by this representation. Keep public-write checks, control-flow restrictions, and scheduling unchanged, and cover propagation and conservative fallbacks in the existing DFG regressions. Part of #7964; array-only follow-up to #7966.
This commit is contained in:
Patrick Creighton
@@ -567,7 +567,7 @@ class TraceDriver final : public DfgVisitor {
|
||||
|
||||
void visit(DfgSpliceArray* vtxp) override {
|
||||
UASSERT_OBJ(m_splicep == vtxp, vtxp, "Unexpected trace of DfgSpliceArray");
|
||||
// DfgVertex* defaultp = m_defaultp;
|
||||
DfgVertex* const defaultp = m_defaultp;
|
||||
m_defaultp = nullptr;
|
||||
m_splicep = nullptr;
|
||||
|
||||
@@ -575,34 +575,17 @@ class TraceDriver final : public DfgVisitor {
|
||||
const uint32_t idx = m_idxs.back();
|
||||
if (DfgVertex* const driverp = vtxp->driverAt(idx)) {
|
||||
DfgVertex* const srcp = driverp->as<DfgUnitArray>()->srcp();
|
||||
// TODO: this is unreachable today, but with e.g. #8316 it wouldn't be
|
||||
// // TODO: replace DfgSplice with DfgInsert modeling
|
||||
// // Annoying corner case: If the element itself is a splice, we need
|
||||
// // a defaultp for that element if there was one for the whole array.
|
||||
// // Make one up by selecting out of the default. It will be removed
|
||||
// // later if unused. Pretend it's in the same component as the array
|
||||
// // default as trace needs to continue in that case.
|
||||
// if (defaultp && srcp->is<DfgVertexSplice>()) {
|
||||
// DfgArraySel* const aselp = new DfgArraySel{m_dfg,
|
||||
// vtxp->fileline(), srcp->dtype()};
|
||||
// m_sccInfo.add(*aselp, m_sccInfo.get(*defaultp));
|
||||
// DfgConst* const idxp = make<DfgConst>(vtxp, 32);
|
||||
// idxp->num().setLong(idx);
|
||||
// aselp->fromp(defaultp);
|
||||
// aselp->bitp(idxp);
|
||||
// m_defaultp = aselp;
|
||||
// m_splicep = srcp;
|
||||
// }
|
||||
if (srcp->is<DfgVertexSplice>()) {
|
||||
// Partial-element propagation is rejected during synthesis.
|
||||
UASSERT_OBJ(!defaultp, vtxp, "Array default with partial element driver");
|
||||
m_splicep = srcp;
|
||||
}
|
||||
// Consume this index, then trace the element value
|
||||
if (srcp->is<DfgVertexSplice>()) m_splicep = srcp;
|
||||
RETURN_RESULT(tracePopIdx(srcp));
|
||||
}
|
||||
// TODO: this is unreachable, as syntheis can't create it today.
|
||||
// // Element not driven explicitly, so it comes from the default array. Keep the
|
||||
// // index pending (the default is the whole array, indexed the same way) and
|
||||
// // continue tracing it.
|
||||
// UASSERT_OBJ(m_defaultp, vtxp, "Independent array element should have a driver or
|
||||
// default"); RETURN RESULT(traceSameIdx(m_defaultp));
|
||||
// An element not driven explicitly comes from the default array at the same index.
|
||||
UASSERT_OBJ(defaultp, vtxp, "Independent array element should have a driver or default");
|
||||
RETURN_RESULT(traceSameIdx(defaultp));
|
||||
}
|
||||
|
||||
void visit(DfgVertexVar* vtxp) override {
|
||||
@@ -612,9 +595,6 @@ class TraceDriver final : public DfgVisitor {
|
||||
DfgVertex* const drvp = srcp ? srcp : defaultp;
|
||||
// If we are about to trace a splice, set the defaultp to the corresponding default
|
||||
if (srcp && srcp->is<DfgVertexSplice>()) {
|
||||
// Unreachable today: getting an array into a fixable cycle needs multiple
|
||||
// assignments in a process, which V3DfgSynthesize rejects ("Can't do arrays yet").
|
||||
UASSERT_OBJ(!defaultp || vtxp->isPacked(), vtxp, "Array variable with defaultp");
|
||||
m_defaultp = defaultp;
|
||||
m_splicep = srcp;
|
||||
}
|
||||
|
||||
+68
-10
@@ -701,6 +701,28 @@ class AstToDfgSynthesize final {
|
||||
return drivers;
|
||||
}
|
||||
|
||||
// Returns true if the driver supplies a complete array element.
|
||||
static bool driverCoversWholeElement(const Driver& driver) {
|
||||
const DfgUnitArray* const unitp = driver.m_vtxp->cast<DfgUnitArray>();
|
||||
if (!unitp) return false;
|
||||
if (DfgVertexSplice* const splicep = unitp->srcp()->cast<DfgVertexSplice>()) {
|
||||
return splicep->wholep();
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
// Unlike 'wholep', several element drivers can collectively cover an array.
|
||||
static bool spliceCoversWhole(DfgVertexSplice* const splicep) {
|
||||
if (splicep->wholep()) return true;
|
||||
if (splicep->isPacked()) return false;
|
||||
uint32_t next = 0;
|
||||
for (const Driver& driver : gatherDrivers(splicep)) {
|
||||
if (driver.m_lo != next || !driverCoversWholeElement(driver)) return false;
|
||||
next = driver.m_hi + 1;
|
||||
}
|
||||
return next == splicep->size();
|
||||
}
|
||||
|
||||
// Returns true if the driver cone contains any variable introduced by
|
||||
// tristate lowering. Used to distinguish intentional tristate contributor
|
||||
// overlap from accidental multidrive.
|
||||
@@ -1313,6 +1335,42 @@ class AstToDfgSynthesize final {
|
||||
return propagatedDrivers;
|
||||
}
|
||||
|
||||
// Propagate whole elements with a linear walk over the sorted driver lists.
|
||||
// Reads use the previous value temporary, preserving assignment-version bindings.
|
||||
// Partial elements and non-unit drivers retain the nonsynthesized-process fallback.
|
||||
bool computePropagatedArrayDrivers(const std::vector<Driver>& newDrivers,
|
||||
DfgVertexVar* const oldp,
|
||||
std::vector<Driver>& propagatedDrivers) {
|
||||
// Bound quadratic vertex growth: array drivers cannot be coalesced.
|
||||
static constexpr uint32_t MAX_ARRAY_ELEMENTS = 32;
|
||||
if (oldp->size() > MAX_ARRAY_ELEMENTS) return false;
|
||||
for (const Driver& driver : newDrivers) {
|
||||
if (!driverCoversWholeElement(driver)) return false;
|
||||
}
|
||||
const std::vector<Driver> oldDrivers = gatherDrivers(oldp->srcp()->as<DfgVertexSplice>());
|
||||
UASSERT_OBJ(!oldDrivers.empty(), oldp, "Should have a proper driver");
|
||||
for (const Driver& driver : oldDrivers) {
|
||||
if (!driverCoversWholeElement(driver)) return false;
|
||||
}
|
||||
|
||||
propagatedDrivers.reserve(oldDrivers.size());
|
||||
auto nIt = newDrivers.begin();
|
||||
for (const Driver& oDriver : oldDrivers) {
|
||||
while (nIt != newDrivers.end() && nIt->m_lo < oDriver.m_lo) ++nIt;
|
||||
if (nIt != newDrivers.end() && nIt->m_lo == oDriver.m_lo) continue;
|
||||
|
||||
FileLine* const flp = oDriver.m_flp;
|
||||
const DfgUnitArray* const oldUnitp = oDriver.m_vtxp->as<DfgUnitArray>();
|
||||
DfgArraySel* const selp = make<DfgArraySel>(flp, oldUnitp->srcp()->dtype());
|
||||
selp->fromp(oldp);
|
||||
selp->bitp(make<DfgConst>(flp, static_cast<size_t>(VL_IDATASIZE), oDriver.m_lo));
|
||||
DfgUnitArray* const newUnitp = make<DfgUnitArray>(flp, oldUnitp->dtype());
|
||||
newUnitp->srcp(selp);
|
||||
propagatedDrivers.emplace_back(newUnitp, oDriver.m_lo, flp);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
// Given the drivers of a variable after converting a single statement
|
||||
// 'newp', add drivers from 'oldp' that were not reassigned be drivers
|
||||
// in newp. This computes the total result of all previous assignments.
|
||||
@@ -1328,25 +1386,25 @@ class AstToDfgSynthesize final {
|
||||
// If the old value is the real variable we just computed the new value for,
|
||||
// then it is the circular feedback into the synthesized block, add it as default driver.
|
||||
if (oldp->vscp() == vscp) {
|
||||
if (!nSplicep->wholep()) newp->defaultp(oldp);
|
||||
if (!spliceCoversWhole(nSplicep)) newp->defaultp(oldp);
|
||||
return true;
|
||||
}
|
||||
|
||||
UASSERT_OBJ(oldp->srcp(), vscp, "Previously assigned variable has no driver");
|
||||
|
||||
// Can't do arrays yet
|
||||
if (!newp->isPacked()) {
|
||||
++m_ctx.m_synt.nonSynArray;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Gather drivers of 'newp' - they are in incresing range order with no overlaps
|
||||
UASSERT_OBJ(!newp->defaultp(), newp, "Converted value should not have default");
|
||||
std::vector<Driver> nDrivers = gatherDrivers(newp->srcp()->as<DfgVertexSplice>());
|
||||
UASSERT_OBJ(!nDrivers.empty(), newp, "Should have a proper driver");
|
||||
|
||||
// Additional drivers of 'newp' propagated from 'oldp'
|
||||
std::vector<Driver> pDrivers = computePropagatedDrivers(nDrivers, oldp);
|
||||
std::vector<Driver> pDrivers;
|
||||
if (newp->isPacked()) {
|
||||
pDrivers = computePropagatedDrivers(nDrivers, oldp);
|
||||
} else if (!computePropagatedArrayDrivers(nDrivers, oldp, pDrivers)) {
|
||||
++m_ctx.m_synt.nonSynArray;
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!pDrivers.empty()) {
|
||||
// Need to merge propagated sources, so reset the splice
|
||||
@@ -1357,13 +1415,13 @@ class AstToDfgSynthesize final {
|
||||
std::merge(nDrivers.begin(), nDrivers.end(), pDrivers.begin(), pDrivers.end(),
|
||||
std::back_inserter(drivers));
|
||||
// Coalesce adjacent ranges
|
||||
coalesceDrivers(drivers);
|
||||
if (newp->isPacked()) coalesceDrivers(drivers);
|
||||
// Reinsert drivers in order
|
||||
for (const Driver& d : drivers) nSplicep->addDriver(d.m_vtxp, d.m_lo, d.m_flp);
|
||||
}
|
||||
|
||||
// If the old had a default, add to the new one too, unless redundant
|
||||
if (oldp->defaultp() && !nSplicep->wholep()) newp->defaultp(oldp->defaultp());
|
||||
if (oldp->defaultp() && !spliceCoversWhole(nSplicep)) newp->defaultp(oldp->defaultp());
|
||||
|
||||
// Done
|
||||
return true;
|
||||
|
||||
@@ -251,6 +251,17 @@ module t (
|
||||
`signal(ARRAY_3, 3); // UNOPTFLAT
|
||||
assign ARRAY_3 = array_3[0];
|
||||
|
||||
logic [6:0] array_default[3]; // UNOPTFLAT
|
||||
logic [6:0] array_default_in;
|
||||
assign array_default[1] = rand_a[6:0];
|
||||
always @* begin
|
||||
array_default_in = array_default[1];
|
||||
array_default[0] = rand_b[6:0] ^ array_default_in;
|
||||
array_default[2] = array_default[1];
|
||||
end
|
||||
`signal(ARRAY_DEFAULT, 21);
|
||||
assign ARRAY_DEFAULT = {array_default[2], array_default[1], array_default[0]};
|
||||
|
||||
`signal(ADD_A, 8); // UNOPTFLAT
|
||||
`signal(ADD_B, 8);
|
||||
`signal(ADD_C, 8);
|
||||
|
||||
@@ -84,27 +84,34 @@
|
||||
t/t_dfg_multidriver_dfg_bad.v:60:18: ... Location of offending driver
|
||||
60 | assign z[10:7] = i[10:7];
|
||||
| ^
|
||||
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:76:16: Bits [5:2] of signal 't.sub_1.a' have multiple combinational drivers. This can cause performance degradation.
|
||||
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:87:16: Bits [5:2] of signal 't.sub_1.a' have multiple combinational drivers. This can cause performance degradation.
|
||||
t/t_dfg_multidriver_dfg_bad.v:63:18: ... Location of offending driver
|
||||
63 | assign sub_1.a = i;
|
||||
| ^
|
||||
t/t_dfg_multidriver_dfg_bad.v:77:17: ... Location of offending driver
|
||||
77 | assign a[5:2] = i[5:2];
|
||||
t/t_dfg_multidriver_dfg_bad.v:88:17: ... Location of offending driver
|
||||
88 | assign a[5:2] = i[5:2];
|
||||
| ^
|
||||
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:76:16: Bits [3:2] of signal 't.sub_2.a' have multiple combinational drivers. This can cause performance degradation.
|
||||
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:87:16: Bits [3:2] of signal 't.sub_2.a' have multiple combinational drivers. This can cause performance degradation.
|
||||
t/t_dfg_multidriver_dfg_bad.v:67:23: ... Location of offending driver
|
||||
67 | assign sub_2.a[3:0] = i[3:0];
|
||||
| ^
|
||||
t/t_dfg_multidriver_dfg_bad.v:77:17: ... Location of offending driver
|
||||
77 | assign a[5:2] = i[5:2];
|
||||
t/t_dfg_multidriver_dfg_bad.v:88:17: ... Location of offending driver
|
||||
88 | assign a[5:2] = i[5:2];
|
||||
| ^
|
||||
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:76:16: Bit [5] of signal 't.sub_2.a' have multiple combinational drivers. This can cause performance degradation.
|
||||
t/t_dfg_multidriver_dfg_bad.v:77:17: ... Location of offending driver
|
||||
77 | assign a[5:2] = i[5:2];
|
||||
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:87:16: Bit [5] of signal 't.sub_2.a' have multiple combinational drivers. This can cause performance degradation.
|
||||
t/t_dfg_multidriver_dfg_bad.v:88:17: ... Location of offending driver
|
||||
88 | assign a[5:2] = i[5:2];
|
||||
| ^
|
||||
t/t_dfg_multidriver_dfg_bad.v:66:24: ... Location of offending driver
|
||||
66 | assign sub_2.a[10:5] = i[10:5];
|
||||
| ^
|
||||
%Warning-MULTIDRIVEN: t/t_dfg_multidriver_dfg_bad.v:69:16: Element [0] of signal 't.array_always' have multiple combinational drivers. This can cause performance degradation.
|
||||
t/t_dfg_multidriver_dfg_bad.v:71:17: ... Location of offending driver
|
||||
71 | array_always[0] = i;
|
||||
| ^
|
||||
t/t_dfg_multidriver_dfg_bad.v:75:17: ... Location of offending driver
|
||||
75 | array_always[0] = k[0];
|
||||
| ^
|
||||
%Error: Internal Error: t/t_dfg_multidriver_dfg_bad.v:48:12: ../V3Gate.cpp:#: Concat on LHS of assignment; V3Const should have deleted it
|
||||
48 | {y[1:0], y[2:1]} = i[3:0] + 4'd5;
|
||||
| ^
|
||||
|
||||
@@ -66,7 +66,18 @@ module t (
|
||||
assign sub_2.a[10:5] = i[10:5];
|
||||
assign sub_2.a[3:0] = i[3:0];
|
||||
|
||||
assign o = a ^ u[3] ^ v[3] ^ w[3] ^ x[3] ^ y ^ z ^ sub_1.a ^ sub_2.a;
|
||||
logic [10:0] array_always[3];
|
||||
always_comb begin
|
||||
array_always[0] = i;
|
||||
array_always[1] = j[1];
|
||||
end
|
||||
always_comb begin
|
||||
array_always[0] = k[0];
|
||||
array_always[2] = j[2];
|
||||
end
|
||||
|
||||
assign o = a ^ u[3] ^ v[3] ^ w[3] ^ x[3] ^ y ^ z ^ sub_1.a ^ sub_2.a
|
||||
^ array_always[0] ^ array_always[1] ^ array_always[2];
|
||||
|
||||
endmodule
|
||||
|
||||
|
||||
@@ -628,4 +628,128 @@ module t (
|
||||
end
|
||||
`signal(ARRAY_READ, array_read);
|
||||
|
||||
logic array_ready;
|
||||
logic array_transfer_ready;
|
||||
logic [1:0] array_status[2];
|
||||
always_comb begin
|
||||
array_ready = rand_a[0];
|
||||
array_transfer_ready = array_ready && rand_b[0];
|
||||
array_status[0] = {array_ready, array_transfer_ready};
|
||||
array_status[1] = {array_transfer_ready, array_ready};
|
||||
end
|
||||
`signal(ARRAY_SEQUENTIAL, {array_status[1], array_status[0]});
|
||||
|
||||
typedef logic [6:0] array_element_t;
|
||||
array_element_t array_descending[3:1];
|
||||
array_element_t array_ascending[1:3];
|
||||
always_comb begin
|
||||
array_descending[2] = rand_a[6:0];
|
||||
array_descending[3] = rand_b[6:0];
|
||||
array_descending[1] = rand_a[13:7];
|
||||
array_ascending[2] = rand_b[6:0];
|
||||
array_ascending[1] = rand_a[6:0];
|
||||
array_ascending[3] = rand_b[13:7];
|
||||
end
|
||||
`signal(ARRAY_BOUNDS, {array_descending[1], array_descending[2], array_descending[3],
|
||||
array_ascending[1], array_ascending[2], array_ascending[3]});
|
||||
|
||||
logic [6:0] array_intermediate[2];
|
||||
logic [6:0] array_before;
|
||||
logic [6:0] array_after;
|
||||
always_comb begin
|
||||
array_intermediate[0] = rand_a[6:0];
|
||||
array_before = array_intermediate[0];
|
||||
array_intermediate[1] = rand_b[6:0];
|
||||
array_intermediate[0] = ~rand_a[6:0];
|
||||
array_after = array_intermediate[0];
|
||||
array_intermediate[1] = array_after ^ rand_b[6:0];
|
||||
end
|
||||
`signal(ARRAY_INTERMEDIATE, {array_intermediate[1], array_intermediate[0], array_before,
|
||||
array_after});
|
||||
|
||||
logic [6:0] array_retained[4];
|
||||
logic [6:0] array_retained_before;
|
||||
assign array_retained[1] = rand_b[6:0];
|
||||
always @* begin
|
||||
array_retained_before = array_retained[1];
|
||||
{array_retained[3], array_retained[0]} = {rand_a[6:0], rand_a[13:7]};
|
||||
array_retained[2] = array_retained[1] ^ rand_a[20:14];
|
||||
{array_retained[3], array_retained[0]} = {array_retained_before ^ rand_a[6:0], ~rand_a[6:0]};
|
||||
end
|
||||
`signal(ARRAY_RETAINED, {array_retained[3], array_retained[2], array_retained[1],
|
||||
array_retained[0], array_retained_before});
|
||||
|
||||
logic [6:0] array_disjoint[4];
|
||||
always_comb begin
|
||||
array_disjoint[3] = rand_a[6:0];
|
||||
array_disjoint[1] = rand_b[6:0];
|
||||
end
|
||||
always_comb begin
|
||||
array_disjoint[0] = rand_a[13:7];
|
||||
array_disjoint[2] = rand_b[13:7];
|
||||
end
|
||||
`signal(ARRAY_DISJOINT, {array_disjoint[3], array_disjoint[2], array_disjoint[1],
|
||||
array_disjoint[0]});
|
||||
|
||||
// verilator lint_off MULTIDRIVEN
|
||||
// Both blocks write the same value to element 0 so the equivalence check
|
||||
// does not depend on their execution order.
|
||||
logic [6:0] array_multidriven[3];
|
||||
always_comb begin // revert
|
||||
array_multidriven[0] = rand_a[6:0];
|
||||
array_multidriven[1] = rand_b[6:0];
|
||||
end
|
||||
always_comb begin // revert
|
||||
array_multidriven[0] = rand_a[6:0];
|
||||
array_multidriven[2] = rand_a[13:7];
|
||||
end
|
||||
// verilator lint_on MULTIDRIVEN
|
||||
`signal(ARRAY_MULTIDRIVEN, {array_multidriven[2], array_multidriven[1], array_multidriven[0]});
|
||||
|
||||
logic [6:0] array_at_limit[32];
|
||||
always_comb begin
|
||||
/*verilator unroll_full*/
|
||||
for (int k = 0; k < 32; ++k) array_at_limit[k] = 7'(rand_a >> k) ^ 7'(k);
|
||||
end
|
||||
`signal(ARRAY_AT_LIMIT, {array_at_limit[31], array_at_limit[16], array_at_limit[0]});
|
||||
|
||||
logic [6:0] array_large[33];
|
||||
always_comb begin // nosynth
|
||||
/*verilator unroll_full*/
|
||||
for (int k = 0; k < 33; ++k) array_large[k] = 7'(rand_a >> k) ^ 7'(k);
|
||||
end
|
||||
`signal(ARRAY_LARGE, {array_large[32], array_large[16], array_large[0]});
|
||||
|
||||
logic [6:0] array_final_read[2];
|
||||
logic [6:0] array_final_value;
|
||||
always_comb begin // nosynth
|
||||
array_final_read[0] = rand_a[6:0];
|
||||
array_final_read[1] = rand_b[6:0];
|
||||
array_final_value = array_final_read[0];
|
||||
end
|
||||
`signal(ARRAY_FINAL_READ, {array_final_read[1], array_final_read[0], array_final_value});
|
||||
|
||||
logic [6:0] array_partial_reassign[2];
|
||||
always_comb begin // nosynth
|
||||
array_partial_reassign[0] = rand_a[6:0];
|
||||
array_partial_reassign[1] = rand_b[6:0];
|
||||
array_partial_reassign[1][2:0] = rand_a[2:0];
|
||||
end
|
||||
`signal(ARRAY_PARTIAL_REASSIGN, {array_partial_reassign[1], array_partial_reassign[0]});
|
||||
|
||||
logic [6:0] array_partial_previous[2];
|
||||
always_comb begin // nosynth
|
||||
array_partial_previous[0][2:0] = rand_a[2:0];
|
||||
array_partial_previous[1] = rand_b[6:0];
|
||||
array_partial_previous[0][6:3] = rand_a[6:3];
|
||||
end
|
||||
`signal(ARRAY_PARTIAL_PREVIOUS, {array_partial_previous[1], array_partial_previous[0]});
|
||||
|
||||
logic [6:0] array_whole_previous[2];
|
||||
always_comb begin // nosynth
|
||||
array_whole_previous = array_intermediate;
|
||||
array_whole_previous[1] = rand_a[6:0];
|
||||
end
|
||||
`signal(ARRAY_WHOLE_PREVIOUS, {array_whole_previous[1], array_whole_previous[0]});
|
||||
|
||||
endmodule
|
||||
|
||||
Reference in New Issue
Block a user