From f7ac724ea93dc0d0b3736a52c66c852ee548663c 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 394c6f25d..6992b45cf 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -404,16 +404,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 ae219cb8f9b9d3a023599ce4a6c3632276b26016 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 6992b45cf..b65b9b6eb 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -415,8 +415,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 d70924ace2ff4f5bee5f8a32991cc7d33b3853ef 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 9e9eb9fb0..028ac388d 100644 --- a/kernel/io.cc +++ b/kernel/io.cc @@ -587,7 +587,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 aa4743a87..4a7bf79b2 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 b65b9b6eb..d570e39a3 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -137,6 +137,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 @@ -150,7 +155,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 @@ -173,10 +178,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 } @@ -203,7 +208,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; } @@ -224,7 +229,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; } @@ -243,32 +248,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 @@ -293,7 +297,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 @@ -307,14 +311,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 @@ -358,11 +362,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 { @@ -394,7 +404,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); @@ -420,7 +430,7 @@ struct RTLIL::IdString } size_t size() const { - return strlen(c_str()); + return global_id_storage_.at(index_).size; } bool empty() const { From 3a4fa325cc17ab0ecb1f0d818e7a11804bbf4c61 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 4a7bf79b2..c01cde61a 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 d570e39a3..47ab6f06c 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -236,9 +236,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 8b8939e21943d094c36290e393a1bc1009a75214 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 c01cde61a..14a0f99a1 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 47ab6f06c..9c9ea6525 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -1700,9 +1700,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 2ca7b2f7d7bf6041bb33e8c62b485dbb83058f86 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 14a0f99a1..0bc5d66c9 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 9c9ea6525..91aa0237e 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -144,7 +144,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 @@ -168,11 +167,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 @@ -188,14 +182,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 @@ -272,15 +258,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 b3f79ed8e77f0fb13a3a62eed76b95df68a037f6 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 0bc5d66c9..d1cf7f23e 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 91aa0237e..f161ef445 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -121,6 +121,7 @@ namespace RTLIL struct Binding; struct IdString; struct StaticIdString; + struct OwningIdString; typedef std::pair SigSig; @@ -459,6 +460,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 { @@ -493,7 +504,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 bc7895505e1908d25916212cf55b5fdc131d5526 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 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> From 3c2caffffeb626d37286a01e5f13ae624ef7952c 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 c78bd00bb..84338297f 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 92a0dffb5..08eeda167 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -158,16 +158,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 @@ -176,7 +178,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 } @@ -196,7 +198,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; } @@ -213,7 +215,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(); @@ -235,7 +236,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; @@ -442,11 +443,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 } @@ -458,11 +465,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 5cc3f27a5f0e20d842e62bde1dbf0bd7fae9dd04 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 84338297f..0c5f113e7 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 08eeda167..a49b58cda 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -120,20 +120,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 @@ -246,13 +235,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; @@ -288,8 +277,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. @@ -374,7 +361,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; @@ -394,7 +380,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(); } @@ -502,21 +487,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 } @@ -524,7 +497,7 @@ namespace RTLIL { struct IdTableEntry { const std::string_view name; - const RTLIL::StaticIdString static_id; + const RTLIL::IdString static_id; }; constexpr IdTableEntry IdTable[] = { @@ -557,15 +530,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(); } }; @@ -574,7 +547,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 bf732df5911ff5f452920c1b93addd4b9638e692 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 442a9698124b754114d910cf72c784ba7127c30a 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 a49b58cda..2610da665 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 @@ -199,6 +200,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 2075b3416f50c1cf91d42e3ce20afba078947d7f 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 028ac388d..468de2758 100644 --- a/kernel/io.cc +++ b/kernel/io.cc @@ -587,7 +587,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 0c5f113e7..b6cb8fec5 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 2610da665..54ba9a7f7 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 @@ -145,8 +144,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_; @@ -192,54 +199,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 @@ -269,17 +237,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 { @@ -335,7 +321,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 { @@ -381,6 +369,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() { @@ -449,7 +445,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 } @@ -462,7 +458,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 15b40a79e..576f59a65 100644 --- a/pyosys/generator.py +++ b/pyosys/generator.py @@ -163,6 +163,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 a534fda855ad8653ba9b3858701c60dad1a51680 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 54ba9a7f7..1d07b6296 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -130,6 +130,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 @@ -260,14 +262,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_; } @@ -284,30 +407,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 { @@ -317,13 +478,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 { @@ -601,13 +762,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 e4a5bd7cb2172eb168ad85329e517c230ad275ac 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 609da65bb4676ebb1c0233688ba57c9c590a33a5 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 c747aa901..bc8d1ee86 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 cd47727c8bffb85177d78c915c20cad2a12b8a23 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 c5c653317..2be8dfa74 100644 --- a/passes/techmap/abc.cc +++ b/passes/techmap/abc.cc @@ -614,7 +614,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 7371388e1d09e401783c42c95ce1013e5f67de33 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 b6cb8fec5..bcb10fad2 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 1d07b6296..ff98ee500 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -578,6 +578,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 @@ -589,6 +591,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 054de3c236bbd0204f610a85216bfc919d7fd0a1 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