grt, rsz: automatic ODB-callback wiring for CUGR and CI log fix#10683
grt, rsz: automatic ODB-callback wiring for CUGR and CI log fix#10683sparsh-karna wants to merge 3 commits into
Conversation
* Removed explicit grt::add_dirty_net calls from src/grt/test/incremental_deleted_net.tcl.
* Removed explicit grt::update_cugr_net calls from src/grt/test/incremental_update_net.tcl.
* Automatic ODB callbacks now handle all necessary topology refreshes and re-routing triggers.
2. SWIG Interface Cleanup:
* Deleted the exported add_dirty_net and update_cugr_net functions from src/grt/src/GlobalRouter.i.
3. C++ Refactoring:
* Removed the redundant GlobalRouter::updateCUGRNet method from GlobalRouter.cpp and GlobalRouter.h as it was only used by the removed SWIG function.
4. Verification:
* Regenerated golden .ok files for the affected tests to align with the new automatic flow.
* Verified that all related integration tests (incremental_deleted_net, and incremental_update_net) pass successfully via ctest.
Signed-off-by: Sparsh Karna <sparsh2005karna@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 refactors the incremental global routing flow when using CUGR, automating net updates and dirty net marking internally instead of relying on manual SWIG interface calls. The review feedback highlights a potential segmentation fault in addDirtyNet if a net is missing from db_net_map_, and points out an unused variable warning in startIncremental when CUGR is disabled.
| std::vector<Net*> nets = initFastRoute(min_layer, max_layer); | ||
| if (use_cugr_) { | ||
| odb::PtrSet<odb::dbNet> clock_nets; | ||
| findClockNets(nets, clock_nets); | ||
|
|
||
| cugr_->setCongestionIterations(congestion_iterations_); | ||
| cugr_->init(min_layer, max_layer, clock_nets); | ||
| } |
There was a problem hiding this comment.
The variable nets is only used when use_cugr_ is true. If use_cugr_ is false, nets is unused, which can trigger compiler warnings or errors (e.g., -Wunused-variable) under strict build configurations. Consider restructuring the block to only declare nets when use_cugr_ is enabled.
if (use_cugr_) {
std::vector<Net*> nets = initFastRoute(min_layer, max_layer);
odb::PtrSet<odb::dbNet> clock_nets;
findClockNets(nets, clock_nets);
cugr_->setCongestionIterations(congestion_iterations_);
cugr_->init(min_layer, max_layer, clock_nets);
} else {
initFastRoute(min_layer, max_layer);
}
Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
Signed-off-by: Sparsh Karna <sparsh2005karna@gmail.com>
| void | ||
| add_dirty_net(odb::dbNet* net) | ||
| { | ||
| if (net != nullptr) { | ||
| getGlobalRouter()->addDirtyNet(net); | ||
| } | ||
| } | ||
|
|
||
| void | ||
| update_cugr_net(odb::dbNet* net) | ||
| { | ||
| getGlobalRouter()->updateCUGRNet(net); | ||
| } |
| Routing statistics | ||
| Wire length: 889 | ||
| Total via count: 30 | ||
| Total congestion: 0 | ||
| Min resource: 1.5 | ||
| Bottleneck: (6, 0, 0) | ||
| Routing statistics | ||
| Wire length: 889 | ||
| Total via count: 30 | ||
| Total congestion: 0 | ||
| Min resource: 1.5 | ||
| Bottleneck: (6, 0, 0) |
There was a problem hiding this comment.
This log seems duplicated. Please check what's exactly going on here. Is CUGR running twice for the same subset of dirty nets?
| getMinMaxLayer(min_layer, max_layer); | ||
| initFastRoute(min_layer, max_layer); | ||
| if (use_cugr_) { | ||
| std::vector<Net*> nets = initFastRoute(min_layer, max_layer); |
There was a problem hiding this comment.
Calling initFastRoute here is awkward, since you're in the CUGR branch. What you really need is the part of CUGR that initializes the netlist:
std::vector<Net*> nets = findNets(true);
check_pin_placement_ = check_pin_placement;
if (check_pin_placement) {
checkPinPlacement();
}
initNetlist(nets);
You can refactor this piece of code out of initFastRoute into an individual function and use it here instead of calling the full initFastRoute function.
| if (use_cugr_) { | ||
| addDirtyNet(db_net1); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Add a comment explaining why we just add the merged nets to the dirty nets list. This can be a TODO comment, explaining that we intend to give full support for merging nets in the future.
| if (use_cugr_) { | ||
| cugr_->routeIncremental(); | ||
| routes_ = cugr_->getRoutes(); | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
This looks correct, as the CUGR APIs seems easier and cleaner to use to update the dirty nets. I'd like to see other tests with more transformations in the design, to see how it behaves.
Look at the global_route.tcl script from ORFS for a reference on how to create a unit test with repair_timing/design calls: https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/master/flow/scripts/global_route.tcl#L51-L90
Notice that repair_design and repair_timing have internal wiring to the incremental GRT, so you don't need to call global_route -start_incremental/-end_incremental before/after them.
2 participants
This PR implements full automatic ODB-callback wiring for the CUGR incremental routing flow, bringing it to feature parity with FastRoute's incremental state management. Structural changes in the database (instance moves, master swaps, connectivity updates) now automatically trigger topology refreshes and re-routing in CUGR without requiring manual Tcl interventions.
Key Changes
GlobalRouter::addDirtyNetto callcugr_->updateNet(net). This ensures that any event marking a net as dirty (like moving an instance) also refreshes CUGR's internal pin and obstruction maps.GlobalRouter::mergeNetsRoutingthat marks affected nets as dirty, ensuring they are re-routed during the next incremental update.addDirtyNet,updateNet,removeNet) to gracefully handle database callbacks that occur before the global router is fully initialized.grt::add_dirty_netandgrt::update_cugr_netTcl/SWIG functions, as they are no longer needed with automatic wiring.Runtime: {:.2f}slog messages in RSZ Python tests to resolve Jenkins CI failures caused by golden file mismatches.Verification
incremental_deleted_net.tclandincremental_update_net.tclto remove manual update calls, confirming the flow is now fully automatic.ctest.Prerequisites
git commit -s).