Skip to content

pad: persist RDL bump routing assignments - #11621

Open
mahjiid wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
mahjiid:fix/rdl-route-missing-assignments
Open

mahjiid wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
mahjiid:fix/rdl-route-missing-assignments

Conversation

@mahjiid

@mahjiid mahjiid commented Oct 3, 2026

Copy link
Copy Markdown

Summary

Persist explicit assign_io_bump RDL routing intent across write_db / read_db.

Problem

assign_io_bump -terminal and -dont_route are currently stored only in ICeWall::routing_map_. After a database checkpoint/reload that map is empty, so rdl_route can silently route a different topology.

Changes

  • Store explicit -terminal targets as a string property on the bump ITerm.
  • Store explicit -dont_route intent as a boolean property on the bump ITerm.
  • Clear stale persisted intent when a bump is reassigned.
  • Teach RDLRouter to restore persisted intent when no in-memory routing-map entry exists.
  • Extend rdl_route_assignments with a real write_db -> separate-process read_db round trip before routing.

Validation

  • git diff --check passes.
  • Commit includes DCO sign-off.
  • Regression uses the existing golden DEF, so routing after reload must remain identical to the pre-checkpoint expected result.
  • Full OpenROAD CI is left to the project workflow because this host does not have the complete Bazel/C++ toolchain installed.

Fixes #11297

Signed-off-by: Abdulrasheed Abdulmajeed <miloboy188@gmail.com>
@mahjiid
mahjiid requested a review from a team as a code owner October 3, 2026 11:55
@mahjiid
mahjiid requested a review from gadfort October 3, 2026 11:55
@github-actions github-actions Bot added the size/S label Oct 3, 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 ensures that explicit -terminal and -dont_route bump assignments survive database checkpoints by persisting them as properties on the bump terminal. The feedback highlights potential robustness issues when restoring these properties, specifically warning about silent failures if a target terminal is missing or if there is a net mismatch between the bump and its target terminal, and suggests adding defensive checks and warnings.

Comment thread src/pad/src/RDLRouter.cpp
Comment on lines +249 to +254
} else if (auto* target
= odb::dbStringProperty::find(iterm, kRdlRouteTargetProperty);
target != nullptr) {
if (auto* target_iterm = block_->findITerm(target->getValue().c_str())) {
iterm_pairs.push_back(target_iterm);
}

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.

high

When restoring the explicit routing target from the persisted string property, there are two potential correctness and robustness issues:

  1. Silent Failure on Missing Target: If the target ITerm is not found in the design (e.g., due to design changes or renaming), the routing is silently skipped because the else block is bypassed. This should be logged as a warning/error.
  2. Net Mismatch: If the restored target ITerm belongs to a different net than the bump ITerm, routing them together would create a short or net mismatch. We should perform a defensive check to ensure their nets match before adding the target to iterm_pairs.
      } else if (auto* target
                 = odb::dbStringProperty::find(iterm, kRdlRouteTargetProperty);
                 target != nullptr) {
        auto* target_iterm = block_->findITerm(target->getValue().c_str());
        if (target_iterm != nullptr) {
          if (target_iterm->getNet() == iterm->getNet()) {
            iterm_pairs.push_back(target_iterm);
          } else {
            logger_->warn(utl::PAD,
                          100,
                          "Persisted RDL routing target {} net {} does not match bump {}/{} net {}",
                          target->getValue(),
                          target_iterm->getNet() ? target_iterm->getNet()->getName() : "NULL",
                          iterm->getInst()->getName(),
                          iterm->getMTerm()->getName(),
                          iterm->getNet() ? iterm->getNet()->getName() : "NULL");
          }
        } else {
          logger_->warn(utl::PAD,
                        101,
                        "Persisted RDL routing target {} for bump {}/{} not found in design",
                        target->getValue(),
                        iterm->getInst()->getName(),
                        iterm->getMTerm()->getName());
        }
      }

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.

pad: rdl_route gives a different result after write_db/read_db — is assign_io_bump's assignment meant to be session state?

1 participant