dpl: draw diamond search as a per-instance outline - #11055
Conversation
Record diamond-search candidates for every cell, keyed by dbInst, instead of only for the debug instance, and clear them per cell at the start of each search. drawObjects() then unions the candidates and strokes just the exterior boundary for the debug instance plus whatever is selected in the GUI, so a cell's last search can be inspected by selecting it -- the same workflow as the negotiation search windows. Filled per-candidate boxes hid the cells underneath and, since consecutive candidates are one site apart, stacked into an unreadable blob. Pink is the one hue not already taken by this renderer. No effect outside debug mode: the observer only exists after set_debug_dpl and both call sites are already guarded. Also report the negotiation best location in absolute dbu so it can be compared against the reported search windows. Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.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 enhances the detailed placement (DPL) debugging graphics by tracking and displaying diamond search candidate regions for multiple cells, using boost::polygon to union overlapping areas and draw their exterior boundaries. The review feedback highlights potential null pointer dereferences on the cell pointer in Graphics::binSearch and Graphics::clearDiamondSearch, and suggests calling clearAllDiamondSearches() at the start of placement to prevent stale visualization data from persisting.
| const odb::dbInst* inst = cell->getDbInst(); | ||
| if (!inst) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The cell pointer is dereferenced via cell->getDbInst() without a prior null check. If cell is null, this will cause a null pointer dereference and crash the application. Please add a defensive check for cell before calling getDbInst().
| const odb::dbInst* inst = cell->getDbInst(); | |
| if (!inst) { | |
| return; | |
| } | |
| if (!cell) { | |
| return; | |
| } | |
| const odb::dbInst* inst = cell->getDbInst(); | |
| if (!inst) { | |
| return; | |
| } |
| void Graphics::clearDiamondSearch(const Node* cell) | ||
| { | ||
| auto it = searched_diamond_.find(cell->getDbInst()); | ||
| if (it != searched_diamond_.end()) { | ||
| // Keep the allocation: the same cell gets searched again on later passes. | ||
| it->second.clear(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The cell pointer is dereferenced via cell->getDbInst() without a prior null check. If cell is null, this will cause a null pointer dereference and crash the application. Please add a defensive check for cell before calling getDbInst().
void Graphics::clearDiamondSearch(const Node* cell)
{
if (!cell) {
return;
}
const odb::dbInst* inst = cell->getDbInst();
if (!inst) {
return;
}
auto it = searched_diamond_.find(inst);
if (it != searched_diamond_.end()) {
// Keep the allocation: the same cell gets searched again on later passes.
it->second.clear();
}
}| void Graphics::clearAllDiamondSearches() | ||
| { | ||
| searched_diamond_.clear(); | ||
| } |
There was a problem hiding this comment.
To prevent stale diamond search outlines from previous placement runs from persisting in the GUI, clearAllDiamondSearches() should also be called inside Graphics::startPlacement(odb::dbBlock* block) when a new placement run begins.
1 participant
Summary
We currently still use diamond search legacy algorithm as a fall back for corner cases when we fail to legalize an instance with negotiation.
This PR reworks the diamond search debug drawing. Previously when we have a defined debug instance from TCL parameter we would draw in teal the search space. It had an issue where we would not clear the drawing even after clicking continue in debug mode:
Now it works more similar to the debug drawing of the negotiation search window. We do not need to set the debug instance from the TCL parameter, we store the search range of each instance and if we click on it we get the diamond search outline, I also changed the drawing so it is just an outline and not draw on top of the layout.
Notice we get the negotiation window there (smaller one in teal).
Type of Change
Impact
No-op, debug changes only.
Verification
./etc/Build.sh).Related Issues