[GPL] Add set_net_weight/unset_net_weight commands

[jeffrey-song-melten]

Summary

• Add set_net_weight / unset_net_weight TCL commands for per-net custom placement weight
• Weights stored as dbDoubleProperty("gpl_weight") on dbNet — persistent across write_db/read_db
• NesterovBase reads the property during GNet construction via existing setCustomWeight()

Motivation

External tools (e.g., timing-driven convergence loops) need to guide global placement by annotating specific nets with higher importance. This is useful for IP-level characterization → integration PnR workflows where sensitive nets identified during standalone analysis should receive higher placement priority.

Test plan

• net_weight01.tcl: 7 positive tests (set, verify ODB property, unset, multiple nets, global_placement with weights, overwrite, write_db persistence)
• net_weight_negative.tcl: 7 negative tests (non-existent net, zero weight, negative weight, unset non-existent, wrong arg count, non-numeric, unset without prior set)
• Tests registered in CMakeLists.txt and BUILD
• Golden .ok files generated

Related

• Related issue: [DPL] Crash in detailed_placement after dbRow_destroy + die resize #10048 (DPL crash during incremental die shrink — separate issue, discovered during same investigation)

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces the ability to set and unset custom net weights for global placement. It adds Tcl commands (set_net_weight, unset_net_weight), underlying C++ implementation using ODB properties to ensure persistence, and comprehensive functional and negative tests. The review feedback highlights several improvement opportunities: adding return statements after error logging to prevent potential null pointer dereferences if the program continues, using named constants instead of magic strings for property names, and optimizing property updates by modifying existing values instead of destroying and recreating them.

[gemini-code-assist]

There are a couple of improvements that can be made here:

1. Robustness: While logger->error is expected to terminate the program, the code proceeds to dereference net on the following lines, which will cause a crash if net is null and termination does not occur. It's safer to add a return statement after calling error.
2. Maintainability: To avoid magic strings, it's a good practice to define property names like "gpl_weight" as a constant.
3. Efficiency: It's more efficient to update the property value if it exists, instead of destroying and recreating it.

The suggested code applies these improvements.

  if (!net) {
    ord::OpenRoad::openRoad()->getLogger()->error(
        utl::GPL, 500, "Net '{}' not found.", net_name);
    return;
  }
  // Store as persistent ODB property so it survives write_db/read_db
  static const char* gpl_weight_prop_name = "gpl_weight";
  if (auto* prop = odb::dbDoubleProperty::find(net, gpl_weight_prop_name)) {
    prop->setValue(weight);
  } else {
    odb::dbDoubleProperty::create(net, gpl_weight_prop_name, weight);
  }

References

1. Define tunable parameters as named constants instead of using hardcoded magic numbers.

[gemini-code-assist]

There are a couple of improvements that can be made here:

1. Robustness: While logger->error is expected to terminate, the code proceeds to dereference net on the following lines, which will cause a crash if net is null and termination does not occur. It's safer to add a return statement after calling error.
2. Maintainability: To avoid magic strings, it's a good practice to define property names like "gpl_weight" as a constant.

The suggested code applies these improvements and uses a more concise if statement with an initializer.

  if (!net) {
    ord::OpenRoad::openRoad()->getLogger()->error(
        utl::GPL, 501, "Net '{}' not found.", net_name);
    return;
  }
  static const char* gpl_weight_prop_name = "gpl_weight";
  if (auto* existing = odb::dbDoubleProperty::find(net, gpl_weight_prop_name)) {
    odb::dbDoubleProperty::destroy(existing);
  }

References

1. Define tunable parameters as named constants instead of using hardcoded magic numbers.

[gemini-code-assist]

To improve maintainability and avoid magic strings, it's a good practice to define property names as constants. This also allows for a more concise check using an if statement with an initializer.

A similar change should be applied in src/gpl/src/replace.i where this property is set.

    static const char* gpl_weight_prop_name = "gpl_weight";
    if (auto* wp = odb::dbDoubleProperty::find(net->getDbNet(), gpl_weight_prop_name)) {
      myGNet.setCustomWeight(static_cast<float>(wp->getValue()));
    }

References

1. Define tunable parameters as named constants instead of using hardcoded magic numbers.

[gudeh]

Would be nice to have better names for variables, for example renaming wp and myGNet. Perhaps net_property, and temp_gnet?

[gudeh]

Most importantly, the setting of timing-driven weights does not originally happen here. This is currently not protected by timing-driven mode being activated.

I believe this should happen at:

OpenROAD/src/gpl/src/timingBase.cpp

Line 120 in 29d97c4

bool TimingBase::executeTimingDriven(bool run_journal_restore)

Notice how we originally set weights at the following, after RSZ is executed:

OpenROAD/src/gpl/src/timingBase.cpp

Line 173 in 29d97c4

gNet->setTimingWeight(weight);

[gudeh]

This is an interesting new feature!