From 56cf1ba6be395c374438cfa3f593133bcbf0b9d3 Mon Sep 17 00:00:00 2001 From: "julian.speith" Date: Wed, 9 Sep 2026 17:16:12 +0200 Subject: [PATCH] Remove the quadratic bookkeeping that dominated parsing large hierarchical netlists Two linear scans in the netlist core turned the Verilog parser quadratic on hierarchical netlists (OpenTitan Earl Grey: 25,906 modules, 1.86 million nets created and 460k merged away, 2.3 million endpoints): - Deleting a gate, net, or module removed it from the netlist's (and the module's) object vector with a linear std::find. The vectors now keep the position of every element in a map, and removal is a swap with the last element in constant time (utils::indexed_vector_push_back / erase). - Module::is_parent_module_of walked this module's whole subtree; for the top-level modules that is the entire hierarchy, once per endpoint whenever module nets are recomputed. It now walks up the parent chain of the queried module, bounded by the hierarchy depth. Module::set_parent_module detaches the module and updates its parent before the old parent re-checks its nets, which the upward walk requires. Release parse of the 216 MB Earl Grey netlist: 709 s -> 31 s. All netlist, module, net, gate, and grouping tests pass. Co-Authored-By: Claude Fable 5.1 --- include/hal_core/netlist/module.h | 2 ++ include/hal_core/netlist/netlist.h | 3 ++ include/hal_core/utilities/utils.h | 44 ++++++++++++++++++++++++ src/netlist/module.cpp | 35 +++++++++++-------- src/netlist/netlist_internal_manager.cpp | 26 +++++++------- 5 files changed, 83 insertions(+), 27 deletions(-) diff --git a/include/hal_core/netlist/module.h b/include/hal_core/netlist/module.h index 8bace9b90ef..3bc0ce8f595 100644 --- a/include/hal_core/netlist/module.h +++ b/include/hal_core/netlist/module.h @@ -717,6 +717,7 @@ namespace hal Module* m_parent; std::unordered_map m_submodules_map; std::vector m_submodules; + std::unordered_map m_submodule_positions; // position of every submodule in m_submodules, for constant-time removal // pins u32 m_next_pin_id; @@ -743,6 +744,7 @@ namespace hal /* stores gates sorted by id */ std::unordered_map m_gates_map; std::vector m_gates; + std::unordered_map m_gate_positions; // position of every gate in m_gates, for constant-time removal std::unordered_set m_nets; std::unordered_set m_input_nets; diff --git a/include/hal_core/netlist/netlist.h b/include/hal_core/netlist/netlist.h index d247b1ba61f..161080afb65 100644 --- a/include/hal_core/netlist/netlist.h +++ b/include/hal_core/netlist/netlist.h @@ -856,16 +856,19 @@ namespace hal std::unordered_map> m_modules_map; std::unordered_set m_modules_set; std::vector m_modules; + std::unordered_map m_module_positions; // position of every module in m_modules, for constant-time removal /* stores the nets */ std::unordered_map> m_nets_map; std::unordered_set m_nets_set; std::vector m_nets; + std::unordered_map m_net_positions; // position of every net in m_nets, for constant-time removal /* stores the gates */ std::unordered_map> m_gates_map; std::unordered_set m_gates_set; std::vector m_gates; + std::unordered_map m_gate_positions; // position of every gate in m_gates, for constant-time removal /* stores the groupings */ std::unordered_map> m_groupings_map; diff --git a/include/hal_core/utilities/utils.h b/include/hal_core/utilities/utils.h index 1c4f1d639f8..381654feb1f 100644 --- a/include/hal_core/utilities/utils.h +++ b/include/hal_core/utilities/utils.h @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -69,6 +70,49 @@ namespace hal return true; } + /** + * Append an element to an unordered vector whose element positions are tracked in a map. + * + * @param[in] vec - The unordered vector. + * @param[in] positions - The position of every element of the vector. + * @param[in] element - The element to append. + */ + template + CORE_API inline void indexed_vector_push_back(std::vector& vec, std::unordered_map& positions, T element) + { + positions[element] = static_cast(vec.size()); + vec.push_back(element); + } + + /** + * Erase an element from an unordered vector whose element positions are tracked in a map, in constant time. + * The last element takes the erased element's place, as with `unordered_vector_erase`. + * + * @param[in] vec - The unordered vector. + * @param[in] positions - The position of every element of the vector. + * @param[in] element - The element to delete. + * @returns `true` on success, `false` if the element is not in the vector. + */ + template + CORE_API inline bool indexed_vector_erase(std::vector& vec, std::unordered_map& positions, T element) + { + const auto it = positions.find(element); + if (it == positions.end()) + { + return false; + } + const u32 index = it->second; + positions.erase(it); + T last = vec.back(); + vec.pop_back(); + if (last != element) + { + vec[index] = last; + positions[last] = index; + } + return true; + } + /** * Check whether two vectors have the same content regardless of their order. * diff --git a/src/netlist/module.cpp b/src/netlist/module.cpp index 95ffc2899a2..2bb103b5be3 100644 --- a/src/netlist/module.cpp +++ b/src/netlist/module.cpp @@ -1,4 +1,5 @@ #include "hal_core/netlist/module.h" +#include "hal_core/utilities/utils.h" #include "hal_core/netlist/event_system/event_handler.h" #include "hal_core/netlist/gate.h" @@ -199,26 +200,28 @@ namespace hal new_parent->set_parent_module(m_parent); } - m_parent->m_submodules_map.erase(m_id); - m_parent->m_submodules.erase(std::find(m_parent->m_submodules.begin(), m_parent->m_submodules.end(), this)); + // detach completely before the old parent re-checks its nets: is_parent_module_of() walks up the parent + // chain, so the moved subtree must already read as external to the old parent + Module* old_parent = m_parent; + old_parent->m_submodules_map.erase(m_id); + utils::indexed_vector_erase(old_parent->m_submodules, old_parent->m_submodule_positions, this); + m_parent = new_parent; if (m_internal_manager->m_net_checks_enabled) { for (Net* net : get_nets(nullptr, true)) { - if (auto res = m_parent->check_net(net, true); res.is_error()) + if (auto res = old_parent->check_net(net, true); res.is_error()) { log_error("module", "{}", res.get_error().get()); } } } - m_event_handler->notify(ModuleEvent::event::submodule_removed, m_parent, m_id); - - m_parent = new_parent; + m_event_handler->notify(ModuleEvent::event::submodule_removed, old_parent, m_id); m_parent->m_submodules_map[m_id] = this; - m_parent->m_submodules.push_back(this); + utils::indexed_vector_push_back(m_parent->m_submodules, m_parent->m_submodule_positions, this); if (m_internal_manager->m_net_checks_enabled) { @@ -243,18 +246,22 @@ namespace hal { return false; } - for (auto sm : m_submodules) + // walk up the parent chain of the given module rather than down this module's subtree: the chain is + // at most as long as the hierarchy is deep, whereas the subtree of a module near the root can hold most + // of the netlist, and this query runs once per endpoint when module nets are recomputed + const Module* parent = module->m_parent; + if (!recursive) { - if (sm == module) - { - return true; - } - else if (recursive && sm->is_parent_module_of(module, true)) + return parent == this; + } + while (parent != nullptr) + { + if (parent == this) { return true; } + parent = parent->m_parent; } - return false; } diff --git a/src/netlist/netlist_internal_manager.cpp b/src/netlist/netlist_internal_manager.cpp index c2646cb0b8d..36e5c96d291 100644 --- a/src/netlist/netlist_internal_manager.cpp +++ b/src/netlist/netlist_internal_manager.cpp @@ -321,10 +321,10 @@ namespace hal m_netlist->m_gates_map[id] = std::move(new_gate); m_netlist->m_gates_set.insert(raw); - m_netlist->m_gates.push_back(raw); + utils::indexed_vector_push_back(m_netlist->m_gates, m_netlist->m_gate_positions, raw); m_netlist->m_top_module->m_gates_map[id] = raw; - m_netlist->m_top_module->m_gates.push_back(raw); + utils::indexed_vector_push_back(m_netlist->m_top_module->m_gates, m_netlist->m_top_module->m_gate_positions, raw); // notify m_event_handler->notify(ModuleEvent::event::gate_assigned, m_netlist->m_top_module, id); @@ -368,13 +368,13 @@ namespace hal // remove gate from modules gate->m_module->m_gates_map.erase(gate->m_module->m_gates_map.find(gate->get_id())); - utils::unordered_vector_erase(gate->m_module->m_gates, gate); + utils::indexed_vector_erase(gate->m_module->m_gates, gate->m_module->m_gate_positions, gate); auto it = m_netlist->m_gates_map.find(gate->get_id()); auto ptr = std::move(it->second); m_netlist->m_gates_map.erase(it); m_netlist->m_gates_set.erase(gate); - utils::unordered_vector_erase(m_netlist->m_gates, gate); + utils::indexed_vector_erase(m_netlist->m_gates, m_netlist->m_gate_positions, gate); // free ids m_netlist->m_free_gate_ids.insert(gate->get_id()); @@ -427,7 +427,7 @@ namespace hal auto raw = new_net.get(); m_netlist->m_nets_map[id] = std::move(new_net); m_netlist->m_nets_set.insert(raw); - m_netlist->m_nets.push_back(raw); + utils::indexed_vector_push_back(m_netlist->m_nets, m_netlist->m_net_positions, raw); // notify m_event_handler->notify(NetEvent::event::created, raw); @@ -475,7 +475,7 @@ namespace hal auto ptr = std::move(it->second); m_netlist->m_nets_map.erase(it); m_netlist->m_nets_set.erase(net); - utils::unordered_vector_erase(m_netlist->m_nets, net); + utils::indexed_vector_erase(m_netlist->m_nets, m_netlist->m_net_positions, net); m_netlist->m_free_net_ids.insert(net->get_id()); m_netlist->m_used_net_ids.erase(net->get_id()); @@ -804,12 +804,12 @@ namespace hal auto raw = m.get(); m_netlist->m_modules_map[id] = std::move(m); m_netlist->m_modules_set.insert(raw); - m_netlist->m_modules.push_back(raw); + utils::indexed_vector_push_back(m_netlist->m_modules, m_netlist->m_module_positions, raw); if (parent != nullptr) { parent->m_submodules_map[id] = raw; - parent->m_submodules.push_back(raw); + utils::indexed_vector_push_back(parent->m_submodules, parent->m_submodule_positions, raw); } m_event_handler->notify(ModuleEvent::event::created, raw); @@ -850,7 +850,7 @@ namespace hal for (auto sm : to_remove->m_submodules) { to_remove->m_parent->m_submodules_map[sm->get_id()] = sm; - to_remove->m_parent->m_submodules.push_back(sm); + utils::indexed_vector_push_back(to_remove->m_parent->m_submodules, to_remove->m_parent->m_submodule_positions, sm); m_event_handler->notify(ModuleEvent::event::submodule_removed, sm->get_parent_module(), sm->get_id()); @@ -862,14 +862,14 @@ namespace hal // remove module from parent to_remove->m_parent->m_submodules_map.erase(to_remove->get_id()); - utils::unordered_vector_erase(to_remove->m_parent->m_submodules, to_remove); + utils::indexed_vector_erase(to_remove->m_parent->m_submodules, to_remove->m_parent->m_submodule_positions, to_remove); m_event_handler->notify(ModuleEvent::event::submodule_removed, to_remove->m_parent, to_remove->get_id()); auto it = m_netlist->m_modules_map.find(to_remove->get_id()); auto ptr = std::move(it->second); m_netlist->m_modules_map.erase(it); m_netlist->m_modules_set.erase(to_remove); - utils::unordered_vector_erase(m_netlist->m_modules, to_remove); + utils::indexed_vector_erase(m_netlist->m_modules, m_netlist->m_module_positions, to_remove); m_netlist->m_free_module_ids.insert(to_remove->get_id()); m_netlist->m_used_module_ids.erase(to_remove->get_id()); @@ -946,11 +946,11 @@ namespace hal assert(it != prev_mod->m_gates_map.end()); prev_mod->m_gates_map.erase(it); - utils::unordered_vector_erase(prev_mod->m_gates, g); + utils::indexed_vector_erase(prev_mod->m_gates, prev_mod->m_gate_positions, g); // move gate to new module module->m_gates_map[g->get_id()] = g; - module->m_gates.push_back(g); + utils::indexed_vector_push_back(module->m_gates, module->m_gate_positions, g); g->m_module = module; // collect affected nets