odb: impl 3dblox parser path asserstions by openroad-ci · Pull Request #10055 · The-OpenROAD-Project/OpenROAD

openroad-ci · 2026-04-04T20:16:28Z

Summary

Add path assertion checking to the 3DBlox checker. Path assertions (parsed from .3dbx files) specify must-touch and do-not-touch region constraints. The checker validates these by building a CSR routing graph from the unfolded model and running BFS connectivity queries to detect violations.

Key changes:

Expose PathAssertionEntry/PathAssertion structs in the public header with a region_inst field resolved at parse time
Add getPathAssertions()/setPathAssertions() API on ThreeDBlox
Implement checkPathAssertions in the checker using a CSR routing graph + BFS
Add dbChip::setIsBlackbox()/isBlackbox() so blackbox chips get intra-chip clique edges in the routing graph
New routingGraph.cpp/h for O(V+E) CSR graph construction
6 new checker tests and 2 new parser tests

Type of Change

New feature

Impact

ThreeDBlox::check() now validates path assertions in addition to existing checks (logical connectivity, floating chips, overlaps, etc.)
Chips without a DEF file are marked as blackbox, causing all their regions to be treated as mutually reachable in the routing graph
New schema revision (129) for the blackbox_ field on dbChip; backward-compatible with schema 128 databases

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

Closes: #9300

gemini-code-assist

Code Review

This pull request implements path assertion validation within the 3DBlox checker. It introduces a 'blackbox' attribute for chips to handle components without detailed routing and adds a CSR-based RoutingGraph to model design connectivity. The checker now utilizes a BFS-based algorithm to verify reachability constraints defined in path assertions. Feedback was provided regarding the use of magic numbers for buffer capacities in the BFS state.

gemini-code-assist · 2026-04-04T20:19:31Z

+    queue.reserve(std::min<uint32_t>(n, 4096u));
+    dirty_blocked.reserve(64);
+    dirty_target.reserve(64);


The values 4096u and 64 are magic numbers. To improve readability and maintainability, they should be defined as named constants. This also aligns with the general rule to avoid hardcoded magic numbers for tunable parameters.

For example, you could add them to the BfsState struct:

struct BfsState { static constexpr uint32_t kInitialBfsQueueCapacity = 4096u; static constexpr uint32_t kInitialDirtyListCapacity = 64u; std::vector<uint8_t> is_blocked; // ... explicit BfsState(uint32_t n) : is_blocked(n, 0), is_target(n, 0), visited(n, 0) { queue.reserve(std::min<uint32_t>(n, kInitialBfsQueueCapacity)); dirty_blocked.reserve(kInitialDirtyListCapacity); dirty_target.reserve(kInitialDirtyListCapacity); } // ... };

References

Define tunable parameters as named constants instead of using hardcoded magic numbers.

github-actions · 2026-04-04T20:22:35Z

clang-tidy review says "All clean, LGTM! 👍"

osamahammad21

Still review BFSstate

osamahammad21 · 2026-04-05T10:06:20Z

+static std::vector<std::string> splitPath(const std::string& path)
+{
+  std::vector<std::string> parts;
+  std::istringstream stream(path);
+  std::string part;
+
+  while (std::getline(stream, part, '/')) {
+    if (!part.empty()) {
+      parts.push_back(part);
+    }
+  }
+
+  return parts;
+}


use boost::split

osamahammad21 · 2026-04-05T10:11:56Z

@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright (c) 2023-2026, The OpenROAD Authors


osamahammad21 · 2026-04-05T10:16:57Z

@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright (c) 2023-2026, The OpenROAD Authors


osamahammad21 · 2026-04-05T10:29:26Z

+    if (!conn.top_region || !conn.bottom_region) {
+      continue;  // Virtual (ground) connection — no region-to-region edge.
+    }


I don't think we should check if top_region is null because that looks like a fail safe. At this point, top_region should never be null, right?

I'm not sure if top or bottom are what we expect to be connected to ground. I think defensive programming is better. If you know that top must not be null tell me and I'll check and error.

According to the standard, only the bottom region is allowed to be ~ (virtual plane). So the top cannot null. In general, I am against defensive programming because it leads to having major issues unnoticed. I would rather a crashing program than a silently wrong one.

I agree with "I would rather a crashing program than a silently wrong one" as I said I'll error in the bad case. I won't continue.

osamahammad21 · 2026-04-05T10:31:17Z

+    if (it_top == region_to_idx.end() || it_bot == region_to_idx.end()) {
+      continue;
+    }


Another fail safe that shouldn't happen. A connection should never hold a region that is not defined already in the unfolded model.

You're right, this should be an error (assert) not continue.

assert is sufficient if you think it is needed.

Asserts might be dropped out in a release build. I think we should error.

osamahammad21 · 2026-04-05T10:35:38Z

+        const auto it_b = region_to_idx.find(&regions[b]);
+        if (it_b == region_to_idx.end()) {
+          continue;
+        }


osamahammad21 · 2026-04-05T10:36:35Z

+        edges.push_back({u, v});
+        g.offsets[u + 1]++;
+        g.offsets[v + 1]++;


Actually, even in blackbox, connecting all regions is not correct according to the standard.

From the spec: "During the blackbox stage, all intra-chiplet regions are assumed to be internally connected".

It depends on the chiplet type and whether it is TSV or not.

osamahammad21 · 2026-04-05T10:45:05Z

+  void setBlackbox(bool blackbox);
+
+  bool isBlackbox() const;
+


I don't think that the blackbox state should be user defined. It should be inferred from the current state of the chiplet. From looking at the code, I can see that you set it once and never change it again even if the netlist is added later with routing.
For now let's remove it and assume that the path assertions are in the scope of the blockbox state only.

Agreed, I did not mean for this to be used by a user. I wanted the system to change it. Do you think there is a better mechanism to track what stage the system is in? I'll check if we have something already.

osamahammad21 · 2026-04-05T10:45:29Z

 #include "objects.h"
 #include "odb/db.h"
 #include "utl/Logger.h"
-#include "yaml-cpp/yaml.h"


osamahammad21 · 2026-04-05T10:48:32Z

+struct RoutingGraph
+{
+  std::vector<uint32_t> offsets;    // size num_nodes+1; CSR row pointers
+  std::vector<uint32_t> neighbors;  // all adjacency lists concatenated
+  uint32_t num_nodes = 0;
+};


use boost::compressed_sparse_row_graph

Signed-off-by: Ahmed R. Mohamed <ahmed@precisioninno.com>

Use boost graph library Implement all spec region connection rules. Signed-off-by: Ahmed R. Mohamed <ahmed@precisioninno.com>

github-actions · 2026-05-14T01:51:31Z

clang-tidy review says "All clean, LGTM! 👍"

openroad-ci assigned ahmed532 Apr 4, 2026

gemini-code-assist Bot reviewed Apr 4, 2026

View reviewed changes

ahmed532 requested a review from osamahammad21 April 4, 2026 20:34

osamahammad21 requested changes Apr 5, 2026

View reviewed changes

Ahmed R. Mohamed added 2 commits May 13, 2026 19:24

odb: impl 3dblox parser path asserstions

4a7eb1d

Signed-off-by: Ahmed R. Mohamed <ahmed@precisioninno.com>

odb: fix 3dblox path assertions PR

1ffe304

Use boost graph library Implement all spec region connection rules. Signed-off-by: Ahmed R. Mohamed <ahmed@precisioninno.com>

openroad-ci force-pushed the feat/3dblox-checker-pathAssertions branch from 5fadd7d to 1ffe304 Compare May 14, 2026 01:45

openroad-ci requested a review from a team as a code owner May 14, 2026 01:45

openroad-ci requested a review from maliberty May 14, 2026 01:45

github-actions Bot added the size/L label May 14, 2026

		@@ -0,0 +1,33 @@
		// SPDX-License-Identifier: BSD-3-Clause
		// Copyright (c) 2023-2026, The OpenROAD Authors

		@@ -0,0 +1,133 @@
		// SPDX-License-Identifier: BSD-3-Clause
		// Copyright (c) 2023-2026, The OpenROAD Authors

Conversation

openroad-ci commented Apr 4, 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 Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 4, 2026

Uh oh!

osamahammad21 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants