From e583da906d8e5f3b8f4146727042c4bc60ad7bb1 Mon Sep 17 00:00:00 2001 From: "Emil J. Tywoniak" Date: Sat, 30 May 2026 19:07:37 +0200 Subject: [PATCH] patch: merge src into existing cells; opt_merge/_inc + onehot + ff.cc use Patch --- kernel/ff.cc | 86 +++++++++++++++++++++---------------- kernel/unstable/patch.cc | 33 +++++++++++++- kernel/unstable/patch.h | 7 +++ passes/opt/opt_merge.cc | 20 ++++++--- passes/opt/opt_merge_inc.cc | 8 ++-- passes/opt/pmux2shiftx.cc | 20 +++++++-- 6 files changed, 122 insertions(+), 52 deletions(-) diff --git a/kernel/ff.cc b/kernel/ff.cc index 0a80641cc..7ce8da9e3 100644 --- a/kernel/ff.cc +++ b/kernel/ff.cc @@ -21,6 +21,16 @@ USING_YOSYS_NAMESPACE +namespace { + // Pull the FF's src attribute so we can propagate it to intermediate + // cells created during unmap / conversion — otherwise downstream tools + // lose source provenance for the unmapped logic. + std::string ff_src(const FfData &ff) { + auto it = ff.attributes.find(ID::src); + return it == ff.attributes.end() ? std::string() : it->second.decode_string(); + } +} + // sorry template>> void manufacture_info(InputType flop, OutputType& info, FfInitVals *initvals) { @@ -484,25 +494,26 @@ void FfData::aload_to_sr() { log_assert(!has_sr); has_sr = true; has_aload = false; + std::string src = ff_src(*this); if (!is_fine) { pol_clr = false; pol_set = true; if (pol_aload) { - sig_clr = module->Mux(NEW_ID, Const(State::S1, width), sig_ad, sig_aload); - sig_set = module->Mux(NEW_ID, Const(State::S0, width), sig_ad, sig_aload); + sig_clr = module->Mux(NEW_ID, Const(State::S1, width), sig_ad, sig_aload, src); + sig_set = module->Mux(NEW_ID, Const(State::S0, width), sig_ad, sig_aload, src); } else { - sig_clr = module->Mux(NEW_ID, sig_ad, Const(State::S1, width), sig_aload); - sig_set = module->Mux(NEW_ID, sig_ad, Const(State::S0, width), sig_aload); + sig_clr = module->Mux(NEW_ID, sig_ad, Const(State::S1, width), sig_aload, src); + sig_set = module->Mux(NEW_ID, sig_ad, Const(State::S0, width), sig_aload, src); } } else { pol_clr = pol_aload; pol_set = pol_aload; if (pol_aload) { - sig_clr = module->AndnotGate(NEW_ID, sig_aload, sig_ad); - sig_set = module->AndGate(NEW_ID, sig_aload, sig_ad); + sig_clr = module->AndnotGate(NEW_ID, sig_aload, sig_ad, src); + sig_set = module->AndGate(NEW_ID, sig_aload, sig_ad, src); } else { - sig_clr = module->OrGate(NEW_ID, sig_aload, sig_ad); - sig_set = module->OrnotGate(NEW_ID, sig_aload, sig_ad); + sig_clr = module->OrGate(NEW_ID, sig_aload, sig_ad, src); + sig_set = module->OrnotGate(NEW_ID, sig_aload, sig_ad, src); } } } @@ -510,36 +521,37 @@ void FfData::aload_to_sr() { void FfData::convert_ce_over_srst(bool val) { if (!has_ce || !has_srst || ce_over_srst == val) return; + std::string src = ff_src(*this); if (val) { // sdffe to sdffce if (!is_fine) { if (pol_ce) { if (pol_srst) { - sig_ce = module->Or(NEW_ID, sig_ce, sig_srst); + sig_ce = module->Or(NEW_ID, sig_ce, sig_srst, false, src); } else { - SigSpec tmp = module->Not(NEW_ID, sig_srst); - sig_ce = module->Or(NEW_ID, sig_ce, tmp); + SigSpec tmp = module->Not(NEW_ID, sig_srst, false, src); + sig_ce = module->Or(NEW_ID, sig_ce, tmp, false, src); } } else { if (pol_srst) { - SigSpec tmp = module->Not(NEW_ID, sig_srst); - sig_ce = module->And(NEW_ID, sig_ce, tmp); + SigSpec tmp = module->Not(NEW_ID, sig_srst, false, src); + sig_ce = module->And(NEW_ID, sig_ce, tmp, false, src); } else { - sig_ce = module->And(NEW_ID, sig_ce, sig_srst); + sig_ce = module->And(NEW_ID, sig_ce, sig_srst, false, src); } } } else { if (pol_ce) { if (pol_srst) { - sig_ce = module->OrGate(NEW_ID, sig_ce, sig_srst); + sig_ce = module->OrGate(NEW_ID, sig_ce, sig_srst, src); } else { - sig_ce = module->OrnotGate(NEW_ID, sig_ce, sig_srst); + sig_ce = module->OrnotGate(NEW_ID, sig_ce, sig_srst, src); } } else { if (pol_srst) { - sig_ce = module->AndnotGate(NEW_ID, sig_ce, sig_srst); + sig_ce = module->AndnotGate(NEW_ID, sig_ce, sig_srst, src); } else { - sig_ce = module->AndGate(NEW_ID, sig_ce, sig_srst); + sig_ce = module->AndGate(NEW_ID, sig_ce, sig_srst, src); } } } @@ -548,31 +560,31 @@ void FfData::convert_ce_over_srst(bool val) { if (!is_fine) { if (pol_srst) { if (pol_ce) { - sig_srst = cell->module->And(NEW_ID, sig_srst, sig_ce); + sig_srst = cell->module->And(NEW_ID, sig_srst, sig_ce, false, src); } else { - SigSpec tmp = module->Not(NEW_ID, sig_ce); - sig_srst = cell->module->And(NEW_ID, sig_srst, tmp); + SigSpec tmp = module->Not(NEW_ID, sig_ce, false, src); + sig_srst = cell->module->And(NEW_ID, sig_srst, tmp, false, src); } } else { if (pol_ce) { - SigSpec tmp = module->Not(NEW_ID, sig_ce); - sig_srst = cell->module->Or(NEW_ID, sig_srst, tmp); + SigSpec tmp = module->Not(NEW_ID, sig_ce, false, src); + sig_srst = cell->module->Or(NEW_ID, sig_srst, tmp, false, src); } else { - sig_srst = cell->module->Or(NEW_ID, sig_srst, sig_ce); + sig_srst = cell->module->Or(NEW_ID, sig_srst, sig_ce, false, src); } } } else { if (pol_srst) { if (pol_ce) { - sig_srst = cell->module->AndGate(NEW_ID, sig_srst, sig_ce); + sig_srst = cell->module->AndGate(NEW_ID, sig_srst, sig_ce, src); } else { - sig_srst = cell->module->AndnotGate(NEW_ID, sig_srst, sig_ce); + sig_srst = cell->module->AndnotGate(NEW_ID, sig_srst, sig_ce, src); } } else { if (pol_ce) { - sig_srst = cell->module->OrnotGate(NEW_ID, sig_srst, sig_ce); + sig_srst = cell->module->OrnotGate(NEW_ID, sig_srst, sig_ce, src); } else { - sig_srst = cell->module->OrGate(NEW_ID, sig_srst, sig_ce); + sig_srst = cell->module->OrGate(NEW_ID, sig_srst, sig_ce, src); } } } @@ -587,16 +599,17 @@ void FfData::unmap_ce() { if (has_srst && ce_over_srst) unmap_srst(); + std::string src = ff_src(*this); if (!is_fine) { if (pol_ce) - sig_d = module->Mux(NEW_ID, sig_q, sig_d, sig_ce); + sig_d = module->Mux(NEW_ID, sig_q, sig_d, sig_ce, src); else - sig_d = module->Mux(NEW_ID, sig_d, sig_q, sig_ce); + sig_d = module->Mux(NEW_ID, sig_d, sig_q, sig_ce, src); } else { if (pol_ce) - sig_d = module->MuxGate(NEW_ID, sig_q, sig_d, sig_ce); + sig_d = module->MuxGate(NEW_ID, sig_q, sig_d, sig_ce, src); else - sig_d = module->MuxGate(NEW_ID, sig_d, sig_q, sig_ce); + sig_d = module->MuxGate(NEW_ID, sig_d, sig_q, sig_ce, src); } has_ce = false; } @@ -607,16 +620,17 @@ void FfData::unmap_srst() { if (has_ce && !ce_over_srst) unmap_ce(); + std::string src = ff_src(*this); if (!is_fine) { if (pol_srst) - sig_d = module->Mux(NEW_ID, sig_d, val_srst, sig_srst); + sig_d = module->Mux(NEW_ID, sig_d, val_srst, sig_srst, src); else - sig_d = module->Mux(NEW_ID, val_srst, sig_d, sig_srst); + sig_d = module->Mux(NEW_ID, val_srst, sig_d, sig_srst, src); } else { if (pol_srst) - sig_d = module->MuxGate(NEW_ID, sig_d, val_srst[0], sig_srst); + sig_d = module->MuxGate(NEW_ID, sig_d, val_srst[0], sig_srst, src); else - sig_d = module->MuxGate(NEW_ID, val_srst[0], sig_d, sig_srst); + sig_d = module->MuxGate(NEW_ID, val_srst[0], sig_d, sig_srst, src); } has_srst = false; } diff --git a/kernel/unstable/patch.cc b/kernel/unstable/patch.cc index 35075bc39..eed377883 100644 --- a/kernel/unstable/patch.cc +++ b/kernel/unstable/patch.cc @@ -149,14 +149,25 @@ Cell* Patch::commit_cell(std::unique_ptr cell) { } void Patch::patch(Cell* old_cell, IdString old_port, SigSpec new_sig) { - patch(old_cell, {{old_port, new_sig}}); + patch(old_cell, {{old_port, new_sig}}, nullptr); } void Patch::patch(Cell* old_cell, const std::vector> &port_replacements) { + patch(old_cell, port_replacements, nullptr); +} + +void Patch::patch(Cell* old_cell, IdString old_port, SigSpec new_sig, Cell* merge_src_into) { + patch(old_cell, {{old_port, new_sig}}, merge_src_into); +} + +void Patch::patch(Cell* old_cell, const std::vector> &port_replacements, Cell* merge_src_into) { std::vector old_sigs; for (auto &[port, new_sig] : port_replacements) { SigSpec old_sig = old_cell->getPort(port); - log_assert(old_sig.size() == new_sig.size()); + if (old_sig.size() != new_sig.size()) + log_error("patch size mismatch on cell %s port %s: old %d (%s) vs new %d (%s)\n", + log_id(old_cell->name), log_id(port), old_sig.size(), log_signal(old_sig), + new_sig.size(), log_signal(new_sig)); log_debug("patching %s %s which is %s with %s:\n", old_cell->name, port, log_signal(old_sig), log_signal(new_sig)); old_sigs.push_back(old_sig); } @@ -164,6 +175,21 @@ void Patch::patch(Cell* old_cell, const std::vector SrcCollector collector; for (auto &old_sig : old_sigs) collector.collect_src(old_sig); + + // The collector should only ever pick up old_cell — the cell whose + // outputs are being patched. If a future change to collect_src ever + // starts walking the fanout or input cone of foreign cells, this + // assertion fires so we notice instead of silently smearing src + // strings across unrelated cells. + for (auto *c : collector.done) + log_assert(c == old_cell); + + // For "merge into existing cell" patches (e.g. opt_merge), also pull + // in the keep-cell's pre-existing src so the merged cell carries both + // source locations. + if (merge_src_into) + collector.src.insert(merge_src_into->get_src_attribute()); + std::string src_str = AttrObject::strpool_attribute_to_str(collector.src); // Record leaves (existing wires consumed as inputs by the new cells) so @@ -192,6 +218,9 @@ void Patch::patch(Cell* old_cell, const std::vector for (auto& wire: wires_) commit_wire(std::move(wire)); + if (merge_src_into) + merge_src_into->set_src_attribute(src_str); + // Now drop old_cell's drivers so old_sigs are undriven, then merge each // into its new_sig. connect_incremental updates sigmap and re-normalizes // fanout consumers in place — no full sigNormalize needed. diff --git a/kernel/unstable/patch.h b/kernel/unstable/patch.h index cc664f511..772f72923 100644 --- a/kernel/unstable/patch.h +++ b/kernel/unstable/patch.h @@ -35,6 +35,13 @@ public: void patch(Cell* old_cell, IdString old_port, SigSpec new_sig); void patch(Cell* old_cell, const std::vector> &port_replacements); + + // Variants for "merge old_cell into an existing keep_cell" (e.g. + // opt_merge): the old_cell's src attribute is collected and combined + // with merge_src_into's existing src, and the result is set on + // merge_src_into. Any new cells in cells_ also receive the combined src. + void patch(Cell* old_cell, IdString old_port, SigSpec new_sig, Cell* merge_src_into); + void patch(Cell* old_cell, const std::vector> &port_replacements, Cell* merge_src_into); RTLIL::Wire *addWire(RTLIL::IdString name, int width = 1); RTLIL::Wire *addWire(RTLIL::IdString name, const RTLIL::Wire *other); diff --git a/passes/opt/opt_merge.cc b/passes/opt/opt_merge.cc index cf12b0d12..37063e74d 100644 --- a/passes/opt/opt_merge.cc +++ b/passes/opt/opt_merge.cc @@ -23,6 +23,7 @@ #include "kernel/log.h" #include "kernel/celltypes.h" #include "kernel/threading.h" +#include "kernel/unstable/patch.h" #include "libs/sha1/sha1.h" #include "passes/opt/opt_merge_common.h" #include @@ -297,6 +298,7 @@ struct OptMergeWorker for (auto [remove_cell, keep_cell] : cell_ptrs) { log_debug(" Cell `%s' is identical to cell `%s'.\n", remove_cell->name, keep_cell->name); + std::vector> port_replacements; for (auto &it : remove_cell->connections()) { if (remove_cell->output(it.first)) { RTLIL::SigSpec keep_sig = keep_cell->getPort(it.first); @@ -305,17 +307,13 @@ struct OptMergeWorker Const init = initvals(keep_sig); initvals.remove_init(it.second); initvals.remove_init(keep_sig); - module->connect(RTLIL::SigSig(it.second, keep_sig)); - auto keep_sig_it = keep_sig.begin(); - for (SigBit remove_sig_bit : it.second) { - assign_map.add(remove_sig_bit, *keep_sig_it); - ++keep_sig_it; - } initvals.set_init(keep_sig, init); + port_replacements.emplace_back(it.first, keep_sig); } } log_debug(" Removing %s cell `%s' from module `%s'.\n", remove_cell->type, remove_cell->name, module->name); - module->remove(remove_cell); + RTLIL::Patch patcher(module, &assign_map); + patcher.patch(remove_cell, port_replacements, keep_cell); total_count++; } did_something = !merged_duplicates.empty(); @@ -400,6 +398,14 @@ struct OptMergePass : public Pass { ct.cell_types.erase(ID($anyconst)); ct.cell_types.erase(ID($allseq)); ct.cell_types.erase(ID($allconst)); + // Synthetic driver cells signorm creates for module ports — must + // never be folded into one another, otherwise distinct ports collapse. + ct.cell_types.erase(ID($input_port)); + ct.cell_types.erase(ID($output_port)); + ct.cell_types.erase(ID($public)); + + // patcher.patch uses connect_incremental + fanout queries. + design->sigNormalize(true); int total_count = 0; for (auto module : design->selected_modules()) { diff --git a/passes/opt/opt_merge_inc.cc b/passes/opt/opt_merge_inc.cc index 1150041ae..39a76709d 100644 --- a/passes/opt/opt_merge_inc.cc +++ b/passes/opt/opt_merge_inc.cc @@ -22,6 +22,7 @@ #include "kernel/sigtools.h" #include "kernel/log.h" #include "kernel/newcelltypes.h" +#include "kernel/unstable/patch.h" #include "libs/sha1/sha1.h" #include "passes/opt/opt_merge_common.h" #include @@ -267,6 +268,7 @@ struct OptMergeIncWorker did_something = true; log_debug(" Cell `%s' is identical to cell `%s'.\n", cell->name, other_cell->name); + std::vector> port_replacements; for (auto &[port, sig] : cell->connections()) { if (cell->output(port)) { // TODO why was this removed before? @@ -274,14 +276,14 @@ struct OptMergeIncWorker Const init = initvals(other_sig); initvals.remove_init(sig); initvals.remove_init(other_sig); - module->connect(sig, other_cell->getPort(port)); - assign_map.add(sig, other_sig); initvals.set_init(other_sig, init); + port_replacements.emplace_back(port, other_sig); } } log_debug(" Removing %s cell `%s' from module `%s'.\n", cell->type, cell->name, module->name); - module->remove(cell); + RTLIL::Patch patcher(module, &assign_map); + patcher.patch(cell, port_replacements, other_cell); total_count++; iter_count++; } diff --git a/passes/opt/pmux2shiftx.cc b/passes/opt/pmux2shiftx.cc index 6668ff2de..38a9fb142 100644 --- a/passes/opt/pmux2shiftx.cc +++ b/passes/opt/pmux2shiftx.cc @@ -21,6 +21,7 @@ #include "kernel/sigtools.h" #include "kernel/ffinit.h" #include "kernel/utils.h" +#include "kernel/unstable/patch.h" USING_YOSYS_NAMESPACE PRIVATE_NAMESPACE_BEGIN @@ -765,15 +766,22 @@ struct OnehotPass : public Pass { } extra_args(args, argidx, design); + // Patcher.gc needs module->fanout() and connect_incremental. + design->sigNormalize(true); + for (auto module : design->selected_modules()) { SigMap sigmap(module); OnehotDatabase onehot_db(module, sigmap); onehot_db.verbose = verbose_onehot; + // Track cells removed inline by patcher.gc so the outer loop + // (and any stale pointer accesses) can skip them. + pool removed_cells; + for (auto cell : module->selected_cells()) { - if (cell->type != ID($eq)) + if (removed_cells.count(cell) || cell->type != ID($eq)) continue; SigSpec A = sigmap(cell->getPort(ID::A)); @@ -825,6 +833,7 @@ struct OnehotPass : public Pass { } SigSpec Y = cell->getPort(ID::Y); + SigSpec replacement; if (not_onehot) { @@ -832,7 +841,7 @@ struct OnehotPass : public Pass { log(" replacing with constant 0 driver.\n"); else log("Replacing one-hot $eq(%s, %s) cell %s/%s with constant 0 driver.\n", log_signal(A), log_signal(B), module, cell); - module->connect(Y, SigSpec(1, GetSize(Y))); + replacement = SigSpec(1, GetSize(Y)); } else { @@ -842,10 +851,13 @@ struct OnehotPass : public Pass { else log("Replacing one-hot $eq(%s, %s) cell %s/%s with signal %s.\n",log_signal(A), log_signal(B), module, cell, log_signal(sig)); sig.extend_u0(GetSize(Y)); - module->connect(Y, sig); + replacement = sig; } - module->remove(cell); + removed_cells.insert(cell); + RTLIL::Patch patcher(module, &sigmap); + patcher.removed_cells = &removed_cells; + patcher.patch(cell, ID::Y, replacement); } } }