pdn: stabilize special wire write order#10367
Conversation
Signed-off-by: naveenvenk17 <naveenvenkat1711@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request improves the determinism of PDN generation by ensuring that nets and shapes are processed in a stable, predictable order. It introduces a net_order vector to track net processing and implements a custom sorting mechanism for shapes based on layer, net name, and geometry. A new test case, core_grid_swire_order, has been added to verify that special wires are created according to voltage domain order rather than internal pointer addresses. Feedback was provided regarding the shape sorting comparator, specifically suggesting an optimization to avoid redundant string comparisons and adding safety checks to prevent potential null pointer dereferences when a shape lacks an associated net.
| std::ranges::sort(shape_order, [](const Shape* lhs, const Shape* rhs) { | ||
| const int lhs_layer = lhs->getLayer()->getNumber(); | ||
| const int rhs_layer = rhs->getLayer()->getNumber(); | ||
| if (lhs_layer != rhs_layer) { | ||
| return lhs_layer < rhs_layer; | ||
| } | ||
|
|
||
| const int net_cmp | ||
| = lhs->getNet()->getName().compare(rhs->getNet()->getName()); | ||
| if (net_cmp != 0) { | ||
| return net_cmp < 0; | ||
| } | ||
|
|
||
| return lhs->getRect() < rhs->getRect(); | ||
| }); |
There was a problem hiding this comment.
The current comparator performs string comparisons for every pair of shapes, which can be inefficient. Additionally, while PDN shapes typically have associated nets, the Shape class allows for null nets (e.g., for obstructions). If a shape with a null net were to be included in shape_map, the current code would crash during getName() dereferencing.
Consider comparing the dbNet* pointers first to avoid string comparisons for shapes on the same net, and add safety checks for null nets.
std::ranges::sort(shape_order, [](const Shape* lhs, const Shape* rhs) {
const int lhs_layer = lhs->getLayer()->getNumber();
const int rhs_layer = rhs->getLayer()->getNumber();
if (lhs_layer != rhs_layer) {
return lhs_layer < rhs_layer;
}
auto* lhs_net = lhs->getNet();
auto* rhs_net = rhs->getNet();
if (lhs_net != rhs_net) {
if (lhs_net == nullptr) {
return true;
}
if (rhs_net == nullptr) {
return false;
}
const int net_cmp = lhs_net->getName().compare(rhs_net->getName());
if (net_cmp != 0) {
return net_cmp < 0;
}
}
return lhs->getRect() < rhs->getRect();
});There was a problem hiding this comment.
Addressed in 414d959 by guarding null nets and avoiding the net-name comparison when both shapes already reference the same net.
Signed-off-by: naveenvenk17 <naveenvenkat1711@gmail.com>
|
std::map order is stabilized by the std::less defined in https://github.com/The-OpenROAD-Project/OpenROAD/blob/master/src/odb/include/odb/dbCompare.inc |
|
|
Fixes #7746.
This makes PDN write generated special wires in voltage-domain net order instead of iterating a
std::map<dbNet*, ...>by raw pointer value. It also makes cleanup of floating generated shapes deterministic by sorting shapes by layer, net name, and rect before destruction, avoiding another pointer-keyed order source that can affect the serialized ODB even when DEF output is identical.A focused regression creates
VSSbeforeVDD, then checks that PDN-created special-wire IDs still follow the voltage-domain order (VDD, thenVSS).Tests:
python -m py_compile src/pdn/test/core_grid_swire_order.pygit diff --checkNot run locally:
bazelisk test //src/pdn/test:core_grid_swire_order //src/pdn/test:core_grid_swire_order-py_test --test_output=errorsbecause this Windows environment does not have bazelisk/bazel/cmake/ninja or an OpenROAD binary on PATH, WSL timed out on a trivial command, and Docker Desktop is not running.