patch: gc collects src from every removed cell; ff.cc routes through Patch

kernel/ff.cc

@@ -18,19 +18,10 @@
  */
 
 #include "kernel/ff.h"
+#include "kernel/unstable/patch.h"
 
 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<typename InputType, typename OutputType, typename = std::enable_if_t<std::is_base_of_v<FfTypeData, OutputType>>>
 void manufacture_info(InputType flop, OutputType& info, FfInitVals *initvals) {
@@ -494,64 +485,65 @@ void FfData::aload_to_sr() {
 	log_assert(!has_sr);
 	has_sr = true;
 	has_aload = false;
-	std::string src = ff_src(*this);
+	RTLIL::Patch patcher(module);
 	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, src);
+			sig_clr = patcher.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, src);
+			sig_set = patcher.Mux(NEW_ID, Const(State::S0, width), sig_ad, sig_aload);
 		} else {
-			sig_clr = module->Mux(NEW_ID, sig_ad, Const(State::S1, width), sig_aload, src);
+			sig_clr = patcher.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, src);
+			sig_set = patcher.Mux(NEW_ID, sig_ad, Const(State::S0, width), sig_aload);
 		}
 	} else {
 		pol_clr = pol_aload;
 		pol_set = pol_aload;
 		if (pol_aload) {
-			sig_clr = module->AndnotGate(NEW_ID, sig_aload, sig_ad, src);
+			sig_clr = patcher.AndnotGate(NEW_ID, sig_aload, sig_ad);
-			sig_set = module->AndGate(NEW_ID, sig_aload, sig_ad, src);
+			sig_set = patcher.AndGate(NEW_ID, sig_aload, sig_ad);
 		} else {
-			sig_clr = module->OrGate(NEW_ID, sig_aload, sig_ad, src);
+			sig_clr = patcher.OrGate(NEW_ID, sig_aload, sig_ad);
-			sig_set = module->OrnotGate(NEW_ID, sig_aload, sig_ad, src);
+			sig_set = patcher.OrnotGate(NEW_ID, sig_aload, sig_ad);
 		}
 	}
+	patcher.commit_inheriting_src(cell);
 }
 
 void FfData::convert_ce_over_srst(bool val) {
 	if (!has_ce || !has_srst || ce_over_srst == val)
 		return;
-	std::string src = ff_src(*this);
+	RTLIL::Patch patcher(module);
 	if (val) {
 		// sdffe to sdffce
 		if (!is_fine) {
 			if (pol_ce) {
 				if (pol_srst) {
-					sig_ce = module->Or(NEW_ID, sig_ce, sig_srst, false, src);
+					sig_ce = patcher.Or(NEW_ID, sig_ce, sig_srst);
 				} else {
-					SigSpec tmp = module->Not(NEW_ID, sig_srst, false, src);
+					SigSpec tmp = patcher.Not(NEW_ID, sig_srst);
-					sig_ce = module->Or(NEW_ID, sig_ce, tmp, false, src);
+					sig_ce = patcher.Or(NEW_ID, sig_ce, tmp);
 				}
 			} else {
 				if (pol_srst) {
-					SigSpec tmp = module->Not(NEW_ID, sig_srst, false, src);
+					SigSpec tmp = patcher.Not(NEW_ID, sig_srst);
-					sig_ce = module->And(NEW_ID, sig_ce, tmp, false, src);
+					sig_ce = patcher.And(NEW_ID, sig_ce, tmp);
 				} else {
-					sig_ce = module->And(NEW_ID, sig_ce, sig_srst, false, src);
+					sig_ce = patcher.And(NEW_ID, sig_ce, sig_srst);
 				}
 			}
 		} else {
 			if (pol_ce) {
 				if (pol_srst) {
-					sig_ce = module->OrGate(NEW_ID, sig_ce, sig_srst, src);
+					sig_ce = patcher.OrGate(NEW_ID, sig_ce, sig_srst);
 				} else {
-					sig_ce = module->OrnotGate(NEW_ID, sig_ce, sig_srst, src);
+					sig_ce = patcher.OrnotGate(NEW_ID, sig_ce, sig_srst);
 				}
 			} else {
 				if (pol_srst) {
-					sig_ce = module->AndnotGate(NEW_ID, sig_ce, sig_srst, src);
+					sig_ce = patcher.AndnotGate(NEW_ID, sig_ce, sig_srst);
 				} else {
-					sig_ce = module->AndGate(NEW_ID, sig_ce, sig_srst, src);
+					sig_ce = patcher.AndGate(NEW_ID, sig_ce, sig_srst);
 				}
 			}
 		}
@@ -560,35 +552,36 @@ 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, false, src);
+					sig_srst = patcher.And(NEW_ID, sig_srst, sig_ce);
 				} else {
-					SigSpec tmp = module->Not(NEW_ID, sig_ce, false, src);
+					SigSpec tmp = patcher.Not(NEW_ID, sig_ce);
-					sig_srst = cell->module->And(NEW_ID, sig_srst, tmp, false, src);
+					sig_srst = patcher.And(NEW_ID, sig_srst, tmp);
 				}
 			} else {
 				if (pol_ce) {
-					SigSpec tmp = module->Not(NEW_ID, sig_ce, false, src);
+					SigSpec tmp = patcher.Not(NEW_ID, sig_ce);
-					sig_srst = cell->module->Or(NEW_ID, sig_srst, tmp, false, src);
+					sig_srst = patcher.Or(NEW_ID, sig_srst, tmp);
 				} else {
-					sig_srst = cell->module->Or(NEW_ID, sig_srst, sig_ce, false, src);
+					sig_srst = patcher.Or(NEW_ID, sig_srst, sig_ce);
 				}
 			}
 		} else {
 			if (pol_srst) {
 				if (pol_ce) {
-					sig_srst = cell->module->AndGate(NEW_ID, sig_srst, sig_ce, src);
+					sig_srst = patcher.AndGate(NEW_ID, sig_srst, sig_ce);
 				} else {
-					sig_srst = cell->module->AndnotGate(NEW_ID, sig_srst, sig_ce, src);
+					sig_srst = patcher.AndnotGate(NEW_ID, sig_srst, sig_ce);
 				}
 			} else {
 				if (pol_ce) {
-					sig_srst = cell->module->OrnotGate(NEW_ID, sig_srst, sig_ce, src);
+					sig_srst = patcher.OrnotGate(NEW_ID, sig_srst, sig_ce);
 				} else {
-					sig_srst = cell->module->OrGate(NEW_ID, sig_srst, sig_ce, src);
+					sig_srst = patcher.OrGate(NEW_ID, sig_srst, sig_ce);
 				}
 			}
 		}
 	}
+	patcher.commit_inheriting_src(cell);
 	ce_over_srst = val;
 }
 
@@ -599,18 +592,19 @@ void FfData::unmap_ce() {
 	if (has_srst && ce_over_srst)
 		unmap_srst();
 
-	std::string src = ff_src(*this);
+	RTLIL::Patch patcher(module);
 	if (!is_fine) {
 		if (pol_ce)
-			sig_d = module->Mux(NEW_ID, sig_q, sig_d, sig_ce, src);
+			sig_d = patcher.Mux(NEW_ID, sig_q, sig_d, sig_ce);
 		else
-			sig_d = module->Mux(NEW_ID, sig_d, sig_q, sig_ce, src);
+			sig_d = patcher.Mux(NEW_ID, sig_d, sig_q, sig_ce);
 	} else {
 		if (pol_ce)
-			sig_d = module->MuxGate(NEW_ID, sig_q, sig_d, sig_ce, src);
+			sig_d = patcher.MuxGate(NEW_ID, sig_q, sig_d, sig_ce);
 		else
-			sig_d = module->MuxGate(NEW_ID, sig_d, sig_q, sig_ce, src);
+			sig_d = patcher.MuxGate(NEW_ID, sig_d, sig_q, sig_ce);
 	}
+	patcher.commit_inheriting_src(cell);
 	has_ce = false;
 }
 
@@ -620,18 +614,19 @@ void FfData::unmap_srst() {
 	if (has_ce && !ce_over_srst)
 		unmap_ce();
 
-	std::string src = ff_src(*this);
+	RTLIL::Patch patcher(module);
 	if (!is_fine) {
 		if (pol_srst)
-			sig_d = module->Mux(NEW_ID, sig_d, val_srst, sig_srst, src);
+			sig_d = patcher.Mux(NEW_ID, sig_d, val_srst, sig_srst);
 		else
-			sig_d = module->Mux(NEW_ID, val_srst, sig_d, sig_srst, src);
+			sig_d = patcher.Mux(NEW_ID, val_srst, sig_d, sig_srst);
 	} else {
 		if (pol_srst)
-			sig_d = module->MuxGate(NEW_ID, sig_d, val_srst[0], sig_srst, src);
+			sig_d = patcher.MuxGate(NEW_ID, sig_d, val_srst[0], sig_srst);
 		else
-			sig_d = module->MuxGate(NEW_ID, val_srst[0], sig_d, sig_srst, src);
+			sig_d = patcher.MuxGate(NEW_ID, val_srst[0], sig_d, sig_srst);
 	}
+	patcher.commit_inheriting_src(cell);
 	has_srst = false;
 }
 

kernel/unstable/patch.cc

@@ -45,50 +45,7 @@ RTLIL::Wire *RTLIL::Patch::addWire(RTLIL::IdString name, const RTLIL::Wire *othe
 	return wire;
 }
 
-struct SrcCollector {
+void Patch::gc(Cell* old_cell, bool track, pool<string>* src_pool) {
-	pool<Cell*> to_do;
-	pool<Cell*> done;
-	pool<string> src;
-
-	void collect_src(Cell* old_cell) {
-		if (done.count(old_cell))
-			return;
-		done.insert(old_cell);
-
-		log_debug("collect %s\n", old_cell->name);
-		src.insert(old_cell->get_src_attribute());
-
-		std::vector<Cell*> input_cells = {};
-		for (auto [port_name, sig] : old_cell->connections()) {
-			auto dir = old_cell->port_dir(port_name);
-			log_assert(dir != PD_UNKNOWN);
-			if (dir == PD_INPUT || dir == PD_INOUT) {
-				if (sig.size() && sig.is_wire()) {
-					Wire* in_wire = sig.as_wire();
-					if (!in_wire->module)
-						input_cells.push_back(in_wire->driverCell());
-					// if (!leaves.count(in_wire))
-				}
-			}
-		}
-		for (auto input : input_cells)
-			collect_src(input);
-	}
-
-	void collect_src(SigSpec old_sig) {
-		log_debug("collect %s\n", log_signal(old_sig));
-		for (auto bit : old_sig) {
-			if (bit.is_wire() && bit.wire->module) {
-				log_assert(bit.wire->driverCell_);
-				to_do.insert(bit.wire->driverCell_);
-			}
-		}
-		for (auto cell : to_do)
-			collect_src(cell);
-	}
-};
-
-void Patch::gc(Cell* old_cell, bool track) {
 	log_debug("gc %s\n", old_cell->name);
 	if (old_cell->type.in(ID($input_port), ID($output_port), ID($public)))
 		return;
@@ -128,9 +85,14 @@ void Patch::gc(Cell* old_cell, bool track) {
 	// caller's current iteration variable and won't be re-encountered.
 	if (track && removed_cells)
 		removed_cells->insert(old_cell);
+	// The cell about to be removed contributes its src so all the cells gc'd
+	// in this patch (top-level + input cone) get merged into the new cells'
+	// src — that's the "transfer src automatically" guarantee.
+	if (src_pool)
+		src_pool->insert(old_cell->get_src_attribute());
 	old_cell->module->remove(old_cell);
 	for (auto input : inputs)
-		gc(input, /*track=*/true);
+		gc(input, /*track=*/true, src_pool);
 }
 
 Wire* Patch::commit_wire(std::unique_ptr<Wire> wire) {
@@ -172,26 +134,6 @@ void Patch::patch(Cell* old_cell, const std::vector<std::pair<IdString, SigSpec>
 		old_sigs.push_back(old_sig);
 	}
 
-	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
 	// gc() stops at them. Detected via bit.wire->module being non-null:
 	// uncommitted wires belong to no module yet.
@@ -208,19 +150,18 @@ void Patch::patch(Cell* old_cell, const std::vector<std::pair<IdString, SigSpec>
 	}
 
 	// Commit new cells/wires first so new_sig becomes a driven signal in the
-	// signorm index before we merge.
+	// signorm index before we merge. Track raw pointers so we can update
+	// their src attribute after gc finishes collecting from removed cells.
+	std::vector<Cell*> committed_new_cells;
+	committed_new_cells.reserve(cells_.size());
 	for (auto& cell: cells_) {
-		cell->set_src_attribute(src_str);
 		cell->fixup_parameters();
-		commit_cell(std::move(cell));
+		committed_new_cells.push_back(commit_cell(std::move(cell)));
 	}
 
 	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.
@@ -233,10 +174,37 @@ void Patch::patch(Cell* old_cell, const std::vector<std::pair<IdString, SigSpec>
 		mod->connect_incremental(old_sigs[i], new_sig);
 	}
 
-	gc(old_cell);
+	// gc removes old_cell AND any newly-dead input-cone cells, contributing
+	// each removed cell's src into the pool. The merged-into cell (e.g. an
+	// opt_merge keep_cell) and any caller-bequeathed pool entries also get
+	// folded in here.
+	pool<string> src_pool;
+	if (merge_src_into)
+		src_pool.insert(merge_src_into->get_src_attribute());
+	gc(old_cell, /*track=*/false, &src_pool);
+
+	std::string src_str = AttrObject::strpool_attribute_to_str(src_pool);
+	for (Cell* c : committed_new_cells)
+		c->set_src_attribute(src_str);
+	if (merge_src_into)
+		merge_src_into->set_src_attribute(src_str);
+
 	cells_.clear();
 	wires_.clear();
 	leaves.clear();
 }
 
+void Patch::commit_inheriting_src(Cell* src_source) {
+	std::string src = src_source ? src_source->get_src_attribute() : std::string();
+	for (auto& cell : cells_) {
+		cell->set_src_attribute(src);
+		cell->fixup_parameters();
+		commit_cell(std::move(cell));
+	}
+	for (auto& wire : wires_)
+		commit_wire(std::move(wire));
+	cells_.clear();
+	wires_.clear();
+}
+
 YOSYS_NAMESPACE_END

kernel/unstable/patch.h

@@ -10,7 +10,7 @@ YOSYS_NAMESPACE_BEGIN
 struct RTLIL::Patch : public CellAdderMixin<RTLIL::Patch>
 {
 private:
-	void gc(Cell* old_cell, bool track = false);
+	void gc(Cell* old_cell, bool track = false, pool<std::string>* src_pool = nullptr);
 
 protected:
 	void add(RTLIL::Wire *wire);
@@ -42,6 +42,14 @@ public:
 	// 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<std::pair<IdString, SigSpec>> &port_replacements, Cell* merge_src_into);
+
+	// Flush staged cells_ / wires_ into the module without doing any
+	// connect_incremental or gc. Each committed cell's src attribute is
+	// pulled from `src_source` (typically the cell that's being expanded /
+	// unmapped into the staged helpers, so source-location tracking carries
+	// through transparently). Pass nullptr for src_source if the staged
+	// helpers have no natural ancestor.
+	void commit_inheriting_src(Cell* src_source);
 	RTLIL::Wire *addWire(RTLIL::IdString name, int width = 1);
 	RTLIL::Wire *addWire(RTLIL::IdString name, const RTLIL::Wire *other);