Pdn secondary ground by Bbn08 · Pull Request #9521 · The-OpenROAD-Project/OpenROAD

Bbn08 · 2026-02-23T07:10:20Z

As per your request i have moved the PDN to its own pr @maliberty.

Added -secondary_ground parameter to set_voltage_domain for 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; CMakeLists registration pending .defok generation.

Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>

…efok Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>

gemini-code-assist

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.

gemini-code-assist · 2026-02-23T07:12:00Z

src/pdn/src/domain.cpp

+  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);
  }


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.

Refactored into a reportSecondaryNets() free function in an anonymous namespace.

gemini-code-assist · 2026-02-23T07:12:00Z

src/pdn/src/pdn.tcl

+  # 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
      }
    }
  }


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.

Extracted into a pdn::collect_secondary_nets helper proc shared by both secondary power and ground collection

Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>

gadfort · 2026-02-23T15:53:00Z

src/pdn/src/domain.cpp

+void reportSecondaryNets(utl::Logger* logger,
+                         const char* label,
+                         const std::vector<odb::dbNet*>& nets)
+{
+  if (nets.empty()) {
+    return;
+  }
+  std::string names;
+  for (auto* net : nets) {
+    names += net->getName() + " ";
+  }
+  logger->report("  Secondary {} nets: {}", label, names);
+}
+}  // namespace


I think you can just make a loop in the reporting function to avoid needing to add this function.

gadfort · 2026-02-23T15:54:38Z

src/pdn/src/pdn.tcl

+  foreach snet $net_names {
+    set db_net [[ord::get_db_block] findNet $snet]
+    if { $db_net == "NULL" } {
+      utl::error PDN $err_id "Unable to find secondary $type_label net: $snet"


I would avoid the use of a variable for the err_id since that will not be detectable by the id parser.

gadfort · 2026-02-23T15:55:56Z

src/pdn/src/PdnGen.cc

+  utl::Timer timer;
+  buildGrids(trim);
+  writeToDb(add_pins, report_file);
+  logger_->info(utl::PDN, 500, "Runtime: {:.2f}s", timer.elapsed());


I like this function, but I think we need to resolve the formatting question before adding the timer code here.

gadfort · 2026-02-23T15:57:13Z

src/pdn/src/domain.cpp

+    nets.push_back(power_);
  }

-  nets.insert(nets.end(), secondary_.begin(), secondary_.end());


I think it will be okay, but I'm a little worried about reordering of the nets for folks who are currently expecting the current order.

github-actions

clang-tidy made some suggestions

github-actions · 2026-02-24T00:01:27Z

src/pdn/src/PdnGen.cc

  updateRenderer();
 }

+void PdnGen::run(bool trim, bool add_pins, const std::string& report_file)


warning: no header providing "std::string" is directly included [misc-include-cleaner]

src/pdn/src/PdnGen.cc:12:

- #include <utility> + #include <string> + #include <utility>

Bbn08 added 2 commits February 23, 2026 12:03

pdn: add secondary_ground support and fix secondary power net ordering

e4a4dab

Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>

pdn: add run() spanning buildGrids+writeToDb, disable tests pending d…

9d447f5

…efok Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>

gemini-code-assist bot reviewed Feb 23, 2026

View reviewed changes

pdn: refactor duplicated secondary net logic into helpers

d399058

Signed-off-by: Bbn08 <atrancendentbeing@gmail.com>

gadfort requested changes Feb 23, 2026

View reviewed changes

github-actions bot reviewed Feb 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pdn secondary ground#9521

Pdn secondary ground#9521
Bbn08 wants to merge 3 commits intoThe-OpenROAD-Project:masterfrom
Bbn08:pdn-secondary-ground

Bbn08 commented Feb 23, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Feb 23, 2026

Uh oh!

Bbn08 Feb 23, 2026

Uh oh!

gemini-code-assist bot Feb 23, 2026

Uh oh!

Bbn08 Feb 23, 2026

Uh oh!

gadfort Feb 23, 2026

Uh oh!

gadfort Feb 23, 2026

Uh oh!

gadfort Feb 23, 2026

Uh oh!

gadfort Feb 23, 2026

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Bbn08 commented Feb 23, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Bbn08 Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Bbn08 Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

gadfort Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

gadfort Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

gadfort Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

gadfort Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants