Conversation
Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>
…efok Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for secondary ground nets in voltage domains, which is a valuable addition for designs with multiple ground rails. The changes are well-implemented across the C++ code, SWIG interface, and TCL commands. The logic for ordering nets in VoltageDomain::getNets() has been correctly updated to primary power -> secondary power(s) -> secondary ground(s) -> primary ground. The introduction of PdnGen::run is a good refactoring that encapsulates the grid generation process and adds timing information. I've included a couple of suggestions to reduce code duplication, which would improve maintainability.
src/pdn/src/domain.cpp
Outdated
| if (!secondary_power_.empty()) { | ||
| std::string nets; | ||
| for (auto* net : secondary_power_) { | ||
| nets += net->getName() + " "; | ||
| } | ||
| logger_->report(" Secondary power nets: {}", nets); | ||
| } | ||
|
|
||
| if (!secondary_ground_.empty()) { | ||
| std::string nets; | ||
| for (auto* net : secondary_) { | ||
| for (auto* net : secondary_ground_) { | ||
| nets += net->getName() + " "; | ||
| } | ||
| logger_->report(" Secondary nets: {}", nets); | ||
| logger_->report(" Secondary ground nets: {}", nets); | ||
| } |
There was a problem hiding this comment.
The logic for reporting secondary power and ground nets is duplicated. This can be refactored into a helper function to avoid repetition and improve readability. According to repository guidelines, helper logic should be defined as a free function in a namespace rather than as a local lambda within a function.
References
- Helper logic should be defined as a free function in a namespace rather than as a local lambda within a function.
There was a problem hiding this comment.
Refactored into a reportSecondaryNets() free function in an anonymous namespace.
src/pdn/src/pdn.tcl
Outdated
| # Collect secondary power nets (placed between primary power and ground). | ||
| set secondary_power {} | ||
| if { [info exists keys(-secondary_power)] } { | ||
| foreach snet $keys(-secondary_power) { | ||
| set db_net [[ord::get_db_block] findNet $snet] | ||
| if { $db_net == "NULL" } { | ||
| utl::error PDN 1006 "Unable to find secondary power net: $snet" | ||
| } else { | ||
| lappend secondary $db_net | ||
| lappend secondary_power $db_net | ||
| } | ||
| } | ||
| } | ||
|
|
||
| # Collect secondary ground nets (placed between secondary power and primary | ||
| # ground). | ||
| set secondary_ground {} | ||
| if { [info exists keys(-secondary_ground)] } { | ||
| foreach snet $keys(-secondary_ground) { | ||
| set db_net [[ord::get_db_block] findNet $snet] | ||
| if { $db_net == "NULL" } { | ||
| utl::error PDN 1007 "Unable to find secondary ground net: $snet" | ||
| } else { | ||
| lappend secondary_ground $db_net | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for collecting secondary power nets and secondary ground nets is duplicated. To improve maintainability and reduce code duplication, consider creating a helper procedure that takes the key name, error code, and net type string as arguments and returns the list of nets.
There was a problem hiding this comment.
Extracted into a pdn::collect_secondary_nets helper proc shared by both secondary power and ground collection
Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>
1 participant
As per your request i have moved the PDN to its own pr @maliberty.
Added
-secondary_groundparameter toset_voltage_domainfor designs with secondary ground rails (e.g. VSSA).Also fixes
VoltageDomain::getNets()which was placing secondary nets after ground instead of between primary power and ground. Correct order is:primary power → secondary power(s) → secondary ground(s) → primary ground
Two regression tests added;
CMakeListsregistration pending.defokgeneration.