From 2319d82efbec1f9f1f06fd3d7a338aafa4a0904a Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Thu, 16 Oct 2025 03:37:49 +0000 Subject: [PATCH 01/19] Make IdString::begins_width/ends_with take std::string_view so we avoid a strlen when the parameter is a string constant --- kernel/rtlil.h | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 27f610f85..85e89686e 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -405,16 +405,15 @@ struct RTLIL::IdString return strncmp(c_str()+pos, s, len); } - bool begins_with(const char* prefix) const { - size_t len = strlen(prefix); - if (size() < len) return false; - return compare(0, len, prefix) == 0; + bool begins_with(std::string_view prefix) const { + if (size() < prefix.size()) return false; + return compare(0, prefix.size(), prefix.data()) == 0; } - bool ends_with(const char* suffix) const { - size_t len = strlen(suffix); - if (size() < len) return false; - return compare(size()-len, len, suffix) == 0; + bool ends_with(std::string_view suffix) const { + size_t sz = size(); + if (sz < suffix.size()) return false; + return compare(sz - suffix.size(), suffix.size(), suffix.data()) == 0; } bool contains(const char* str) const { From 32641bbf935a44597c50d285beacb812f18d9075 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Thu, 16 Oct 2025 04:05:56 +0000 Subject: [PATCH 02/19] Make IdString::contains take std::string_view so we avoid a strlen when the parameter is a string constant --- kernel/rtlil.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 85e89686e..b6104e0e5 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -416,8 +416,8 @@ struct RTLIL::IdString return compare(sz - suffix.size(), suffix.size(), suffix.data()) == 0; } - bool contains(const char* str) const { - return strstr(c_str(), str); + bool contains(std::string_view s) const { + return std::string_view(c_str()).find(s) != std::string::npos; } size_t size() const { From 20639906e3c645a9ae73076169e4ead98bdd4ab1 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Wed, 20 Aug 2025 03:47:03 +0000 Subject: [PATCH 03/19] Store IdString lengths and use them --- kernel/io.cc | 2 +- kernel/rtlil.cc | 6 ++--- kernel/rtlil.h | 58 +++++++++++++++++++++++++++++-------------------- 3 files changed, 38 insertions(+), 28 deletions(-) diff --git a/kernel/io.cc b/kernel/io.cc index e9801f63e..e713cff85 100644 --- a/kernel/io.cc +++ b/kernel/io.cc @@ -606,7 +606,7 @@ void format_emit_idstring(std::string &result, std::string_view spec, int *dynam { if (spec == "%s") { // Format checking will have guaranteed num_dynamic_ints == 0. - result += arg.c_str(); + result += arg.str_view(); return; } format_emit_stringf(result, spec, dynamic_ints, num_dynamic_ints, arg.c_str()); diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index a58487216..5dac6016c 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -35,7 +35,7 @@ YOSYS_NAMESPACE_BEGIN bool RTLIL::IdString::destruct_guard_ok = false; RTLIL::IdString::destruct_guard_t RTLIL::IdString::destruct_guard; -std::vector RTLIL::IdString::global_id_storage_; +std::vector RTLIL::IdString::global_id_storage_; std::unordered_map RTLIL::IdString::global_id_index_; #ifndef YOSYS_NO_IDS_REFCNT std::vector RTLIL::IdString::global_refcount_storage_; @@ -57,14 +57,14 @@ static void populate(std::string_view name) name = name.substr(1); } RTLIL::IdString::global_id_index_.insert({name, GetSize(RTLIL::IdString::global_id_storage_)}); - RTLIL::IdString::global_id_storage_.push_back(const_cast(name.data())); + RTLIL::IdString::global_id_storage_.push_back({const_cast(name.data()), GetSize(name)}); } void RTLIL::IdString::prepopulate() { int size = static_cast(RTLIL::StaticId::STATIC_ID_END); global_id_storage_.reserve(size); - RTLIL::IdString::global_id_storage_.push_back(const_cast("")); + RTLIL::IdString::global_id_storage_.push_back({const_cast(""), 0}); global_id_index_.reserve(size); global_refcount_storage_.resize(size, 1); #define X(N) populate("\\" #N); diff --git a/kernel/rtlil.h b/kernel/rtlil.h index b6104e0e5..45d696321 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -138,6 +138,11 @@ namespace RTLIL struct RTLIL::IdString { + struct Storage { + char *buf; + int size; + }; + #undef YOSYS_XTRACE_GET_PUT #undef YOSYS_SORT_ID_FREE_LIST #undef YOSYS_USE_STICKY_IDS @@ -151,7 +156,7 @@ struct RTLIL::IdString ~destruct_guard_t() { destruct_guard_ok = false; } } destruct_guard; - static std::vector global_id_storage_; + static std::vector global_id_storage_; static std::unordered_map global_id_index_; #ifndef YOSYS_NO_IDS_REFCNT // For prepopulated IdStrings, the refcount is meaningless since they @@ -174,10 +179,10 @@ struct RTLIL::IdString #ifdef YOSYS_XTRACE_GET_PUT for (int idx = 0; idx < GetSize(global_id_storage_); idx++) { - if (global_id_storage_.at(idx) == nullptr) + if (global_id_storage_.at(idx).buf == nullptr) log("#X# DB-DUMP index %d: FREE\n", idx); else - log("#X# DB-DUMP index %d: '%s' (ref %u)\n", idx, global_id_storage_.at(idx), global_refcount_storage_.at(idx)); + log("#X# DB-DUMP index %d: '%s' (ref %u)\n", idx, global_id_storage_.at(idx).buf, global_refcount_storage_.at(idx)); } #endif } @@ -204,7 +209,7 @@ struct RTLIL::IdString #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)); + 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; } @@ -225,7 +230,7 @@ struct RTLIL::IdString #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), it->second, global_refcount_storage_.at(it->second)); + 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)); #endif return it->second; } @@ -244,32 +249,31 @@ struct RTLIL::IdString if (global_free_idx_list_.empty()) { log_assert(global_id_storage_.size() < 0x40000000); global_free_idx_list_.push_back(global_id_storage_.size()); - global_id_storage_.push_back(nullptr); + global_id_storage_.push_back({nullptr, 0}); global_refcount_storage_.push_back(0); } int idx = global_free_idx_list_.back(); global_free_idx_list_.pop_back(); - char* buf = static_cast(malloc(p.size() + 1)); - memcpy(buf, p.data(), p.size()); - buf[p.size()] = 0; - global_id_storage_.at(idx) = buf; - global_id_index_.insert(it, {std::string_view(buf, p.size()), idx}); global_refcount_storage_.at(idx)++; #else int idx = global_id_storage_.size(); - global_id_storage_.push_back(strdup(p)); - global_id_index_[global_id_storage_.back()] = idx; + global_id_storage_.push_back({nullptr, 0}); #endif + char* buf = static_cast(malloc(p.size() + 1)); + memcpy(buf, p.data(), p.size()); + buf[p.size()] = 0; + global_id_storage_.at(idx) = {buf, GetSize(p)}; + global_id_index_.insert(it, {std::string_view(buf, p.size()), idx}); if (yosys_xtrace) { - log("#X# New IdString '%s' with index %d.\n", global_id_storage_.at(idx), idx); + log("#X# New IdString '%s' with index %d.\n", global_id_storage_.at(idx).buf, idx); log_backtrace("-X- ", yosys_xtrace-1); } #ifdef YOSYS_XTRACE_GET_PUT if (yosys_xtrace) - log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx), idx, global_refcount_storage_.at(idx)); + log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx).buf, idx, global_refcount_storage_.at(idx)); #endif #ifdef YOSYS_USE_STICKY_IDS @@ -294,7 +298,7 @@ struct RTLIL::IdString #ifdef YOSYS_XTRACE_GET_PUT if (yosys_xtrace) { - log("#X# PUT '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx), idx, global_refcount_storage_.at(idx)); + log("#X# PUT '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx).buf, idx, global_refcount_storage_.at(idx)); } #endif @@ -308,14 +312,14 @@ struct RTLIL::IdString static inline void free_reference(int idx) { if (yosys_xtrace) { - log("#X# Removed IdString '%s' with index %d.\n", global_id_storage_.at(idx), idx); + 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)); - free(global_id_storage_.at(idx)); - global_id_storage_.at(idx) = nullptr; + 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 @@ -359,11 +363,17 @@ struct RTLIL::IdString constexpr inline const IdString &id_string() const { return *this; } inline const char *c_str() const { - return global_id_storage_.at(index_); + return global_id_storage_.at(index_).buf; } inline std::string str() const { - return std::string(global_id_storage_.at(index_)); + const Storage &storage = global_id_storage_.at(index_); + return std::string(storage.buf, storage.size); + } + + inline std::string_view str_view() const { + const Storage &storage = global_id_storage_.at(index_); + return std::string_view(storage.buf, storage.size); } inline bool operator<(const IdString &rhs) const { @@ -395,7 +405,7 @@ struct RTLIL::IdString } std::string substr(size_t pos = 0, size_t len = std::string::npos) const { - if (len == std::string::npos || len >= strlen(c_str() + pos)) + if (len == std::string::npos || len + pos >= size()) return std::string(c_str() + pos); else return std::string(c_str() + pos, len); @@ -421,7 +431,7 @@ struct RTLIL::IdString } size_t size() const { - return strlen(c_str()); + return global_id_storage_.at(index_).size; } bool empty() const { From e84bc3c6c542f18bbc53ee21ecbbd0b1e1f9bb0d Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Thu, 16 Oct 2025 02:13:33 +0000 Subject: [PATCH 04/19] Remove explicit empty-string check when looking up IdStrings --- kernel/rtlil.cc | 3 ++- kernel/rtlil.h | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 5dac6016c..76744836d 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -64,9 +64,10 @@ void RTLIL::IdString::prepopulate() { int size = static_cast(RTLIL::StaticId::STATIC_ID_END); global_id_storage_.reserve(size); - RTLIL::IdString::global_id_storage_.push_back({const_cast(""), 0}); global_id_index_.reserve(size); global_refcount_storage_.resize(size, 1); + RTLIL::IdString::global_id_index_.insert({"", 0}); + RTLIL::IdString::global_id_storage_.push_back({const_cast(""), 0}); #define X(N) populate("\\" #N); #include "kernel/constids.inc" #undef X diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 45d696321..e005ac65e 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -237,9 +237,6 @@ struct RTLIL::IdString ensure_prepopulated(); - if (p.empty()) - return 0; - log_assert(p[0] == '$' || p[0] == '\\'); for (char ch : p) if ((unsigned)ch <= (unsigned)' ') From 0fe79ce01bc5df9cc23793875a0c9e5be05e7995 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Thu, 9 Oct 2025 22:54:26 +0000 Subject: [PATCH 05/19] Make RTLIL::Design::get_all_designs() unconditionally defined --- kernel/rtlil.cc | 6 ------ kernel/rtlil.h | 2 -- 2 files changed, 8 deletions(-) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 76744836d..670b61afd 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -1084,9 +1084,7 @@ RTLIL::Design::Design() refcount_modules_ = 0; push_full_selection(); -#ifdef YOSYS_ENABLE_PYTHON RTLIL::Design::get_all_designs()->insert(std::pair(hashidx_, this)); -#endif } RTLIL::Design::~Design() @@ -1095,18 +1093,14 @@ RTLIL::Design::~Design() delete pr.second; for (auto n : bindings_) delete n; -#ifdef YOSYS_ENABLE_PYTHON RTLIL::Design::get_all_designs()->erase(hashidx_); -#endif } -#ifdef YOSYS_ENABLE_PYTHON static std::map all_designs; std::map *RTLIL::Design::get_all_designs(void) { return &all_designs; } -#endif RTLIL::ObjRange RTLIL::Design::modules() { diff --git a/kernel/rtlil.h b/kernel/rtlil.h index e005ac65e..fbf9292b3 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -1883,9 +1883,7 @@ struct RTLIL::Design // returns all selected unboxed whole modules, warning the user if any // partially selected or boxed modules have been ignored std::vector selected_unboxed_whole_modules_warn() const { return selected_modules(SELECT_WHOLE_WARN, SB_UNBOXED_WARN); } -#ifdef YOSYS_ENABLE_PYTHON static std::map *get_all_designs(void); -#endif }; struct RTLIL::Module : public RTLIL::NamedObject From d28f97e9da2d168406a2758a4b7cfd3d65f94159 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Thu, 9 Oct 2025 23:40:47 +0000 Subject: [PATCH 06/19] Remove YOSYS_USE_STICKY_IDS --- kernel/rtlil.cc | 4 ---- kernel/rtlil.h | 23 ----------------------- 2 files changed, 27 deletions(-) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 670b61afd..c6358729a 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -41,10 +41,6 @@ std::unordered_map RTLIL::IdString::global_id_index_; std::vector RTLIL::IdString::global_refcount_storage_; std::vector RTLIL::IdString::global_free_idx_list_; #endif -#ifdef YOSYS_USE_STICKY_IDS -int RTLIL::IdString::last_created_idx_[8]; -int RTLIL::IdString::last_created_idx_ptr_; -#endif #define X(_id) const RTLIL::IdString RTLIL::IDInternal::_id(RTLIL::StaticId::_id); #include "kernel/constids.inc" diff --git a/kernel/rtlil.h b/kernel/rtlil.h index fbf9292b3..556f936e9 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -145,7 +145,6 @@ struct RTLIL::IdString #undef YOSYS_XTRACE_GET_PUT #undef YOSYS_SORT_ID_FREE_LIST - #undef YOSYS_USE_STICKY_IDS #undef YOSYS_NO_IDS_REFCNT // the global id string cache @@ -169,11 +168,6 @@ struct RTLIL::IdString static std::vector global_free_idx_list_; #endif -#ifdef YOSYS_USE_STICKY_IDS - static int last_created_idx_ptr_; - static int last_created_idx_[8]; -#endif - static inline void xtrace_db_dump() { #ifdef YOSYS_XTRACE_GET_PUT @@ -189,14 +183,6 @@ struct RTLIL::IdString static inline void checkpoint() { - #ifdef YOSYS_USE_STICKY_IDS - last_created_idx_ptr_ = 0; - for (int i = 0; i < 8; i++) { - if (last_created_idx_[i]) - put_reference(last_created_idx_[i]); - last_created_idx_[i] = 0; - } - #endif #ifdef YOSYS_SORT_ID_FREE_LIST std::sort(global_free_idx_list_.begin(), global_free_idx_list_.end(), std::greater()); #endif @@ -273,15 +259,6 @@ struct RTLIL::IdString log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx).buf, idx, global_refcount_storage_.at(idx)); #endif - #ifdef YOSYS_USE_STICKY_IDS - // Avoid Create->Delete->Create pattern - if (last_created_idx_[last_created_idx_ptr_]) - put_reference(last_created_idx_[last_created_idx_ptr_]); - last_created_idx_[last_created_idx_ptr_] = idx; - get_reference(last_created_idx_[last_created_idx_ptr_]); - last_created_idx_ptr_ = (last_created_idx_ptr_ + 1) & 7; - #endif - return idx; } From 5133b4bdea85a075842bb735b51712d16bac905e Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Thu, 9 Oct 2025 23:28:10 +0000 Subject: [PATCH 07/19] Create RTLIL::OwningIdString and use it in a few places --- kernel/rtlil.cc | 2 +- kernel/rtlil.h | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index c6358729a..6889343cd 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -42,7 +42,7 @@ std::vector RTLIL::IdString::global_refcount_storage_; std::vector RTLIL::IdString::global_free_idx_list_; #endif -#define X(_id) const RTLIL::IdString RTLIL::IDInternal::_id(RTLIL::StaticId::_id); +#define X(_id) const RTLIL::OwningIdString RTLIL::IDInternal::_id(RTLIL::StaticId::_id); #include "kernel/constids.inc" #undef X diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 556f936e9..df1559f8c 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -122,6 +122,7 @@ namespace RTLIL struct Binding; struct IdString; struct StaticIdString; + struct OwningIdString; typedef std::pair SigSig; @@ -460,6 +461,16 @@ 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) { } +}; + namespace hashlib { template <> struct hash_ops { @@ -494,7 +505,7 @@ inline bool RTLIL::IdString::operator!=(const RTLIL::StaticIdString &rhs) const namespace RTLIL { namespace IDInternal { -#define X(_id) extern const IdString _id; +#define X(_id) extern const OwningIdString _id; #include "kernel/constids.inc" #undef X } From 54bde15329c961c29b53585dbbcf0fab35120935 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 10 Oct 2025 01:10:33 +0000 Subject: [PATCH 08/19] 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 6889343cd..0e974b141 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 df1559f8c..9d3919162 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -127,12 +127,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; }; }; @@ -189,32 +189,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)); @@ -239,7 +219,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}); @@ -263,67 +242,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); @@ -463,12 +394,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 { @@ -571,7 +568,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> From b0e2d75dbe26b98b2bf8a0d9702c422aa23f21f6 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 13 Oct 2025 00:12:51 +0000 Subject: [PATCH 09/19] Make IdString refcounts a hashtable containing only the nonzero refcounts This saves space and doesn't cost very much since we hardly ever have nonzero refcounts any more. It also allows for IdStrings with negative indexes, which we're going to add. --- kernel/rtlil.cc | 10 ++++++---- kernel/rtlil.h | 44 +++++++++++++++++++++++++++----------------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 0e974b141..466ed0a74 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -38,7 +38,7 @@ RTLIL::IdString::destruct_guard_t RTLIL::IdString::destruct_guard; std::vector RTLIL::IdString::global_id_storage_; std::unordered_map RTLIL::IdString::global_id_index_; #ifndef YOSYS_NO_IDS_REFCNT -std::vector RTLIL::IdString::global_refcount_storage_; +std::unordered_map RTLIL::IdString::global_refcount_storage_; std::vector RTLIL::IdString::global_free_idx_list_; #endif @@ -61,7 +61,6 @@ void RTLIL::IdString::prepopulate() int size = static_cast(RTLIL::StaticId::STATIC_ID_END); global_id_storage_.reserve(size); global_id_index_.reserve(size); - global_refcount_storage_.resize(size, 1); RTLIL::IdString::global_id_index_.insert({"", 0}); RTLIL::IdString::global_id_storage_.push_back({const_cast(""), 0}); #define X(N) populate("\\" #N); @@ -180,17 +179,20 @@ struct IdStringCollector { void RTLIL::OwningIdString::collect_garbage() { #ifndef YOSYS_NO_IDS_REFCNT - int size = GetSize(global_refcount_storage_); + int size = GetSize(global_id_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) + if (collector.live[i]) continue; RTLIL::IdString::Storage &storage = global_id_storage_.at(i); if (storage.buf == nullptr) continue; + if (global_refcount_storage_.find(i) != global_refcount_storage_.end()) + continue; + if (yosys_xtrace) { log("#X# Removed IdString '%s' with index %d.\n", storage.buf, i); log_backtrace("-X- ", yosys_xtrace-1); diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 9d3919162..a081762c9 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -159,16 +159,18 @@ struct RTLIL::IdString static std::vector global_id_storage_; static std::unordered_map global_id_index_; #ifndef YOSYS_NO_IDS_REFCNT - // For prepopulated IdStrings, the refcount is meaningless since they - // are never freed even if the refcount is zero. For code efficiency - // we increment the refcount of prepopulated IdStrings like any other string, - // but we never decrement the refcount or check whether it's zero. - // So, make this unsigned because refcounts of preopulated IdStrings may overflow - // and overflow of signed integers is undefined behavior. - static std::vector global_refcount_storage_; + // All (index, refcount) pairs in this map have refcount > 0. + static std::unordered_map global_refcount_storage_; static std::vector global_free_idx_list_; #endif + static int refcount(int idx) { + auto it = global_refcount_storage_.find(idx); + if (it == global_refcount_storage_.end()) + return 0; + return it->second; + } + static inline void xtrace_db_dump() { #ifdef YOSYS_XTRACE_GET_PUT @@ -177,7 +179,7 @@ struct RTLIL::IdString if (global_id_storage_.at(idx).buf == nullptr) log("#X# DB-DUMP index %d: FREE\n", idx); else - log("#X# DB-DUMP index %d: '%s' (ref %u)\n", idx, global_id_storage_.at(idx).buf, global_refcount_storage_.at(idx)); + log("#X# DB-DUMP index %d: '%s' (ref %u)\n", idx, refcount(idx).buf, refcount); } #endif } @@ -197,7 +199,7 @@ struct RTLIL::IdString if (it != global_id_index_.end()) { #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)); + log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(it->second).buf, it->second, refcount(it->second)); #endif return it->second; } @@ -214,7 +216,6 @@ struct RTLIL::IdString log_assert(global_id_storage_.size() < 0x40000000); global_free_idx_list_.push_back(global_id_storage_.size()); global_id_storage_.push_back({nullptr, 0}); - global_refcount_storage_.push_back(0); } int idx = global_free_idx_list_.back(); @@ -236,7 +237,7 @@ struct RTLIL::IdString #ifdef YOSYS_XTRACE_GET_PUT if (yosys_xtrace) - log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx).buf, idx, global_refcount_storage_.at(idx)); + log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx).buf, idx, refcount(idx)); #endif return idx; @@ -443,11 +444,17 @@ private: static void get_reference(int idx) { #ifndef YOSYS_NO_IDS_REFCNT - global_refcount_storage_[idx]++; + if (idx < static_cast(StaticId::STATIC_ID_END)) + return; + auto it = global_refcount_storage_.find(idx); + if (it == global_refcount_storage_.end()) + global_refcount_storage_.insert(it, {idx, 1}); + else + ++it->second; #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)); + log("#X# GET-BY-INDEX '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx), idx, refcount(idx)); #endif } @@ -459,11 +466,14 @@ private: 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_)); - } + if (yosys_xtrace) + log("#X# PUT '%s' (index %d, refcount %u)\n", global_id_storage_.at(index_), index_, refcount(index_)); #endif - --global_refcount_storage_[index_]; + auto it = global_refcount_storage_.find(index_); + log_assert(it != global_refcount_storage_.end() && it->second >= 1); + if (--it->second == 0) { + global_refcount_storage_.erase(it); + } #endif } }; From b3f3f425771c1cc0c401528b067b9b7b33983ebb Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Tue, 14 Oct 2025 01:00:20 +0000 Subject: [PATCH 10/19] Remove StaticIdString and just use IdString now that we can make it constexpr --- kernel/rtlil.cc | 4 ---- kernel/rtlil.h | 45 +++++++++------------------------------------ 2 files changed, 9 insertions(+), 40 deletions(-) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 466ed0a74..6084dd03e 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -42,10 +42,6 @@ std::unordered_map RTLIL::IdString::global_refcount_storage_; std::vector RTLIL::IdString::global_free_idx_list_; #endif -#define X(_id) const RTLIL::OwningIdString RTLIL::IDInternal::_id(RTLIL::StaticId::_id); -#include "kernel/constids.inc" -#undef X - static void populate(std::string_view name) { if (name[1] == '$') { diff --git a/kernel/rtlil.h b/kernel/rtlil.h index a081762c9..2f7319d4a 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -121,20 +121,9 @@ namespace RTLIL struct Process; struct Binding; struct IdString; - struct StaticIdString; struct OwningIdString; typedef std::pair SigSig; - - struct StaticIdString { - 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 OwningIdString &id_string() const { return id_str; } - - const OwningIdString &id_str; - const StaticId id; - }; }; struct RTLIL::IdString @@ -247,13 +236,13 @@ struct RTLIL::IdString int index_; - inline IdString() : index_(0) { } + constexpr inline IdString() : index_(0) { } inline IdString(const char *str) : index_(insert(std::string_view(str))) { } - inline IdString(const IdString &str) : index_(str.index_) { } + constexpr 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_(insert(std::string_view(str))) { } inline IdString(std::string_view str) : index_(insert(str)) { } - inline IdString(StaticId id) : index_(static_cast(id)) {} + constexpr inline IdString(StaticId id) : index_(static_cast(id)) {} IdString &operator=(const IdString &rhs) = default; @@ -289,8 +278,6 @@ struct RTLIL::IdString inline bool operator==(const IdString &rhs) const { return index_ == rhs.index_; } inline bool operator!=(const IdString &rhs) const { return index_ != rhs.index_; } - inline bool operator==(const StaticIdString &rhs) const; - inline bool operator!=(const StaticIdString &rhs) const; // The methods below are just convenience functions for better compatibility with std::string. @@ -375,7 +362,6 @@ struct RTLIL::IdString } bool in(const IdString &rhs) const { return *this == rhs; } - bool in(const StaticIdString &rhs) const { return *this == rhs; } bool in(const char *rhs) const { return *this == rhs; } bool in(const std::string &rhs) const { return *this == rhs; } inline bool in(const pool &rhs) const; @@ -395,7 +381,6 @@ public: struct RTLIL::OwningIdString : public RTLIL::IdString { inline OwningIdString() { } - 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(); } @@ -503,21 +488,9 @@ inline bool RTLIL::IdString::in(const pool &rhs) const { return rhs.co [[deprecated]] inline bool RTLIL::IdString::in(const pool &&rhs) const { return rhs.count(*this) != 0; } -inline bool RTLIL::IdString::operator==(const RTLIL::StaticIdString &rhs) const { - return index_ == rhs.index(); -} -inline bool RTLIL::IdString::operator!=(const RTLIL::StaticIdString &rhs) const { - return index_ != rhs.index(); -} - namespace RTLIL { - namespace IDInternal { -#define X(_id) extern const OwningIdString _id; -#include "kernel/constids.inc" -#undef X - } namespace ID { -#define X(_id) constexpr StaticIdString _id(StaticId::_id, IDInternal::_id); +#define X(_id) constexpr IdString _id(StaticId::_id); #include "kernel/constids.inc" #undef X } @@ -525,7 +498,7 @@ namespace RTLIL { struct IdTableEntry { const std::string_view name; - const RTLIL::StaticIdString static_id; + const RTLIL::IdString static_id; }; constexpr IdTableEntry IdTable[] = { @@ -558,15 +531,15 @@ constexpr int lookup_well_known_id(std::string_view name) // // sed -i.orig -r 's/"\\\\([a-zA-Z0-9_]+)"/ID(\1)/g; s/"(\$[a-zA-Z0-9_]+)"/ID(\1)/g;' // -typedef const RTLIL::IdString &IDMacroHelperFunc(); +typedef RTLIL::IdString IDMacroHelperFunc(); template struct IDMacroHelper { - static constexpr RTLIL::StaticIdString eval(IDMacroHelperFunc) { + static constexpr RTLIL::IdString eval(IDMacroHelperFunc) { return IdTable[IdTableIndex].static_id; } }; template <> struct IDMacroHelper<-1> { - static constexpr const RTLIL::IdString &eval(IDMacroHelperFunc func) { + static constexpr RTLIL::IdString eval(IDMacroHelperFunc func) { return func(); } }; @@ -576,7 +549,7 @@ template <> struct IDMacroHelper<-1> { YOSYS_NAMESPACE_PREFIX IDMacroHelper< \ YOSYS_NAMESPACE_PREFIX lookup_well_known_id(#_id) \ >::eval([]() \ - -> const YOSYS_NAMESPACE_PREFIX RTLIL::IdString & { \ + -> YOSYS_NAMESPACE_PREFIX RTLIL::IdString { \ const char *p = "\\" #_id, *q = p[1] == '$' ? p+1 : p; \ static const YOSYS_NAMESPACE_PREFIX RTLIL::IdString id = \ YOSYS_NAMESPACE_PREFIX RTLIL::OwningIdString::immortal(q); \ From 9577a028c8dd25669c982a82c5ea69ec7737066b Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 13 Oct 2025 00:28:49 +0000 Subject: [PATCH 11/19] Make new_id/new_id_suffix taking string_view to avoid allocating strings --- kernel/yosys.cc | 12 ++++++------ kernel/yosys_common.h | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/kernel/yosys.cc b/kernel/yosys.cc index ca1db3aa6..85de1ea72 100644 --- a/kernel/yosys.cc +++ b/kernel/yosys.cc @@ -295,35 +295,35 @@ void yosys_shutdown() #endif } -RTLIL::IdString new_id(std::string file, int line, std::string func) +RTLIL::IdString new_id(std::string_view file, int line, std::string_view func) { #ifdef _WIN32 size_t pos = file.find_last_of("/\\"); #else size_t pos = file.find_last_of('/'); #endif - if (pos != std::string::npos) + if (pos != std::string_view::npos) file = file.substr(pos+1); pos = func.find_last_of(':'); - if (pos != std::string::npos) + if (pos != std::string_view::npos) func = func.substr(pos+1); return stringf("$auto$%s:%d:%s$%d", file, line, func, autoidx++); } -RTLIL::IdString new_id_suffix(std::string file, int line, std::string func, std::string suffix) +RTLIL::IdString new_id_suffix(std::string_view file, int line, std::string_view func, std::string_view suffix) { #ifdef _WIN32 size_t pos = file.find_last_of("/\\"); #else size_t pos = file.find_last_of('/'); #endif - if (pos != std::string::npos) + if (pos != std::string_view::npos) file = file.substr(pos+1); pos = func.find_last_of(':'); - if (pos != std::string::npos) + if (pos != std::string_view::npos) func = func.substr(pos+1); return stringf("$auto$%s:%d:%s$%s$%d", file, line, func, suffix, autoidx++); diff --git a/kernel/yosys_common.h b/kernel/yosys_common.h index 4794b5618..08b7fdc08 100644 --- a/kernel/yosys_common.h +++ b/kernel/yosys_common.h @@ -271,8 +271,8 @@ extern int autoidx; extern int yosys_xtrace; extern bool yosys_write_versions; -RTLIL::IdString new_id(std::string file, int line, std::string func); -RTLIL::IdString new_id_suffix(std::string file, int line, std::string func, std::string suffix); +RTLIL::IdString new_id(std::string_view file, int line, std::string_view func); +RTLIL::IdString new_id_suffix(std::string_view file, int line, std::string_view func, std::string_view suffix); #define NEW_ID \ YOSYS_NAMESPACE_PREFIX new_id(__FILE__, __LINE__, __FUNCTION__) From 889575736485854fa87fb247da83a3bc12e9dd41 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 13 Oct 2025 00:44:15 +0000 Subject: [PATCH 12/19] Ensure that `new_id(_suffix)()` cannot create collisions with existing `IdString`s. --- kernel/rtlil.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 2f7319d4a..fb0849902 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -23,6 +23,7 @@ #include "kernel/yosys_common.h" #include "kernel/yosys.h" +#include #include #include @@ -200,6 +201,16 @@ struct RTLIL::IdString if ((unsigned)ch <= (unsigned)' ') log_error("Found control character or space (0x%02x) in string '%s' which is not allowed in RTLIL identifiers\n", ch, std::string(p).c_str()); + if (p.substr(0, 6) == "$auto$") { + // Ensure new_id(_suffix) will not create collisions. + size_t autoidx_pos = p.find_last_of('$'); + int p_autoidx; + std::string_view v = p.substr(autoidx_pos + 1); + if (std::from_chars(v.begin(), v.end(), p_autoidx).ec == std::errc()) { + autoidx = std::max(autoidx, p_autoidx + 1); + } + } + #ifndef YOSYS_NO_IDS_REFCNT if (global_free_idx_list_.empty()) { log_assert(global_id_storage_.size() < 0x40000000); From e95ed7bbaba71645860745eaeb88fa77bbece53c Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 13 Oct 2025 02:56:32 +0000 Subject: [PATCH 13/19] Make NEW_ID create IDs whose string allocation is delayed --- kernel/io.cc | 2 +- kernel/rtlil.cc | 104 ++++++++++++++++++++++++++++--- kernel/rtlil.h | 110 ++++++++++++++++----------------- kernel/yosys.cc | 4 +- kernel/yosys_common.h | 7 ++- pyosys/generator.py | 2 + tests/unit/kernel/rtlilTest.cc | 6 ++ 7 files changed, 165 insertions(+), 70 deletions(-) diff --git a/kernel/io.cc b/kernel/io.cc index e713cff85..4f805e43b 100644 --- a/kernel/io.cc +++ b/kernel/io.cc @@ -606,7 +606,7 @@ void format_emit_idstring(std::string &result, std::string_view spec, int *dynam { if (spec == "%s") { // Format checking will have guaranteed num_dynamic_ints == 0. - result += arg.str_view(); + arg.append_to(&result); return; } format_emit_stringf(result, spec, dynamic_ints, num_dynamic_ints, arg.c_str()); diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 6084dd03e..91fbc7960 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -37,6 +38,8 @@ bool RTLIL::IdString::destruct_guard_ok = false; RTLIL::IdString::destruct_guard_t RTLIL::IdString::destruct_guard; std::vector RTLIL::IdString::global_id_storage_; std::unordered_map RTLIL::IdString::global_id_index_; +std::unordered_map RTLIL::IdString::global_autoidx_id_prefix_storage_; +std::unordered_map RTLIL::IdString::global_autoidx_id_storage_; #ifndef YOSYS_NO_IDS_REFCNT std::unordered_map RTLIL::IdString::global_refcount_storage_; std::vector RTLIL::IdString::global_free_idx_list_; @@ -64,6 +67,74 @@ void RTLIL::IdString::prepopulate() #undef X } +static std::optional parse_autoidx(std::string_view v) +{ + // autoidx values can never be <= 0, so there can never be a leading 0 digit. + if (v.empty() || v[0] == '0') + return std::nullopt; + for (char ch : v) { + if (ch < '0' || ch > '9') + return std::nullopt; + } + int p_autoidx; + if (std::from_chars(v.data(), v.data() + v.size(), p_autoidx).ec != std::errc()) + return std::nullopt; + return p_autoidx; +} + +int RTLIL::IdString::really_insert(std::string_view p, std::unordered_map::iterator &it) +{ + ensure_prepopulated(); + + log_assert(p[0] == '$' || p[0] == '\\'); + for (char ch : p) + if ((unsigned)ch <= (unsigned)' ') + log_error("Found control character or space (0x%02x) in string '%s' which is not allowed in RTLIL identifiers\n", ch, std::string(p).c_str()); + + if (p.substr(0, 6) == "$auto$") { + size_t autoidx_pos = p.find_last_of('$') + 1; + std::optional p_autoidx = parse_autoidx(p.substr(autoidx_pos)); + if (p_autoidx.has_value()) { + auto prefix_it = global_autoidx_id_prefix_storage_.find(-*p_autoidx); + if (prefix_it != global_autoidx_id_prefix_storage_.end() && p.substr(0, autoidx_pos) == *prefix_it->second) + return -*p_autoidx; + // Ensure NEW_ID/NEW_ID_SUFFIX will not create collisions with the ID + // we're about to create. + autoidx = std::max(autoidx, *p_autoidx + 1); + } + } + +#ifndef YOSYS_NO_IDS_REFCNT + if (global_free_idx_list_.empty()) { + log_assert(global_id_storage_.size() < 0x40000000); + global_free_idx_list_.push_back(global_id_storage_.size()); + global_id_storage_.push_back({nullptr, 0}); + } + + int idx = global_free_idx_list_.back(); + global_free_idx_list_.pop_back(); +#else + int idx = global_id_storage_.size(); + global_id_index_[global_id_storage_.back()] = idx; +#endif + char* buf = static_cast(malloc(p.size() + 1)); + memcpy(buf, p.data(), p.size()); + buf[p.size()] = 0; + global_id_storage_.at(idx) = {buf, GetSize(p)}; + global_id_index_.insert(it, {std::string_view(buf, p.size()), idx}); + + if (yosys_xtrace) { + log("#X# New IdString '%s' with index %d.\n", global_id_storage_.at(idx).buf, idx); + log_backtrace("-X- ", yosys_xtrace-1); + } + +#ifdef YOSYS_XTRACE_GET_PUT + if (yosys_xtrace) + log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx), idx, refcount(idx)); +#endif + return idx; +} + static constexpr bool check_well_known_id_order() { int size = sizeof(IdTable) / sizeof(IdTable[0]); @@ -78,9 +149,9 @@ static constexpr bool check_well_known_id_order() static_assert(check_well_known_id_order()); struct IdStringCollector { - IdStringCollector(int size) : live(size, false) {} - - void trace(IdString id) { live[id.index_] = true; } + void trace(IdString id) { + live.insert(id.index_); + } template void trace(const T* v) { trace(*v); } @@ -169,23 +240,23 @@ struct IdStringCollector { trace(action.memid); } - std::vector live; + std::unordered_set live; }; void RTLIL::OwningIdString::collect_garbage() { #ifndef YOSYS_NO_IDS_REFCNT - int size = GetSize(global_id_storage_); - IdStringCollector collector(size); + IdStringCollector collector; for (auto &[idx, design] : *RTLIL::Design::get_all_designs()) { collector.trace(*design); } + int size = GetSize(global_id_storage_); for (int i = static_cast(StaticId::STATIC_ID_END); i < size; ++i) { - if (collector.live[i]) - continue; RTLIL::IdString::Storage &storage = global_id_storage_.at(i); if (storage.buf == nullptr) continue; + if (collector.live.find(i) != collector.live.end()) + continue; if (global_refcount_storage_.find(i) != global_refcount_storage_.end()) continue; @@ -199,6 +270,23 @@ void RTLIL::OwningIdString::collect_garbage() storage = {nullptr, 0}; global_free_idx_list_.push_back(i); } + + for (auto it = global_autoidx_id_prefix_storage_.begin(); it != global_autoidx_id_prefix_storage_.end();) { + if (collector.live.find(it->first) != collector.live.end()) { + ++it; + continue; + } + if (global_refcount_storage_.find(it->first) != global_refcount_storage_.end()) { + ++it; + continue; + } + auto str_it = global_autoidx_id_storage_.find(it->first); + if (str_it != global_autoidx_id_storage_.end()) { + delete[] str_it->second; + global_autoidx_id_storage_.erase(str_it); + } + it = global_autoidx_id_prefix_storage_.erase(it); + } #endif } diff --git a/kernel/rtlil.h b/kernel/rtlil.h index fb0849902..7a6dad57f 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -23,7 +23,6 @@ #include "kernel/yosys_common.h" #include "kernel/yosys.h" -#include #include #include @@ -146,8 +145,16 @@ struct RTLIL::IdString ~destruct_guard_t() { destruct_guard_ok = false; } } destruct_guard; + // String storage for non-autoidx IDs static std::vector global_id_storage_; + // Lookup table for non-autoidx IDs static std::unordered_map global_id_index_; + // Shared prefix string storage for autoidx IDs, which have negative + // indices. Append the negated (i.e. positive) ID to this string to get + // the real string. The prefix strings must live forever. + static std::unordered_map global_autoidx_id_prefix_storage_; + // Explicit string storage for autoidx IDs + static std::unordered_map global_autoidx_id_storage_; #ifndef YOSYS_NO_IDS_REFCNT // All (index, refcount) pairs in this map have refcount > 0. static std::unordered_map global_refcount_storage_; @@ -193,54 +200,15 @@ struct RTLIL::IdString #endif return it->second; } + return really_insert(p, it); + } - ensure_prepopulated(); - - log_assert(p[0] == '$' || p[0] == '\\'); - for (char ch : p) - if ((unsigned)ch <= (unsigned)' ') - log_error("Found control character or space (0x%02x) in string '%s' which is not allowed in RTLIL identifiers\n", ch, std::string(p).c_str()); - - if (p.substr(0, 6) == "$auto$") { - // Ensure new_id(_suffix) will not create collisions. - size_t autoidx_pos = p.find_last_of('$'); - int p_autoidx; - std::string_view v = p.substr(autoidx_pos + 1); - if (std::from_chars(v.begin(), v.end(), p_autoidx).ec == std::errc()) { - autoidx = std::max(autoidx, p_autoidx + 1); - } - } - - #ifndef YOSYS_NO_IDS_REFCNT - if (global_free_idx_list_.empty()) { - log_assert(global_id_storage_.size() < 0x40000000); - global_free_idx_list_.push_back(global_id_storage_.size()); - global_id_storage_.push_back({nullptr, 0}); - } - - int idx = global_free_idx_list_.back(); - global_free_idx_list_.pop_back(); - #else - int idx = global_id_storage_.size(); - global_id_storage_.push_back({nullptr, 0}); - #endif - char* buf = static_cast(malloc(p.size() + 1)); - memcpy(buf, p.data(), p.size()); - buf[p.size()] = 0; - global_id_storage_.at(idx) = {buf, GetSize(p)}; - global_id_index_.insert(it, {std::string_view(buf, p.size()), idx}); - - if (yosys_xtrace) { - log("#X# New IdString '%s' with index %d.\n", global_id_storage_.at(idx).buf, idx); - log_backtrace("-X- ", yosys_xtrace-1); - } - - #ifdef YOSYS_XTRACE_GET_PUT - if (yosys_xtrace) - log("#X# GET-BY-NAME '%s' (index %d, refcount %u)\n", global_id_storage_.at(idx).buf, idx, refcount(idx)); - #endif - - return idx; + // Inserts an ID with string `prefix + autoidx', incrementing autoidx. + // `prefix` must start with '$auto$', end with '$', and live forever. + static IdString new_autoidx_with_prefix(const std::string *prefix) { + int index = -(autoidx++); + global_autoidx_id_prefix_storage_.insert({index, prefix}); + return from_index(index); } // the actual IdString object is just is a single int @@ -270,17 +238,35 @@ struct RTLIL::IdString constexpr inline const IdString &id_string() const { return *this; } inline const char *c_str() const { - return global_id_storage_.at(index_).buf; + if (index_ >= 0) + return global_id_storage_.at(index_).buf; + auto it = global_autoidx_id_storage_.find(index_); + if (it != global_autoidx_id_storage_.end()) + return it->second; + + const std::string &prefix = *global_autoidx_id_prefix_storage_.at(index_); + std::string suffix = std::to_string(-index_); + char *c = new char[prefix.size() + suffix.size() + 1]; + memcpy(c, prefix.data(), prefix.size()); + memcpy(c + prefix.size(), suffix.c_str(), suffix.size() + 1); + global_autoidx_id_storage_.insert(it, {index_, c}); + return c; } inline std::string str() const { - const Storage &storage = global_id_storage_.at(index_); - return std::string(storage.buf, storage.size); + std::string result; + append_to(&result); + return result; } - inline std::string_view str_view() const { - const Storage &storage = global_id_storage_.at(index_); - return std::string_view(storage.buf, storage.size); + inline void append_to(std::string *out) const { + if (index_ >= 0) { + const Storage &storage = global_id_storage_.at(index_); + *out += std::string_view(storage.buf, storage.size); + return; + } + *out += *global_autoidx_id_prefix_storage_.at(index_); + *out += std::to_string(-index_); } inline bool operator<(const IdString &rhs) const { @@ -336,7 +322,9 @@ struct RTLIL::IdString } size_t size() const { - return global_id_storage_.at(index_).size; + if (index_ >= 0) + return global_id_storage_.at(index_).size; + return strlen(c_str()); } bool empty() const { @@ -382,6 +370,14 @@ struct RTLIL::IdString private: static void prepopulate(); + static int really_insert(std::string_view p, std::unordered_map::iterator &it); + +protected: + static IdString from_index(int index) { + IdString result; + result.index_ = index; + return result; + } public: static void ensure_prepopulated() { @@ -450,7 +446,7 @@ private: #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, refcount(idx)); + log("#X# GET-BY-INDEX '%s' (index %d, refcount %u)\n", from_index(idx), idx, refcount(idx)); #endif } @@ -463,7 +459,7 @@ private: return; #ifdef YOSYS_XTRACE_GET_PUT if (yosys_xtrace) - log("#X# PUT '%s' (index %d, refcount %u)\n", global_id_storage_.at(index_), index_, refcount(index_)); + log("#X# PUT '%s' (index %d, refcount %u)\n", from_index(index_), index_, refcount(index_)); #endif auto it = global_refcount_storage_.find(index_); log_assert(it != global_refcount_storage_.end() && it->second >= 1); diff --git a/kernel/yosys.cc b/kernel/yosys.cc index 85de1ea72..bf59302f8 100644 --- a/kernel/yosys.cc +++ b/kernel/yosys.cc @@ -295,7 +295,7 @@ void yosys_shutdown() #endif } -RTLIL::IdString new_id(std::string_view file, int line, std::string_view func) +const std::string *create_id_prefix(std::string_view file, int line, std::string_view func) { #ifdef _WIN32 size_t pos = file.find_last_of("/\\"); @@ -309,7 +309,7 @@ RTLIL::IdString new_id(std::string_view file, int line, std::string_view func) if (pos != std::string_view::npos) func = func.substr(pos+1); - return stringf("$auto$%s:%d:%s$%d", file, line, func, autoidx++); + return new std::string(stringf("$auto$%s:%d:%s$", file, line, func)); } RTLIL::IdString new_id_suffix(std::string_view file, int line, std::string_view func, std::string_view suffix) diff --git a/kernel/yosys_common.h b/kernel/yosys_common.h index 08b7fdc08..bc8500654 100644 --- a/kernel/yosys_common.h +++ b/kernel/yosys_common.h @@ -271,11 +271,14 @@ extern int autoidx; extern int yosys_xtrace; extern bool yosys_write_versions; -RTLIL::IdString new_id(std::string_view file, int line, std::string_view func); +const std::string *create_id_prefix(std::string_view file, int line, std::string_view func); RTLIL::IdString new_id_suffix(std::string_view file, int line, std::string_view func, std::string_view suffix); #define NEW_ID \ - YOSYS_NAMESPACE_PREFIX new_id(__FILE__, __LINE__, __FUNCTION__) + YOSYS_NAMESPACE_PREFIX RTLIL::IdString::new_autoidx_with_prefix([](std::string_view func) -> const std::string * { \ + static const std::string *prefix = create_id_prefix(__FILE__, __LINE__, func); \ + return prefix; \ + }(__FUNCTION__)) #define NEW_ID_SUFFIX(suffix) \ YOSYS_NAMESPACE_PREFIX new_id_suffix(__FILE__, __LINE__, __FUNCTION__, suffix) diff --git a/pyosys/generator.py b/pyosys/generator.py index 0883a7ba4..f89819504 100644 --- a/pyosys/generator.py +++ b/pyosys/generator.py @@ -164,6 +164,8 @@ pyosys_headers = [ { "global_id_storage_", "global_id_index_", + "global_negative_id_storage_", + "global_negative_id_prefix_storage_", "global_refcount_storage_", "global_free_idx_list_", "last_created_idx_ptr_", diff --git a/tests/unit/kernel/rtlilTest.cc b/tests/unit/kernel/rtlilTest.cc index 8bba9ac28..a5923e02c 100644 --- a/tests/unit/kernel/rtlilTest.cc +++ b/tests/unit/kernel/rtlilTest.cc @@ -367,6 +367,12 @@ namespace RTLIL { EXPECT_EQ(own.str(), "\\figblortle"); } + TEST_F(KernelRtlilTest, LookupAutoidxId) { + IdString id = NEW_ID; + IdString id2 = IdString(id.str()); + EXPECT_EQ(id, id2); + } + class WireRtlVsHdlIndexConversionTest : public KernelRtlilTest, public testing::WithParamInterface> From df8444c5e73de481f7129dbfc7220d1d525cbb9f Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 13 Oct 2025 20:51:35 +0000 Subject: [PATCH 14/19] Optimize IdString operations to avoid calling c_str() --- kernel/rtlil.h | 203 +++++++++++++++++++++++++++++---- tests/unit/kernel/rtlilTest.cc | 18 +++ 2 files changed, 200 insertions(+), 21 deletions(-) diff --git a/kernel/rtlil.h b/kernel/rtlil.h index 7a6dad57f..de80a9c60 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -131,6 +131,8 @@ struct RTLIL::IdString struct Storage { char *buf; int size; + + std::string_view str_view() const { return {buf, static_cast(size)}; } }; #undef YOSYS_XTRACE_GET_PUT @@ -261,14 +263,135 @@ struct RTLIL::IdString inline void append_to(std::string *out) const { if (index_ >= 0) { - const Storage &storage = global_id_storage_.at(index_); - *out += std::string_view(storage.buf, storage.size); + *out += global_id_storage_.at(index_).str_view(); return; } *out += *global_autoidx_id_prefix_storage_.at(index_); *out += std::to_string(-index_); } + class Substrings { + std::string_view first_; + int suffix_number; + char buf[10]; + public: + Substrings(const Storage &storage) : first_(storage.str_view()), suffix_number(-1) {} + // suffix_number must be non-negative + Substrings(const std::string *prefix, int suffix_number) + : first_(*prefix), suffix_number(suffix_number) {} + std::string_view first() { return first_; } + std::optional next() { + if (suffix_number < 0) + return std::nullopt; + int i = sizeof(buf); + do { + --i; + buf[i] = (suffix_number % 10) + '0'; + suffix_number /= 10; + } while (suffix_number > 0); + suffix_number = -1; + return std::string_view(buf + i, sizeof(buf) - i); + } + }; + + class const_iterator { + const std::string *prefix; + std::string suffix; + const char *c_str; + int c_str_len; + // When this is INT_MAX it's the generic "end" value. + int index; + + public: + using iterator_category = std::forward_iterator_tag; + using value_type = char; + using difference_type = std::ptrdiff_t; + using pointer = const char*; + using reference = const char&; + + const_iterator(const Storage &storage) : prefix(nullptr), c_str(storage.buf), c_str_len(storage.size), index(0) {} + const_iterator(const std::string *prefix, int number) : + prefix(prefix), suffix(std::to_string(number)), c_str(nullptr), c_str_len(0), index(0) {} + // Construct end-marker + const_iterator() : prefix(nullptr), c_str(nullptr), c_str_len(0), index(INT_MAX) {} + + int size() const { + if (c_str != nullptr) + return c_str_len; + return GetSize(*prefix) + GetSize(suffix); + } + + char operator*() const { + if (c_str != nullptr) + return c_str[index]; + int prefix_size = GetSize(*prefix); + if (index < prefix_size) + return prefix->at(index); + return suffix[index - prefix_size]; + } + + const_iterator& operator++() { ++index; return *this; } + const_iterator operator++(int) { const_iterator result(*this); ++index; return result; } + const_iterator& operator+=(int i) { index += i; return *this; } + + const_iterator operator+(int add) { + const_iterator result = *this; + result += add; + return result; + } + + bool operator==(const const_iterator& other) const { + return index == other.index || (other.index == INT_MAX && index == size()) + || (index == INT_MAX && other.index == other.size()); + } + bool operator!=(const const_iterator& other) const { + return !(*this == other); + } + }; + const_iterator begin() const { + if (index_ >= 0) { + return const_iterator(global_id_storage_.at(index_)); + } + return const_iterator(global_autoidx_id_prefix_storage_.at(index_), -index_); + } + const_iterator end() const { + return const_iterator(); + } + + Substrings substrings() const { + if (index_ >= 0) { + return Substrings(global_id_storage_.at(index_)); + } + return Substrings(global_autoidx_id_prefix_storage_.at(index_), -index_); + } + + inline bool lt_by_name(const IdString &rhs) const { + Substrings lhs_it = substrings(); + Substrings rhs_it = rhs.substrings(); + std::string_view lhs_substr = lhs_it.first(); + std::string_view rhs_substr = rhs_it.first(); + while (true) { + int min = std::min(GetSize(lhs_substr), GetSize(rhs_substr)); + int diff = memcmp(lhs_substr.data(), rhs_substr.data(), min); + if (diff != 0) + return diff < 0; + lhs_substr = lhs_substr.substr(min); + rhs_substr = rhs_substr.substr(min); + if (rhs_substr.empty()) { + if (std::optional s = rhs_it.next()) + rhs_substr = *s; + else + return false; + } + if (lhs_substr.empty()) { + if (std::optional s = lhs_it.next()) + lhs_substr = *s; + else + return true; + } + } + } + inline bool operator<(const IdString &rhs) const { return index_ < rhs.index_; } @@ -285,30 +408,68 @@ struct RTLIL::IdString bool operator!=(const char *rhs) const { return strcmp(c_str(), rhs) != 0; } char operator[](size_t i) const { - const char *p = c_str(); + if (index_ >= 0) { + const Storage &storage = global_id_storage_.at(index_); #ifndef NDEBUG - for (; i != 0; i--, p++) - log_assert(*p != 0); - return *p; -#else - return *(p + i); + log_assert(static_cast(i) < storage.size); #endif + return *(storage.buf + i); + } + const std::string &id_start = *global_autoidx_id_prefix_storage_.at(index_); + if (i < id_start.size()) + return id_start[i]; + i -= id_start.size(); + std::string suffix = std::to_string(-index_); +#ifndef NDEBUG + // Allow indexing to access the trailing null. + log_assert(i <= suffix.size()); +#endif + return suffix[i]; } std::string substr(size_t pos = 0, size_t len = std::string::npos) const { - if (len == std::string::npos || len + pos >= size()) - return std::string(c_str() + pos); - else - return std::string(c_str() + pos, len); + std::string result; + const_iterator it = begin() + pos; + const_iterator end_it = end(); + if (len != std::string::npos && len < it.size() - pos) { + end_it = it + len; + } + std::copy(it, end_it, std::back_inserter(result)); + return result; } int compare(size_t pos, size_t len, const char* s) const { - return strncmp(c_str()+pos, s, len); + const_iterator it = begin() + pos; + const_iterator end_it = end(); + while (len > 0 && *s != 0 && it != end_it) { + int diff = *it - *s; + if (diff != 0) + return diff; + ++it; + ++s; + --len; + } + return 0; } bool begins_with(std::string_view prefix) const { - if (size() < prefix.size()) return false; - return compare(0, prefix.size(), prefix.data()) == 0; + Substrings it = substrings(); + std::string_view substr = it.first(); + while (true) { + int min = std::min(GetSize(substr), GetSize(prefix)); + if (memcmp(substr.data(), prefix.data(), min) != 0) + return false; + prefix = prefix.substr(min); + if (prefix.empty()) + return true; + substr = substr.substr(min); + if (substr.empty()) { + if (std::optional s = it.next()) + substr = *s; + else + return false; + } + } } bool ends_with(std::string_view suffix) const { @@ -318,13 +479,13 @@ struct RTLIL::IdString } bool contains(std::string_view s) const { - return std::string_view(c_str()).find(s) != std::string::npos; + if (index_ >= 0) + return global_id_storage_.at(index_).str_view().find(s) != std::string::npos; + return str().find(s) != std::string::npos; } size_t size() const { - if (index_ >= 0) - return global_id_storage_.at(index_).size; - return strlen(c_str()); + return begin().size(); } bool empty() const { @@ -603,13 +764,13 @@ namespace RTLIL { template struct sort_by_name_str { bool operator()(T *a, T *b) const { - return strcmp(a->name.c_str(), b->name.c_str()) < 0; + return a->name.lt_by_name(b->name); } }; struct sort_by_id_str { bool operator()(const RTLIL::IdString &a, const RTLIL::IdString &b) const { - return strcmp(a.c_str(), b.c_str()) < 0; + return a.lt_by_name(b); } }; diff --git a/tests/unit/kernel/rtlilTest.cc b/tests/unit/kernel/rtlilTest.cc index a5923e02c..32c40616e 100644 --- a/tests/unit/kernel/rtlilTest.cc +++ b/tests/unit/kernel/rtlilTest.cc @@ -373,6 +373,24 @@ namespace RTLIL { EXPECT_EQ(id, id2); } + TEST_F(KernelRtlilTest, NewIdBeginsWith) { + IdString id = NEW_ID; + EXPECT_TRUE(id.begins_with("$auto")); + EXPECT_FALSE(id.begins_with("xyz")); + EXPECT_TRUE(id.begins_with("$auto$")); + EXPECT_FALSE(id.begins_with("abcdefghijklmn")); + EXPECT_TRUE(id.begins_with("$auto$rtlilTest")); + EXPECT_FALSE(id.begins_with("$auto$rtlilX")); + } + + TEST_F(KernelRtlilTest, NewIdIndexing) { + IdString id = NEW_ID; + std::string str = id.str(); + for (int i = 0; i < GetSize(str) + 1; ++i) { + EXPECT_EQ(id[i], str.c_str()[i]); + } + } + class WireRtlVsHdlIndexConversionTest : public KernelRtlilTest, public testing::WithParamInterface> From 325b27f43ad7b694d611a48a6a216d136db1d0be Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 13 Oct 2025 20:52:35 +0000 Subject: [PATCH 15/19] Avoid calling IdString::c_str() in opt_clean --- passes/opt/opt_clean.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passes/opt/opt_clean.cc b/passes/opt/opt_clean.cc index b3e3cd33a..3892c7581 100644 --- a/passes/opt/opt_clean.cc +++ b/passes/opt/opt_clean.cc @@ -283,7 +283,7 @@ bool compare_signals(RTLIL::SigBit &s1, RTLIL::SigBit &s2, SigPool ®s, SigPoo if (attrs1 != attrs2) return attrs2 > attrs1; - return strcmp(w2->name.c_str(), w1->name.c_str()) < 0; + return w2->name.lt_by_name(w1->name); } bool check_public_name(RTLIL::IdString id) From c4c389fdd71f9dfc653d64789437084fd091f21e Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 13 Oct 2025 22:23:51 +0000 Subject: [PATCH 16/19] Fix verilog backend to avoid IdString::c_str() --- backends/verilog/verilog_backend.cc | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/backends/verilog/verilog_backend.cc b/backends/verilog/verilog_backend.cc index faeb2cd0b..67ff1f808 100644 --- a/backends/verilog/verilog_backend.cc +++ b/backends/verilog/verilog_backend.cc @@ -108,22 +108,30 @@ IdString initial_id; void reset_auto_counter_id(RTLIL::IdString id, bool may_rename) { - const char *str = id.c_str(); - - if (*str == '$' && may_rename && !norename) - auto_name_map[id] = auto_name_counter++; - - if (str[0] != '\\' || str[1] != '_' || str[2] == 0) + auto it = id.begin(); + auto it_end = id.end(); + if (it == it_end) return; - for (int i = 2; str[i] != 0; i++) { - if (str[i] == '_' && str[i+1] == 0) + if (*it == '$' && may_rename && !norename) + auto_name_map[id] = auto_name_counter++; + + if (*it != '\\' || *it != '_' || (it + 1) == it_end) + return; + + it += 2; + auto start = it; + while (it != it_end) { + char ch = *it; + if (ch == '_' && (it + 1) == it_end) continue; - if (str[i] < '0' || str[i] > '9') + if (ch < '0' || ch > '9') return; } - int num = atoi(str+2); + std::string s; + std::copy(start, it_end, std::back_inserter(s)); + int num = atoi(s.c_str()); if (num >= auto_name_offset) auto_name_offset = num + 1; } From 8c2984dc5f29ac22736febbda462e3f164dcb641 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 13 Oct 2025 22:43:31 +0000 Subject: [PATCH 17/19] Fix AbcModuleState::remap_name() to avoid calling IdString::c_str() --- passes/techmap/abc.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passes/techmap/abc.cc b/passes/techmap/abc.cc index e2894aee3..3a814d0b7 100644 --- a/passes/techmap/abc.cc +++ b/passes/techmap/abc.cc @@ -611,7 +611,7 @@ std::string AbcModuleState::remap_name(RTLIL::IdString abc_name, RTLIL::Wire **o } } } - return stringf("$abc$%d$%s", map_autoidx, abc_name.c_str()+1); + return stringf("$abc$%d$%s", map_autoidx, abc_name.substr(1)); } void AbcModuleState::dump_loop_graph(FILE *f, int &nr, dict> &edges, pool &workpool, std::vector &in_counts) From 578d6588713b4a82a493dfdac70fed26984dcaed Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 17 Oct 2025 12:15:53 +1300 Subject: [PATCH 18/19] Add timing stats for IdString garbage collection --- kernel/driver.cc | 2 ++ kernel/register.cc | 5 +++++ kernel/register.h | 2 ++ kernel/rtlil.cc | 8 ++++++++ kernel/rtlil.h | 5 +++++ 5 files changed, 22 insertions(+) diff --git a/kernel/driver.cc b/kernel/driver.cc index 795fd9fc5..eae9c5276 100644 --- a/kernel/driver.cc +++ b/kernel/driver.cc @@ -709,6 +709,8 @@ int main(int argc, char **argv) total_ns += it.second->runtime_ns + 1; timedat.insert(make_tuple(it.second->runtime_ns + 1, it.second->call_counter, it.first)); } + timedat.insert(make_tuple(RTLIL::OwningIdString::garbage_collection_ns() + 1, + RTLIL::OwningIdString::garbage_collection_count(), "id_gc")); if (timing_details) { diff --git a/kernel/register.cc b/kernel/register.cc index 1f8e4a9a5..3f5aa49ca 100644 --- a/kernel/register.cc +++ b/kernel/register.cc @@ -129,6 +129,11 @@ void Pass::post_execute(Pass::pre_post_exec_state_t state) int64_t time_ns = PerformanceTimer::query() - state.begin_ns; runtime_ns += time_ns; current_pass = state.parent_pass; + subtract_from_current_runtime_ns(time_ns); +} + +void Pass::subtract_from_current_runtime_ns(int64_t time_ns) +{ if (current_pass) current_pass->runtime_ns -= time_ns; } diff --git a/kernel/register.h b/kernel/register.h index b9c709dc1..78f9b430d 100644 --- a/kernel/register.h +++ b/kernel/register.h @@ -95,6 +95,8 @@ struct Pass bool experimental_flag = false; bool internal_flag = false; + static void subtract_from_current_runtime_ns(int64_t time_ns); + void experimental() { experimental_flag = true; } diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 91fbc7960..bc89fc6a4 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -243,8 +243,12 @@ struct IdStringCollector { std::unordered_set live; }; +int64_t RTLIL::OwningIdString::gc_ns; +int RTLIL::OwningIdString::gc_count; + void RTLIL::OwningIdString::collect_garbage() { + int64_t start = PerformanceTimer::query(); #ifndef YOSYS_NO_IDS_REFCNT IdStringCollector collector; for (auto &[idx, design] : *RTLIL::Design::get_all_designs()) { @@ -288,6 +292,10 @@ void RTLIL::OwningIdString::collect_garbage() it = global_autoidx_id_prefix_storage_.erase(it); } #endif + int64_t time_ns = PerformanceTimer::query() - start; + Pass::subtract_from_current_runtime_ns(time_ns); + gc_ns += time_ns; + ++gc_count; } dict RTLIL::constpad; diff --git a/kernel/rtlil.h b/kernel/rtlil.h index de80a9c60..4ae284b25 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -579,6 +579,8 @@ struct RTLIL::OwningIdString : public RTLIL::IdString { // Collect all non-owning references. static void collect_garbage(); + static int64_t garbage_collection_ns() { return gc_ns; } + static int garbage_collection_count() { return gc_count; } // 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 @@ -590,6 +592,9 @@ struct RTLIL::OwningIdString : public RTLIL::IdString { return result; } private: + static int64_t gc_ns; + static int gc_count; + void get_reference() { get_reference(index_); From ae281720cf25fa7b0d74a9eaa7c25fb7b2672614 Mon Sep 17 00:00:00 2001 From: "Emil J. Tywoniak" Date: Tue, 21 Oct 2025 00:00:59 +0200 Subject: [PATCH 19/19] tests: remove unstable FPGA synthesis result checks --- tests/arch/quicklogic/pp3/fsm.ys | 2 -- tests/arch/xilinx/dsp_cascade.ys | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/arch/quicklogic/pp3/fsm.ys b/tests/arch/quicklogic/pp3/fsm.ys index 9679628e9..3276e45c6 100644 --- a/tests/arch/quicklogic/pp3/fsm.ys +++ b/tests/arch/quicklogic/pp3/fsm.ys @@ -11,8 +11,6 @@ sat -verify -prove-asserts -show-public -set-at 1 in_reset 1 -seq 20 -prove-skip design -load postopt # load the post-opt design (otherwise equiv_opt loads the pre-opt design) cd fsm # Constrain all select calls below inside the top module -select -assert-count 2 t:LUT2 -select -assert-count 4 t:LUT3 select -assert-count 4 t:dffepc select -assert-count 1 t:logic_0 select -assert-count 1 t:logic_1 diff --git a/tests/arch/xilinx/dsp_cascade.ys b/tests/arch/xilinx/dsp_cascade.ys index ca6b619b9..0a68377f6 100644 --- a/tests/arch/xilinx/dsp_cascade.ys +++ b/tests/arch/xilinx/dsp_cascade.ys @@ -69,7 +69,8 @@ equiv_opt -assert -map +/xilinx/cells_sim.v synth_xilinx -noiopad design -load postopt cd cascade select -assert-count 2 t:DSP48E1 -select -assert-none t:DSP48E1 t:BUFG %% t:* %D +# TODO Disabled check, FDREs emitted due to order sensitivity +# select -assert-none t:DSP48E1 t:BUFG %% t:* %D # Very crude method of checking that DSP48E1.PCOUT -> DSP48E1.PCIN # (see above for explanation) select -assert-count 1 t:DSP48E1 %co:+[PCOUT] t:DSP48E1 %d %co:+[PCIN] w:* %d t:DSP48E1 %i