From bc7895505e1908d25916212cf55b5fdc131d5526 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 10 Oct 2025 01:10:33 +0000 Subject: [PATCH] Implement IdString garbage collection instead of refcounting. --- kernel/register.cc | 30 +++++- kernel/register.h | 30 ++++++ kernel/rtlil.cc | 122 +++++++++++++++++++++++ kernel/rtlil.h | 170 ++++++++++++++++----------------- passes/opt/opt_clean.cc | 4 + tests/unit/kernel/rtlilTest.cc | 6 ++ 6 files changed, 272 insertions(+), 90 deletions(-) diff --git a/kernel/register.cc b/kernel/register.cc index bd12dcc38..1f8e4a9a5 100644 --- a/kernel/register.cc +++ b/kernel/register.cc @@ -42,6 +42,23 @@ std::map backend_register; std::vector 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 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(); } diff --git a/kernel/register.h b/kernel/register.h index 534cfbc28..b9c709dc1 100644 --- a/kernel/register.h +++ b/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 diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index d1cf7f23e..c78bd00bb 100644 --- a/kernel/rtlil.cc +++ b/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 void trace(const T* v) { + trace(*v); + } + template void trace(const std::vector &v) { + for (const auto &element : v) + trace(element); + } + template void trace(const pool &p) { + for (const auto &element : p) + trace(element); + } + template void trace(const dict &d) { + for (const auto &[key, value] : d) { + trace(key); + trace(value); + } + } + template void trace_keys(const dict &d) { + for (const auto &[key, value] : d) { + trace(key); + } + } + template void trace_values(const dict &d) { + for (const auto &[key, value] : d) { + trace(value); + } + } + template void trace(const idict &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 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(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 RTLIL::constpad; static const pool &builtin_ff_cell_types_internal() { diff --git a/kernel/rtlil.h b/kernel/rtlil.h index f161ef445..92a0dffb5 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -126,12 +126,12 @@ namespace RTLIL typedef std::pair 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(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(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(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(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(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(StaticId id) : IdString(id) { } + 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(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(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; \ }) diff --git a/passes/opt/opt_clean.cc b/passes/opt/opt_clean.cc index 996a9b3c9..b3e3cd33a 100644 --- a/passes/opt/opt_clean.cc +++ b/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; diff --git a/tests/unit/kernel/rtlilTest.cc b/tests/unit/kernel/rtlilTest.cc index 557355ed9..8bba9ac28 100644 --- a/tests/unit/kernel/rtlilTest.cc +++ b/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>