Skip to content

dpl: negotiation dynamic window#10807

Open
gudeh wants to merge 27 commits into
The-OpenROAD-Project:masterfrom
gudeh:dpl-negotiation-dynamic-window
Open

dpl: negotiation dynamic window#10807
gudeh wants to merge 27 commits into
The-OpenROAD-Project:masterfrom
gudeh:dpl-negotiation-dynamic-window

Conversation

@gudeh

@gudeh gudeh commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Makes the Negotiation Legalizer search window range adaptive instead of fixed X and Y dimensions.

We apply two extensions:

  1. An extension is applied depending on instance size.
    • For Y direction we guarantee at least the cell height times the tunable constant (kRowSearchWindow, default 5).
      • We consider for at least kRowSearchWindow valid rows. Multi-height rows may require more rows included.
    • For the X direction we guarantee at least 1 full instance width on each side.
    • Both X and Y are clamped at the same max displacement used for Diamond Search.

Experiments showed far more problems on the Y direction with multi-height and hybrid row structures, requiring more extensions on such dimension.

  1. A second extension may be applied if an instance is close to an off-core or macro border, we apply an extension to the search window on the opposite side overlapping the off-core or macro.
    • Experiments showed problems with instances close to borders and neighboring fixed instances.

Type of Change

  • New feature

Impact

This solves convergence issues Negotiation Legalizer was having with multi-height cells designs. We were getting search windows too small, and a few instances never moving.

Verification

  • I have verified that the local build succeeds (./etc/Build.sh).
  • I have run the relevant tests and they pass.
  • My code follows the repository's formatting guidelines.
  • I have included tests to prevent regressions.
  • I have signed my commits (DCO).

Related Issues

This is one of multiple PRs splitting up PR #10226 to make Negotiation the default algorithm at DPL.

This is the last PR to solve all known issues with Negotiation Legalizer. After this one I expect only to make cosmetic modifications and actually switching to the new default.

gudeh added 27 commits June 10, 2026 21:52
stop every iteration on iterative mode, include stop on final state,
introduce iterative_jump to stop at desired iteration

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…oves in grey, track current-iter movers

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…sites,

print table similar to gpl with controled number of lines, less verbose,
rename overflow to violations

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
this fix a debug visualization bug,
also rename some functions for better clarity

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
include more debug info for stuck cells

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
make sure the window fits properly with valid rows,
Y range is given by min between: (cell height * constantY), versus max y displacement
X range is given by min between: max(constantX, cell width), versus max x displacement

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
include new debug messa for debuging window range

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…gotiation-dynamic-window

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…verbosity

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…t row only

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…d macros

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
…larity also

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
@gudeh gudeh requested a review from a team as a code owner July 4, 2026 05:58
@gudeh gudeh requested a review from osamahammad21 July 4, 2026 05:58
@gudeh

gudeh commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

The diffs here should be more clear after #10681 is merged.

@gudeh gudeh changed the title Dpl negotiation dynamic window dpl: negotiation dynamic window Jul 4, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the detailed_placement command in OpenDP by introducing configurable search windows (-site_search_window, -row_search_window) and DRC penalties (-drc_penalty) for the NegotiationLegalizer. It also adds stuck-cell diagnostics, improves GUI debugging with a negotiation violations chart, and updates the observer to track current-iteration movers. Feedback is provided regarding a potential null pointer dereference when retrieving the master of an instance in Opendp::detailedPlacement, which should be guarded to prevent segmentation faults.

Comment thread src/dpl/src/Opendp.cpp
Comment on lines +166 to +170
for (odb::dbInst* inst : block_->getInsts()) {
odb::dbMaster* master = inst->getMaster();
total_inst_area += static_cast<int64_t>(master->getWidth())
* static_cast<int64_t>(master->getHeight());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In the calculation of total_inst_area, inst->getMaster() is called and dereferenced directly without checking if it is nullptr. While instances in a placed design typically have a master, it is safer to add a null check to prevent potential segmentation faults in edge cases or during intermediate database states.

    for (odb::dbInst* inst : block_->getInsts()) {
      odb::dbMaster* master = inst->getMaster();
      if (master != nullptr) {
        total_inst_area += static_cast<int64_t>(master->getWidth())
                           * static_cast<int64_t>(master->getHeight());
      }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant