Skip to content

odb: support embedded chiplets via internal-side regions#10982

Open
osamahammad21 wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
osamahammad21:3dblox-internal-region-embedded-chiplet
Open

odb: support embedded chiplets via internal-side regions#10982
osamahammad21 wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
osamahammad21:3dblox-internal-region-embedded-chiplet

Conversation

@osamahammad21

Copy link
Copy Markdown
Member

Summary

  • Restrict INTERNAL/INTERNAL_EXT region sides to RDL and SUBSTRATE chiplets; error otherwise during 3DBV parsing.
  • Skip the overlapping-chips check for chip pairs connected through an INTERNAL_EXT region, so a legitimately embedded chiplet is not reported as an overlap violation.

Type of Change

  • New feature

Impact

  • 3DBV designs with embedded chiplets no longer emit spurious overlap markers.
  • Invalid internal-side regions on non-RDL/SUBSTRATE chiplets now fail fast with ODB-0564.

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).

osamahammad21 added 2 commits July 23, 2026 21:28
Signed-off-by: osamahammad21 <osama@precisioninno.com>
…EXT region

Signed-off-by: osamahammad21 <osama@precisioninno.com>
@osamahammad21
osamahammad21 requested a review from a team as a code owner July 23, 2026 19:00
@osamahammad21
osamahammad21 requested a review from maliberty July 23, 2026 19:00

@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 introduces validation rules for creating chip regions, restricting INTERNAL and INTERNAL_EXT sides to RDL or SUBSTRATE chiplets, and preventing region creation on hierarchical chips. It also updates the overlapping chip checker to avoid flagging expected geometric overlaps for embedded chiplets connected via INTERNAL_EXT regions, and updates the corresponding unit tests. The review feedback suggests correcting a typo in an error message (changing 'RTL' to 'RDL'), simplifying deep pointer chaining in the checker using a helper method, and fixing a missing space in another error message.

Comment thread src/odb/src/3dblox/3dblox.cpp Outdated
Comment thread src/odb/src/3dblox/checker.cpp Outdated
Comment thread src/odb/src/db/dbChipRegion.cpp Outdated
Signed-off-by: osamahammad21 <osama@precisioninno.com>
@maliberty

Copy link
Copy Markdown
Member

@codex review

@maliberty

Copy link
Copy Markdown
Member

Appears to have real CI failures.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17ef408444

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +366 to +367
const Rect cavity = bot->getCuboid().getEnclosingRect().intersect(
top->getCuboid().getEnclosingRect());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Guard disjoint INTERNAL_EXT regions before intersecting

When an INTERNAL_EXT connection has disjoint XY region footprints, as can occur in a malformed design that the connection checker is supposed to report, Rect::intersect() hits its assert(intersects(r)) and aborts assertion-enabled builds before a "Connection regions" marker can be emitted. Check the footprints first or use the non-asserting intersection API so invalid connections remain diagnosable.

Useful? React with 👍 / 👎.

Comment on lines +233 to +236
if ((side == dbChipRegion::Side::INTERNAL_EXT
|| side == dbChipRegion::Side::INTERNAL)
&& (chip->getChipType() != dbChip::ChipType::RDL
&& chip->getChipType() != dbChip::ChipType::SUBSTRATE)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Update the existing DIE-based INTERNAL region test fixture

This restriction makes the registered TestChips suite fail during ChipHierarchyFixture construction: memory_chip is created with the default DIE type, then createChipRegions() creates its R3 region with side INTERNAL, which now throws ODB-0566 before every fixture test runs. The checker fixture was converted to SUBSTRATE, but this existing fixture must also be reconciled with the new invariant.

Useful? React with 👍 / 👎.

}
dbChipRegion* chip_region = dbChipRegion::create(
chip, region.name, getChipRegionSide(region.side, logger_), layer);
const auto side = getChipRegionSide(region.side, logger_);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Sign off the commit for DCO compliance

The raw commit message for 7a008af5a62ef3ce984f79ae49d4ed43b14d17fb has no Signed-off-by: trailer, despite its verification text claiming DCO sign-off; recreate the commit with git commit -s before submission.

AGENTS.md reference: AGENTS.md:L18-L18

Useful? React with 👍 / 👎.

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.

2 participants