ODB: replace struct UnfoldedModel with dbObject-backed unfolded model by osamahammad21 · Pull Request #10588 · The-OpenROAD-Project/OpenROAD

osamahammad21 · 2026-06-03T16:19:12Z

Summary

Replaces the struct-based UnfoldedModel with a persistent dbObject-backed unfolded model that lives under dbDatabase. Adds five new dbObject classes — dbUnfoldedChip, dbUnfoldedRegion, dbUnfoldedBump, dbUnfoldedConn, dbUnfoldedNet — populated by dbUnfoldedBuilder (which walks the chip-instance tree the same way the old constructor did). Geometry that the struct model stored (chip/region cuboids, bump global positions) is now computed on demand from source dbInst transforms instead of cached. The 3DBlox checker is migrated to the new API; unfoldedModel.{h,cpp} and the old getUnfoldedModel() accessor are removed.

Type of Change

Refactoring

Impact

dbDatabase::constructUnfoldedModel() is preserved as the build entry point; it now populates the new dbObject tables instead of the old struct.
The checker emits the same markers as before for every check it ran previously. The per-region isUsed bit is no longer stored — checkInternalExtUsage derives the same set from a single pass over getUnfoldedConns().
Memory: the unfolded model no longer maintains per-chip bump_inst_map / region_map indices (the largest cost in 3DBlox-scale designs); region cuboids and bump global positions are computed, not stored. The dbObject tables themselves are flagged no-serial for now (no .odb size change).
Alignment-marker checking is kept as a checker-local concept (no new schema). It has a single consumer; when STA introduces a shared dbUnfoldedInst, alignment markers can fold into that.
Auto-rebuild from _dbDatabase deserialization (multi-chip designs) is unchanged.

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 signed my commits (DCO).

Related Issues

N/A

Signed-off-by: osamahammad21 <osama@precisioninno.com>

…ructuree Signed-off-by: osamahammad21 <osama@precisioninno.com>

gemini-code-assist

Code Review

This pull request refactors the unfolded model representation in ODB by replacing the non-persistent UnfoldedModel with persistent dbObject-derived classes (dbUnfoldedChip, dbUnfoldedRegion, dbUnfoldedBump, dbUnfoldedConn, and dbUnfoldedNet) and introducing a dbUnfoldedBuilder. The review feedback highlights several critical issues, including a missing table instantiation in one of the dbDatabase constructors, multiple potential null pointer dereferences due to a lack of defensive checks on lookups, and style violations where helper lambdas should be refactored into free functions in anonymous namespaces.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-03T16:23:07Z

  chip_net_itr_ = new dbChipNetItr(chip_net_tbl_);

-  unfolded_model_ = nullptr;
+  unfolded_region_itr_ = new dbUnfoldedRegionItr(unfolded_region_tbl_);


The second constructor _dbDatabase::_dbDatabase(_dbDatabase* db, int id) is missing the instantiation of the new unfolded_*_tbl_ tables (e.g., unfolded_chip_tbl_, unfolded_region_tbl_, etc.). Since unfolded_region_itr_ and unfolded_bump_itr_ are initialized using these tables, this will lead to a null pointer dereference or undefined behavior when they are accessed. Please instantiate these tables in this constructor as well, similar to the first constructor.

unfolded_chip_tbl_ = new dbTable<_dbUnfoldedChip>(this, this, (GetObjTbl_t) &_dbDatabase::getObjectTable, dbUnfoldedChipObj); unfolded_region_tbl_ = new dbTable<_dbUnfoldedRegion>(this, this, (GetObjTbl_t) &_dbDatabase::getObjectTable, dbUnfoldedRegionObj); unfolded_bump_tbl_ = new dbTable<_dbUnfoldedBump>(this, this, (GetObjTbl_t) &_dbDatabase::getObjectTable, dbUnfoldedBumpObj); unfolded_conn_tbl_ = new dbTable<_dbUnfoldedConn>(this, this, (GetObjTbl_t) &_dbDatabase::getObjectTable, dbUnfoldedConnObj); unfolded_net_tbl_ = new dbTable<_dbUnfoldedNet>(this, this, (GetObjTbl_t) &_dbDatabase::getObjectTable, dbUnfoldedNetObj); unfolded_region_itr_ = new dbUnfoldedRegionItr(unfolded_region_tbl_);

gemini-code-assist · 2026-06-03T16:23:07Z

+    dbChipBump* bump = bump_inst->getChipBump();
+    if (bump->getInst() == nullptr) {


If bump_inst->getChipBump() returns nullptr, calling bump->getInst() will result in a null pointer dereference. Please add a null check for bump before accessing its members.

dbChipBump* bump = bump_inst->getChipBump(); if (bump == nullptr || bump->getInst() == nullptr) {

@maliberty FYI

gemini-code-assist · 2026-06-03T16:23:07Z

+{
+  auto& chip_region_map = region_map_[uf_chip->getOID()];
+  for (auto* region_inst : inst->getRegions()) {
+    auto region = region_inst->getChipRegion();


If region_inst->getChipRegion() returns nullptr, calling region->getSide() in the switch statement will result in a null pointer dereference. Please add a null check for region.

auto region = region_inst->getChipRegion(); if (region == nullptr) { continue; }

gemini-code-assist · 2026-06-03T16:23:07Z

+  dbChipBumpInst* bump_inst
+      = (dbChipBumpInst*) db->chip_bump_inst_tbl_->getPtr(obj->chip_bump_inst_);
+  dbInst* inst = bump_inst->getChipBump()->getInst();
+  if (inst == nullptr) {
+    return Point3D();
+  }
+  Point pt = inst->getBBox()->getBox().center();
+  dbUnfoldedRegion* region
+      = (dbUnfoldedRegion*) db->unfolded_region_tbl_->getPtr(
+          obj->parent_region_);
+  _dbUnfoldedRegion* _region = (_dbUnfoldedRegion*) region;
+  _dbUnfoldedChip* parent_chip
+      = db->unfolded_chip_tbl_->getPtr(_region->parent_chip_);
+  parent_chip->transform_.apply(pt);
+  return Point3D(pt.x(), pt.y(), region->getSurfaceZ());


This method lacks defensive null checks for bump_inst, bump_inst->getChipBump(), region, and parent_chip. If any of these lookups return nullptr, it will result in a crash. Please add appropriate null checks.

dbChipBumpInst* bump_inst = (dbChipBumpInst*) db->chip_bump_inst_tbl_->getPtr(obj->chip_bump_inst_); if (bump_inst == nullptr) { return Point3D(); } dbChipBump* bump = bump_inst->getChipBump(); if (bump == nullptr) { return Point3D(); } dbInst* inst = bump->getInst(); if (inst == nullptr) { return Point3D(); } Point pt = inst->getBBox()->getBox().center(); dbUnfoldedRegion* region = (dbUnfoldedRegion*) db->unfolded_region_tbl_->getPtr( obj->parent_region_); if (region == nullptr) { return Point3D(); } _dbUnfoldedRegion* _region = (_dbUnfoldedRegion*) region; _dbUnfoldedChip* parent_chip = db->unfolded_chip_tbl_->getPtr(_region->parent_chip_); if (parent_chip != nullptr) { parent_chip->transform_.apply(pt); } return Point3D(pt.x(), pt.y(), region->getSurfaceZ());

gemini-code-assist · 2026-06-03T16:23:07Z

+  dbChipRegionInst* reg_inst
+      = (dbChipRegionInst*) db->chip_region_inst_tbl_->getPtr(
+          obj->chip_region_inst_);
+  Cuboid c = reg_inst->getChipRegion()->getCuboid();
+  _dbUnfoldedChip* parent = db->unfolded_chip_tbl_->getPtr(obj->parent_chip_);
+  parent->transform_.apply(c);


Please add defensive null checks for reg_inst, reg_inst->getChipRegion(), and parent to prevent potential null pointer dereferences.

dbChipRegionInst* reg_inst = (dbChipRegionInst*) db->chip_region_inst_tbl_->getPtr( obj->chip_region_inst_); if (reg_inst == nullptr) { return Cuboid(); } dbChipRegion* region = reg_inst->getChipRegion(); if (region == nullptr) { return Cuboid(); } Cuboid c = region->getCuboid(); _dbUnfoldedChip* parent = db->unfolded_chip_tbl_->getPtr(obj->parent_chip_); if (parent != nullptr) { parent->transform_.apply(c); }

gemini-code-assist · 2026-06-03T16:23:08Z

+  auto describe = [](dbUnfoldedRegion* r, dbMarker* marker) {
+    const Cuboid cuboid = r->getCuboid();
+    marker->addSource(r->getChipRegionInst());
+    marker->addShape(
+        Rect(cuboid.xMin(), cuboid.yMin(), cuboid.xMax(), cuboid.yMax()));
    return fmt::format("{}/{} (faces {})",
-                       r->parent_chip->name,
-                       r->region_inst->getChipRegion()->getName(),
-                       sideToString(r->effective_side));
+                       r->getParentChip()->getName(),
+                       r->getChipRegionInst()->getChipRegion()->getName(),
+                       sideToString(r->getEffectiveSide()));
  };


Helper logic should be defined as a free function in a namespace rather than as a local lambda within a function. Please refactor the describe lambda into a free function in an anonymous namespace, and add defensive null checks for r, r->getParentChip(), and r->getChipRegionInst() to prevent potential null pointer dereferences.

namespace { std::string describe(dbUnfoldedRegion* r, dbMarker* marker) { if (r == nullptr) { return "NULL"; } const Cuboid cuboid = r->getCuboid(); marker->addSource(r->getChipRegionInst()); marker->addShape( Rect(cuboid.xMin(), cuboid.yMin(), cuboid.xMax(), cuboid.yMax())); dbUnfoldedChip* parent = r->getParentChip(); std::string chip_name = parent ? parent->getName() : "UNKNOWN"; dbChipRegionInst* region_inst = r->getChipRegionInst(); std::string region_name = (region_inst && region_inst->getChipRegion()) ? region_inst->getChipRegion()->getName() : "UNKNOWN"; return fmt::format("{}/{} (faces {})", chip_name, region_name, sideToString(r->getEffectiveSide())); } } // namespace

References

Helper logic should be defined as a free function in a namespace rather than as a local lambda within a function.

gemini-code-assist · 2026-06-03T16:23:08Z

+  auto get_net_name = [&](dbUnfoldedBump* bump) -> std::string {
    auto it = bump_net_map.find(bump);
    if (it != bump_net_map.end()) {
-      return it->second->chip_net->getName();
+      return it->second->getChipNet()->getName();
    }
    return "defines no net";
  };


Helper logic should be defined as a free function in a namespace rather than as a local lambda within a function. Please refactor the get_net_name lambda into a free function in an anonymous namespace, passing the map as a parameter, and add a null check for the returned chip net.

namespace { std::string get_net_name(dbUnfoldedBump* bump, const std::map<dbUnfoldedBump*, dbUnfoldedNet*>& bump_net_map) { auto it = bump_net_map.find(bump); if (it != bump_net_map.end() && it->second != nullptr) { dbChipNet* chip_net = it->second->getChipNet(); if (chip_net != nullptr) { return chip_net->getName(); } } return "defines no net"; } } // namespace

References

Helper logic should be defined as a free function in a namespace rather than as a local lambda within a function.

gemini-code-assist · 2026-06-03T16:23:08Z

+    if (path.empty()) {
+      continue;
+    }
+    dbBlock* block = path.back()->getMasterChip()->getBlock();


Please add defensive null checks for path.back() and path.back()->getMasterChip() to prevent potential null pointer dereferences.

Suggested change

dbBlock* block = path.back()->getMasterChip()->getBlock();

dbChipInst* leaf = path.back();

if (leaf == nullptr) {

continue;

}

dbChip* master_chip = leaf->getMasterChip();

if (master_chip == nullptr) {

continue;

}

dbBlock* block = master_chip->getBlock();

gemini-code-assist · 2026-06-03T16:23:08Z

+      if (conn->getTopRegion() && conn->getBottomRegion()) {
+        auto it1 = chip_map.find(conn->getTopRegion()->getParentChip());
+        auto it2 = chip_map.find(conn->getBottomRegion()->getParentChip());
        if (it1 != chip_map.end() && it2 != chip_map.end()) {
          uf.unite(it1->second, it2->second);
        }


To improve robustness, please check that getParentChip() does not return nullptr before performing the map lookup.

if (conn->getTopRegion() && conn->getBottomRegion()) { dbUnfoldedChip* top_chip = conn->getTopRegion()->getParentChip(); dbUnfoldedChip* bot_chip = conn->getBottomRegion()->getParentChip(); if (top_chip != nullptr && bot_chip != nullptr) { auto it1 = chip_map.find(top_chip); auto it2 = chip_map.find(bot_chip); if (it1 != chip_map.end() && it2 != chip_map.end()) { uf.unite(it1->second, it2->second); } } }

gemini-code-assist · 2026-06-03T16:23:08Z

+  for (const auto& id : obj->chip_inst_path_) {
+    path.push_back((dbChipInst*) db->chip_inst_tbl_->getPtr(id));
+  }


To prevent nullptr elements from being added to the returned path, please filter out null pointers returned by getPtr.

for (const auto& id : obj->chip_inst_path_) { dbChipInst* inst = (dbChipInst*) db->chip_inst_tbl_->getPtr(id); if (inst != nullptr) { path.push_back(inst); } }

maliberty · 2026-06-03T17:02:46Z

@codex review

chatgpt-codex-connector · 2026-06-03T17:10:28Z

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ 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".

…ructuree

Signed-off-by: osamahammad21 <osama@precisioninno.com>

…ructuree

maliberty · 2026-06-04T19:49:13Z

+      "name": "effective_side_",
+      "type": "uint32_t",
+      "default": 0,
+      "flags": ["private"]


There are only four values. Why not use a "bits": 2 ?

maliberty · 2026-06-04T19:50:29Z

+  if (master->getChipType() == dbChip::ChipType::HIER) {
+    for (auto* sub : master->getChipInsts()) {


No unfolding of HIER chips implies you are flattening rather than unfolding.

maliberty · 2026-06-04T19:53:03Z

+void dbUnfoldedBuilder::unfoldNets(dbChip* chip,
+                                   const std::vector<dbChipInst*>& parent_path)
+{
+  for (auto* net : chip->getChipNets()) {
+    _dbUnfoldedNet* uf_net = db_->unfolded_net_tbl_->create();


How does this work in the presence of hiearchy? It seems like you would get hierarhical nets

maliberty · 2026-06-04T21:27:29Z

Have you resolved the gemini comments?

maliberty · 2026-06-05T03:37:44Z

Rename classes as discussed to better align unfolded names to the object they are unfolding.

osamahammad21 added 2 commits June 3, 2026 19:11

odb: replace struct UnfoldedModel with dbObject-backed unfolded model

01900ec

Signed-off-by: osamahammad21 <osama@precisioninno.com>

Merge remote-tracking branch 'origin/master' into odb-new-unfolded-st…

97bd1d3

…ructuree Signed-off-by: osamahammad21 <osama@precisioninno.com>

osamahammad21 requested a review from a team as a code owner June 3, 2026 16:19

osamahammad21 requested a review from maliberty June 3, 2026 16:19

github-actions Bot added the size/XL label Jun 3, 2026

gemini-code-assist Bot reviewed Jun 3, 2026

View reviewed changes

osamahammad21 added 3 commits June 4, 2026 13:17

Merge remote-tracking branch 'origin/master' into odb-new-unfolded-st…

06a09d2

…ructuree

odb: fix clang-tidy issues

d9f4874

Signed-off-by: osamahammad21 <osama@precisioninno.com>

Merge remote-tracking branch 'origin/master' into odb-new-unfolded-st…

bc7f988

…ructuree

maliberty reviewed Jun 4, 2026

View reviewed changes

Comment thread src/odb/src/db/dbDatabase.h

maliberty reviewed Jun 4, 2026

View reviewed changes

Comment thread src/odb/src/codeGenerator/schema/chip/dbUnfoldedChip.json

maliberty reviewed Jun 4, 2026

View reviewed changes

		dbChipBump* bump = bump_inst->getChipBump();
		if (bump->getInst() == nullptr) {

-    dbBlock* block = path.back()->getMasterChip()->getBlock();
+    dbChipInst* leaf = path.back();
+    if (leaf == nullptr) {
+      continue;
+    }
+    dbChip* master_chip = leaf->getMasterChip();
+    if (master_chip == nullptr) {
+      continue;
+    }
+    dbBlock* block = master_chip->getBlock();

		if (master->getChipType() == dbChip::ChipType::HIER) {
		for (auto* sub : master->getChipInsts()) {

Conversation

osamahammad21 commented Jun 3, 2026

Summary

Type of Change

Impact

Verification

Related Issues

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

osamahammad21 Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty commented Jun 3, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jun 3, 2026

Uh oh!

Uh oh!

Uh oh!

maliberty Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty commented Jun 4, 2026

Uh oh!

maliberty commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants