mirror of https://github.com/YosysHQ/yosys.git
cxxrtl: don't overwrite buffered inputs.
Before this commit, a cell's input was always assigned like: p_cell.p_input = (value...); If `p_input` is buffered (e.g. if the design is built at -O0), this is not correct. (In practice, this breaks clocking.) Unfortunately, the incorrect design was compiled without diagnostics because wire<> was move-assignable and also implicitly constructible from value<>. After this commit, cell inputs are no longer incorrectly assumed to always be unbuffered, and wires are not assignable from values.
This commit is contained in:
parent
ec410c9b19
commit
e4aa8bc96b
|
@ -659,7 +659,7 @@ struct wire {
|
||||||
value<Bits> next;
|
value<Bits> next;
|
||||||
|
|
||||||
wire() = default;
|
wire() = default;
|
||||||
constexpr wire(const value<Bits> &init) : curr(init), next(init) {}
|
explicit constexpr wire(const value<Bits> &init) : curr(init), next(init) {}
|
||||||
template<typename... Init>
|
template<typename... Init>
|
||||||
explicit constexpr wire(Init ...init) : curr{init...}, next{init...} {}
|
explicit constexpr wire(Init ...init) : curr{init...}, next{init...} {}
|
||||||
|
|
||||||
|
|
|
@ -1240,10 +1240,19 @@ struct CxxrtlWorker {
|
||||||
// User cells
|
// User cells
|
||||||
} else {
|
} else {
|
||||||
log_assert(cell->known());
|
log_assert(cell->known());
|
||||||
|
bool buffered_inputs = false;
|
||||||
const char *access = is_cxxrtl_blackbox_cell(cell) ? "->" : ".";
|
const char *access = is_cxxrtl_blackbox_cell(cell) ? "->" : ".";
|
||||||
for (auto conn : cell->connections())
|
for (auto conn : cell->connections())
|
||||||
if (cell->input(conn.first) && !cell->output(conn.first)) {
|
if (cell->input(conn.first)) {
|
||||||
f << indent << mangle(cell) << access << mangle_wire_name(conn.first) << " = ";
|
RTLIL::Module *cell_module = cell->module->design->module(cell->type);
|
||||||
|
log_assert(cell_module != nullptr && cell_module->wire(conn.first) && conn.second.is_wire());
|
||||||
|
RTLIL::Wire *cell_module_wire = cell_module->wire(conn.first);
|
||||||
|
f << indent << mangle(cell) << access << mangle_wire_name(conn.first);
|
||||||
|
if (!is_cxxrtl_blackbox_cell(cell) && !unbuffered_wires[cell_module_wire]) {
|
||||||
|
buffered_inputs = true;
|
||||||
|
f << ".next";
|
||||||
|
}
|
||||||
|
f << " = ";
|
||||||
dump_sigspec_rhs(conn.second);
|
dump_sigspec_rhs(conn.second);
|
||||||
f << ";\n";
|
f << ";\n";
|
||||||
if (getenv("CXXRTL_VOID_MY_WARRANTY")) {
|
if (getenv("CXXRTL_VOID_MY_WARRANTY")) {
|
||||||
|
@ -1255,20 +1264,12 @@ struct CxxrtlWorker {
|
||||||
// with:
|
// with:
|
||||||
// top.prev_p_clk = value<1>{0u}; top.p_clk = value<1>{1u}; top.step();
|
// top.prev_p_clk = value<1>{0u}; top.p_clk = value<1>{1u}; top.step();
|
||||||
// Don't rely on this; it will be removed without warning.
|
// Don't rely on this; it will be removed without warning.
|
||||||
RTLIL::Module *cell_module = cell->module->design->module(cell->type);
|
|
||||||
if (cell_module != nullptr && cell_module->wire(conn.first) && conn.second.is_wire()) {
|
|
||||||
RTLIL::Wire *cell_module_wire = cell_module->wire(conn.first);
|
|
||||||
if (edge_wires[conn.second.as_wire()] && edge_wires[cell_module_wire]) {
|
if (edge_wires[conn.second.as_wire()] && edge_wires[cell_module_wire]) {
|
||||||
f << indent << mangle(cell) << access << "prev_" << mangle(cell_module_wire) << " = ";
|
f << indent << mangle(cell) << access << "prev_" << mangle(cell_module_wire) << " = ";
|
||||||
f << "prev_" << mangle(conn.second.as_wire()) << ";\n";
|
f << "prev_" << mangle(conn.second.as_wire()) << ";\n";
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if (cell->input(conn.first)) {
|
|
||||||
f << indent << mangle(cell) << access << mangle_wire_name(conn.first) << ".next = ";
|
|
||||||
dump_sigspec_rhs(conn.second);
|
|
||||||
f << ";\n";
|
|
||||||
}
|
|
||||||
auto assign_from_outputs = [&](bool cell_converged) {
|
auto assign_from_outputs = [&](bool cell_converged) {
|
||||||
for (auto conn : cell->connections()) {
|
for (auto conn : cell->connections()) {
|
||||||
if (cell->output(conn.first)) {
|
if (cell->output(conn.first)) {
|
||||||
|
@ -1285,9 +1286,9 @@ struct CxxrtlWorker {
|
||||||
// have any buffered wires if they were not output ports. Imagine inlining the cell's eval() function,
|
// have any buffered wires if they were not output ports. Imagine inlining the cell's eval() function,
|
||||||
// and consider the fate of the localized wires that used to be output ports.)
|
// and consider the fate of the localized wires that used to be output ports.)
|
||||||
//
|
//
|
||||||
// Unlike cell inputs (which are never buffered), it is not possible to know apriori whether the cell
|
// It is not possible to know apriori whether the cell (which may be late bound) will converge immediately.
|
||||||
// (which may be late bound) will converge immediately. Because of this, the choice between using .curr
|
// Because of this, the choice between using .curr (appropriate for buffered outputs) and .next (appropriate
|
||||||
// (appropriate for buffered outputs) and .next (appropriate for unbuffered outputs) is made at runtime.
|
// for unbuffered outputs) is made at runtime.
|
||||||
if (cell_converged && is_cxxrtl_comb_port(cell, conn.first))
|
if (cell_converged && is_cxxrtl_comb_port(cell, conn.first))
|
||||||
f << ".next;\n";
|
f << ".next;\n";
|
||||||
else
|
else
|
||||||
|
@ -1295,6 +1296,12 @@ struct CxxrtlWorker {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
if (buffered_inputs) {
|
||||||
|
// If we have any buffered inputs, there's no chance of converging immediately.
|
||||||
|
f << indent << mangle(cell) << access << "eval();\n";
|
||||||
|
f << indent << "converged = false;\n";
|
||||||
|
assign_from_outputs(/*cell_converged=*/false);
|
||||||
|
} else {
|
||||||
f << indent << "if (" << mangle(cell) << access << "eval()) {\n";
|
f << indent << "if (" << mangle(cell) << access << "eval()) {\n";
|
||||||
inc_indent();
|
inc_indent();
|
||||||
assign_from_outputs(/*cell_converged=*/true);
|
assign_from_outputs(/*cell_converged=*/true);
|
||||||
|
@ -1307,6 +1314,7 @@ struct CxxrtlWorker {
|
||||||
f << indent << "}\n";
|
f << indent << "}\n";
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
void dump_assign(const RTLIL::SigSig &sigsig)
|
void dump_assign(const RTLIL::SigSig &sigsig)
|
||||||
{
|
{
|
||||||
|
|
Loading…
Reference in New Issue