Implement IdString garbage collection instead of refcounting.

kernel/register.cc

@@ -42,6 +42,23 @@ std::map<std::string, Backend*> backend_register;
 
 std::vector<std::string> Frontend::next_args;
 
+bool GarbageCollectionGuard::is_enabled_ = true;
+
+static bool garbage_collection_requested = false;
+
+void request_garbage_collection()
+{
+	garbage_collection_requested = true;
+}
+
+void try_collect_garbage()
+{
+	if (!GarbageCollectionGuard::is_enabled() || !garbage_collection_requested)
+		return;
+	garbage_collection_requested = false;
+	RTLIL::OwningIdString::collect_garbage();
+}
+
 Pass::Pass(std::string name, std::string short_help, source_location location) : 
 	pass_name(name), short_help(short_help), location(location)
 {
@@ -263,14 +280,19 @@ void Pass::call(RTLIL::Design *design, std::vector<std::string> args)
 
 	if (pass_register.count(args[0]) == 0)
 		log_cmd_error("No such command: %s (type 'help' for a command overview)\n", args[0]);
+	Pass *pass = pass_register[args[0]];
 
-	if (pass_register[args[0]]->experimental_flag)
+	// Collect garbage before the next pass if requested. No need to collect garbage after the last pass.
+	try_collect_garbage();
+	GarbageCollectionGuard gc_guard(pass->allow_garbage_collection_during_pass());
+
+	if (pass->experimental_flag)
 		log_experimental(args[0]);
 
 	size_t orig_sel_stack_pos = design->selection_stack.size();
-	auto state = pass_register[args[0]]->pre_execute();
-	pass_register[args[0]]->execute(args, design);
-	pass_register[args[0]]->post_execute(state);
+	auto state = pass->pre_execute();
+	pass->execute(args, design);
+	pass->post_execute(state);
 	while (design->selection_stack.size() > orig_sel_stack_pos)
 		design->pop_selection();
 }

kernel/register.h

@@ -50,6 +50,30 @@ struct source_location { // dummy placeholder
 
 YOSYS_NAMESPACE_BEGIN
 
+// Track whether garbage collection is enabled. Garbage collection must be disabled
+// while any RTLIL objects (e.g. non-owning non-immortal IdStrings) exist outside Designs.
+// Garbage collection is disabled whenever any GarbageCollectionGuard(false) is on the
+// stack. These objects must be stack-allocated on the main thread.
+class GarbageCollectionGuard
+{
+	bool was_enabled;
+	static bool is_enabled_;
+public:
+	GarbageCollectionGuard(bool allow) : was_enabled(is_enabled_) {
+		is_enabled_ &= allow;
+	}
+	~GarbageCollectionGuard() {
+		is_enabled_ = was_enabled;
+	}
+	static bool is_enabled() { return is_enabled_; }
+};
+
+// Call from anywhere to request GC at the next safe point.
+void request_garbage_collection();
+
+// GC if GarbageCollectionGuard::is_enabled() and GC was requested.
+void try_collect_garbage();
+
 struct Pass
 {
 	std::string pass_name, short_help;
@@ -108,6 +132,10 @@ struct Pass
 	virtual void on_register();
 	virtual void on_shutdown();
 	virtual bool replace_existing_pass() const { return false; }
+
+	// This should return false if the pass holds onto RTLIL objects outside a Design while it
+	// calls nested passes. For safety, we default to assuming the worst.
+	virtual bool allow_garbage_collection_during_pass() const { return false; }
 };
 
 struct ScriptPass : Pass
@@ -126,6 +154,8 @@ struct ScriptPass : Pass
 	void run_nocheck(std::string command, std::string info = std::string());
 	void run_script(RTLIL::Design *design, std::string run_from = std::string(), std::string run_to = std::string());
 	void help_script();
+
+	bool allow_garbage_collection_during_pass() const override { return true; }
 };
 
 struct Frontend : Pass

kernel/rtlil.cc

@@ -82,6 +82,128 @@ static constexpr bool check_well_known_id_order()
 // and in sorted ascii order, as required by the ID macro.
 static_assert(check_well_known_id_order());
 
+struct IdStringCollector {
+	IdStringCollector(int size) : live(size, false) {}
+
+	void trace(IdString id) { live[id.index_] = true; }
+	template <typename T> void trace(const T* v) {
+		trace(*v);
+	}
+	template <typename V> void trace(const std::vector<V> &v) {
+		for (const auto &element : v)
+			trace(element);
+	}
+	template <typename K> void trace(const pool<K> &p) {
+		for (const auto &element : p)
+			trace(element);
+	}
+	template <typename K, typename V> void trace(const dict<K, V> &d) {
+		for (const auto &[key, value] : d) {
+			trace(key);
+			trace(value);
+		}
+	}
+	template <typename K, typename V> void trace_keys(const dict<K, V> &d) {
+		for (const auto &[key, value] : d) {
+			trace(key);
+		}
+	}
+	template <typename K, typename V> void trace_values(const dict<K, V> &d) {
+		for (const auto &[key, value] : d) {
+			trace(value);
+		}
+	}
+	template <typename K> void trace(const idict<K> &d) {
+		for (const auto &element : d)
+			trace(element);
+	}
+
+	void trace(const RTLIL::Design &design) {
+		trace_values(design.modules_);
+		trace(design.selection_vars);
+	}
+	void trace(const RTLIL::Selection &selection_var) {
+		trace(selection_var.selected_modules);
+		trace(selection_var.selected_members);
+	}
+	void trace_named(const RTLIL::NamedObject named) {
+		trace_keys(named.attributes);
+		trace(named.name);
+	}
+	void trace(const RTLIL::Module &module) {
+		trace_named(module);
+		trace_values(module.wires_);
+		trace_values(module.cells_);
+		trace(module.avail_parameters);
+		trace_keys(module.parameter_default_values);
+		trace_values(module.memories);
+		trace_values(module.processes);
+	}
+	void trace(const RTLIL::Wire &wire) {
+		trace_named(wire);
+		if (wire.known_driver())
+			trace(wire.driverPort());
+	}
+	void trace(const RTLIL::Cell &cell) {
+		trace_named(cell);
+		trace(cell.type);
+		trace_keys(cell.connections_);
+		trace_keys(cell.parameters);
+	}
+	void trace(const RTLIL::Memory &mem) {
+		trace_named(mem);
+	}
+	void trace(const RTLIL::Process &proc) {
+		trace_named(proc);
+		trace(proc.root_case);
+		trace(proc.syncs);
+	}
+	void trace(const RTLIL::CaseRule &rule) {
+		trace_keys(rule.attributes);
+		trace(rule.switches);
+	}
+	void trace(const RTLIL::SwitchRule &rule) {
+		trace_keys(rule.attributes);
+		trace(rule.cases);
+	}
+	void trace(const RTLIL::SyncRule &rule) {
+		trace(rule.mem_write_actions);
+	}
+	void trace(const RTLIL::MemWriteAction &action) {
+		trace_keys(action.attributes);
+		trace(action.memid);
+	}
+
+	std::vector<bool> live;
+};
+
+void RTLIL::OwningIdString::collect_garbage()
+{
+#ifndef YOSYS_NO_IDS_REFCNT
+	int size = GetSize(global_refcount_storage_);
+	IdStringCollector collector(size);
+	for (auto &[idx, design] : *RTLIL::Design::get_all_designs()) {
+		collector.trace(*design);
+	}
+	for (int i = static_cast<int>(StaticId::STATIC_ID_END); i < size; ++i) {
+		if (collector.live[i] || global_refcount_storage_[i] > 0)
+			continue;
+		RTLIL::IdString::Storage &storage = global_id_storage_.at(i);
+		if (storage.buf == nullptr)
+			continue;
+		if (yosys_xtrace) {
+			log("#X# Removed IdString '%s' with index %d.\n", storage.buf, i);
+			log_backtrace("-X- ", yosys_xtrace-1);
+		}
+
+		global_id_index_.erase(std::string_view(storage.buf, storage.size));
+		free(storage.buf);
+		storage = {nullptr, 0};
+		global_free_idx_list_.push_back(i);
+	}
+#endif
+}
+
 dict<std::string, std::string> RTLIL::constpad;
 
 static const pool<IdString> &builtin_ff_cell_types_internal() {

kernel/rtlil.h

@@ -126,12 +126,12 @@ namespace RTLIL
 	typedef std::pair<SigSpec, SigSpec> SigSig;
 
 	struct StaticIdString {
-		constexpr StaticIdString(StaticId id, const IdString &id_str) : id_str(id_str), id(id) {}
-		constexpr inline operator const IdString &() const { return id_str; }
+		constexpr StaticIdString(StaticId id, const OwningIdString &id_str) : id_str(id_str), id(id) {}
+		constexpr inline operator const OwningIdString &() const { return id_str; }
 		constexpr inline int index() const { return static_cast<short>(id); }
-		constexpr inline const IdString &id_string() const { return id_str; }
+		constexpr inline const OwningIdString &id_string() const { return id_str; }
 
-		const IdString &id_str;
+		const OwningIdString &id_str;
 		const StaticId id;
 	};
 };
@@ -188,32 +188,12 @@ struct RTLIL::IdString
 	#endif
 	}
 
-	static inline int get_reference(int idx)
-	{
-	#ifndef YOSYS_NO_IDS_REFCNT
-		global_refcount_storage_[idx]++;
-	#endif
-	#ifdef YOSYS_XTRACE_GET_PUT
-		if (yosys_xtrace && idx >= static_cast<short>(StaticId::STATIC_ID_END))
-			log("#X# GET-BY-INDEX '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx).buf, idx, global_refcount_storage_.at(idx));
-	#endif
-		return idx;
-	}
-
-	static int get_reference(const char *p)
-	{
-		return get_reference(std::string_view(p));
-	}
-
-	static int get_reference(std::string_view p)
+	static int insert(std::string_view p)
 	{
 		log_assert(destruct_guard_ok);
 
 		auto it = global_id_index_.find(p);
 		if (it != global_id_index_.end()) {
-	#ifndef YOSYS_NO_IDS_REFCNT
-			global_refcount_storage_.at(it->second)++;
-	#endif
 	#ifdef YOSYS_XTRACE_GET_PUT
 			if (yosys_xtrace)
 				log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(it->second).buf, it->second, global_refcount_storage_.at(it->second));
@@ -238,7 +218,6 @@ struct RTLIL::IdString
 
 		int idx = global_free_idx_list_.back();
 		global_free_idx_list_.pop_back();
-		global_refcount_storage_.at(idx)++;
 	#else
 		int idx = global_id_storage_.size();
 		global_id_storage_.push_back({nullptr, 0});
@@ -262,67 +241,19 @@ struct RTLIL::IdString
 		return idx;
 	}
 
-#ifndef YOSYS_NO_IDS_REFCNT
-	static inline void put_reference(int idx)
-	{
-		// put_reference() may be called from destructors after the destructor of
-		// global_refcount_storage_ has been run. in this case we simply do nothing.
-		if (idx < static_cast<short>(StaticId::STATIC_ID_END) || !destruct_guard_ok)
-			return;
-
-	#ifdef YOSYS_XTRACE_GET_PUT
-		if (yosys_xtrace) {
-			log("#X# PUT '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx).buf, idx, global_refcount_storage_.at(idx));
-		}
-	#endif
-
-		uint32_t &refcount = global_refcount_storage_[idx];
-
-		if (--refcount > 0)
-			return;
-
-		free_reference(idx);
-	}
-	static inline void free_reference(int idx)
-	{
-		if (yosys_xtrace) {
-			log("#X# Removed IdString '%s' with index %d.\n", global_id_storage_.at(idx).buf, idx);
-			log_backtrace("-X- ", yosys_xtrace-1);
-		}
-		log_assert(idx >= static_cast<short>(StaticId::STATIC_ID_END));
-
-		global_id_index_.erase(global_id_storage_.at(idx).buf);
-		free(global_id_storage_.at(idx).buf);
-		global_id_storage_.at(idx) = {nullptr, 0};
-		global_free_idx_list_.push_back(idx);
-	}
-#else
-	static inline void put_reference(int) { }
-#endif
-
 	// the actual IdString object is just is a single int
 
 	int index_;
 
 	inline IdString() : index_(0) { }
-	inline IdString(const char *str) : index_(get_reference(str)) { }
-	inline IdString(const IdString &str) : index_(get_reference(str.index_)) { }
+	inline IdString(const char *str) : index_(insert(std::string_view(str))) { }
+	inline IdString(const IdString &str) : index_(str.index_) { }
 	inline IdString(IdString &&str) : index_(str.index_) { str.index_ = 0; }
-	inline IdString(const std::string &str) : index_(get_reference(std::string_view(str))) { }
-	inline IdString(std::string_view str) : index_(get_reference(str)) { }
+	inline IdString(const std::string &str) : index_(insert(std::string_view(str))) { }
+	inline IdString(std::string_view str) : index_(insert(str)) { }
 	inline IdString(StaticId id) : index_(static_cast<short>(id)) {}
-	inline ~IdString() { put_reference(index_); }
 
-	inline void operator=(const IdString &rhs) {
-		put_reference(index_);
-		index_ = get_reference(rhs.index_);
-	}
-
-	inline void operator=(IdString &&rhs) {
-		put_reference(index_);
-		index_ = rhs.index_;
-		rhs.index_ = 0;
-	}
+	IdString &operator=(const IdString &rhs) = default;
 
 	inline void operator=(const char *rhs) {
 		IdString id(rhs);
@@ -462,12 +393,78 @@ public:
 
 struct RTLIL::OwningIdString : public RTLIL::IdString {
 	inline OwningIdString() { }
-	inline OwningIdString(const char *str) : IdString(str) { }
-	inline OwningIdString(const IdString &str) : IdString(str) { }
-	inline OwningIdString(IdString &&str) : IdString(str) { }
-	inline OwningIdString(const std::string &str) : IdString(str) { }
-	inline OwningIdString(std::string_view str) : IdString(str) { }
+	inline OwningIdString(const StaticIdString &str) : IdString(str) { }
+	inline OwningIdString(const OwningIdString &str) : IdString(str) { get_reference(); }
+	inline OwningIdString(const char *str) : IdString(str) { get_reference(); }
+	inline OwningIdString(const IdString &str) : IdString(str) { get_reference(); }
+	inline OwningIdString(IdString &&str) : IdString(str) {	get_reference(); }
+	inline OwningIdString(const std::string &str) : IdString(str) { get_reference(); }
+	inline OwningIdString(std::string_view str) : IdString(str) { get_reference(); }
 	inline OwningIdString(StaticId id) : IdString(id) {}
+	inline ~OwningIdString() {
+		put_reference();
+	}
+
+	inline OwningIdString &operator=(const OwningIdString &rhs) {
+		put_reference();
+		index_ = rhs.index_;
+		get_reference();
+		return *this;
+	}
+	inline OwningIdString &operator=(const IdString &rhs) {
+		put_reference();
+		index_ = rhs.index_;
+		get_reference();
+		return *this;
+	}
+	inline OwningIdString &operator=(OwningIdString &&rhs) {
+		std::swap(index_, rhs.index_);
+		return *this;
+	}
+
+	// Collect all non-owning references.
+	static void collect_garbage();
+
+	// Used by the ID() macro to create an IdString with no destructor whose string will
+	// never be released. If ID() creates a closure-static `OwningIdString` then
+	// initialization of the static registers its destructor to run at exit, which is
+	// wasteful.
+	static IdString immortal(const char* str) {
+		IdString result(str);
+		get_reference(result.index_);
+		return result;
+	}
+private:
+	void get_reference()
+	{
+		get_reference(index_);
+	}
+	static void get_reference(int idx)
+	{
+	#ifndef YOSYS_NO_IDS_REFCNT
+		global_refcount_storage_[idx]++;
+	#endif
+	#ifdef YOSYS_XTRACE_GET_PUT
+		if (yosys_xtrace && idx >= static_cast<short>(StaticId::STATIC_ID_END))
+			log("#X# GET-BY-INDEX '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx), idx, global_refcount_storage_.at(idx));
+	#endif
+	}
+
+	void put_reference()
+	{
+	#ifndef YOSYS_NO_IDS_REFCNT
+		// put_reference() may be called from destructors after the destructor of
+		// global_refcount_storage_ has been run. in this case we simply do nothing.
+		if (index_ < static_cast<short>(StaticId::STATIC_ID_END) || !destruct_guard_ok)
+			return;
+	#ifdef YOSYS_XTRACE_GET_PUT
+		if (yosys_xtrace) {
+			log("#X# PUT '%s' (index %d, refcount %u)\n", global_id_storage_.at(index_), index_, global_refcount_storage_.at(index_));
+		}
+	#endif
+		--global_refcount_storage_[index_];
+	#endif
+	}
 };
 
 namespace hashlib {
@@ -569,7 +566,8 @@ template <> struct IDMacroHelper<-1> {
 		>::eval([]() \
 		-> const YOSYS_NAMESPACE_PREFIX RTLIL::IdString & { \
 			const char *p = "\\" #_id, *q = p[1] == '$' ? p+1 : p; \
-			static const YOSYS_NAMESPACE_PREFIX RTLIL::IdString id(q); \
+			static const YOSYS_NAMESPACE_PREFIX RTLIL::IdString id = \
+				YOSYS_NAMESPACE_PREFIX RTLIL::OwningIdString::immortal(q); \
 			return id; \
         })
 

passes/opt/opt_clean.cc

@@ -722,6 +722,8 @@ struct OptCleanPass : public Pass {
 		ct_reg.clear();
 		ct_all.clear();
 		log_pop();
+
+		request_garbage_collection();
 	}
 } OptCleanPass;
 
@@ -784,6 +786,8 @@ struct CleanPass : public Pass {
 		keep_cache.reset();
 		ct_reg.clear();
 		ct_all.clear();
+
+		request_garbage_collection();
 	}
 } CleanPass;
 

tests/unit/kernel/rtlilTest.cc

@@ -361,6 +361,12 @@ namespace RTLIL {
 		EXPECT_FALSE(Const().is_onehot(&pos));
 	}
 
+	TEST_F(KernelRtlilTest, OwningIdString) {
+		OwningIdString own("\\figblortle");
+		OwningIdString::collect_garbage();
+		EXPECT_EQ(own.str(), "\\figblortle");
+	}
+
 	class WireRtlVsHdlIndexConversionTest :
 		public KernelRtlilTest,
 		public testing::WithParamInterface<std::tuple<bool, int, int>>